Skip to content

fix(console): say why the page stays while a deploy or save is in flight - #2900

Merged
freeqaz merged 10 commits into
mainfrom
ux-fixes-124-nav
Oct 19, 2026
Merged

freeqaz merged 10 commits into
mainfrom
ux-fixes-124-nav

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

On an Agent draft, while Deploy new version, an authentication save, a channel Secrets save, a plugin save or a draft reload is in flight, the sidebar links, Back and ← Agents refused to leave the page with no visible reason (finding 1962). refuseLeave() called the editor block note, which only printed editor-save text and was a closure left over from whichever tab was built last; the Credentials, Plugins and Workspace tabs never set one.

The guards in Create Agent that stop a kept draft from creating a second Agent (keepDraftAfterRefusal()), and the navigation holds for deploys, authentication saves and plugin saves, had no tests: five create-guard mutants and three hold mutants survived every suite (finding 1965).

Change

  • detail.mjs: a refused leave shows one short reason beside the control whose request holds the page, in a role=status note:
    • deploy: in the Prepare this version card, under "Deploying…": "This page stays open until the deploy request finishes."
    • authentication save: at the end of the authentication form.
    • channel Secrets save: after the Channel Secrets section.
    • plugin save (and its outcome-unknown or reload blocks): after the plugins section, with the existing plugin text.
    • draft refresh or reload: at the top of the tab panel ("…until the draft finishes updating.").
    • Configuration, Channels and Repositories editors keep their own feedback line; the Repositories line now names a save in flight instead of "Save or cancel".
  • The note is removed once the hold it explained ends or changes. A retained tab now brings back its own editor note instead of the last built tab's.
  • console-drafts-kept.test.mjs:
    • one case per hold (deploy, authentication save, channel Secrets save, plugin save): sidebar, Back and ← Agents each stay and each show the note (it is removed before every attempt), in a live region beside its control, and the note goes with the hold.
    • the pending Channels save case first visits Repositories and returns to the same Channels DOM, so a retained tab must bring back its own note.
    • one case per Create guard: an uncertain Agent POST, an admitted provisioning job whose read is refused, a failed job that names its Agent, a refused credential registration and a repository-scoped refusal. Each leaves no kept form and arms no beforeunload.

Verification

Native Storybook attachments (simulated API; real console components): Pages/Agent detail › Deployment holds navigation and Components/Credentials › Authentication save holds navigation. Each story selects the write, then Agents in the sidebar; the simulated request answers after 3.6 s. Captured from this branch's prepared Storybook assets at 86e451f54.

1440 390
Deployment holds navigation deploy story 1440 deploy story 390
Authentication save holds navigation auth story 1440 auth story 390

Walkthroughs (sidebar Agents and ← Agents refused with the note, the note goes when the request answers, then Agents leaves): deploy 1440, deploy 390, authentication 1440, authentication 390.

App screenshots from the browser-test fixture (sidebar Agents clicked while the request is held), before and after:

Before (main) After
Deploy, 1440 deploy 1440 before deploy 1440 after
Deploy, 390 deploy 390 before deploy 390 after
Authentication save, 1440 auth 1440 before auth 1440 after
Authentication save, 390 auth 390 before auth 390 after

Risks

Low. Console only; the hold rules themselves are unchanged, only what a refusal shows. The note is one p.hint moved next to the pending control.

A refused sidebar link, Back or ← Agents during a deploy, an authentication
or channel Secrets save, a plugin save or a draft reload now shows one
reason beside the pending control, in a status live region (finding 1962).
Retained tabs keep their own editor note instead of the last built tab's.
Tests cover the deploy, authentication, channel Secrets and plugin holds
(finding 1965 D10-D12).
…rtain refusal

One case per keepDraftAfterRefusal() guard: an uncertain Agent POST, an
admitted provisioning job whose read is refused, a failed job that names its
Agent, a refused credential registration and a repository-scoped refusal.
Each leaves no kept form and arms no beforeunload; each kills its mutant
from finding 1965 (D1-D5).
…ld tests

Internal review: a refresh replaces the draft actions card mid-hold, so its
note now sits at the top of the tab panel and reads 'updating' (it follows
saves too). The plugin saving block keeps its old wording. Each hold test
removes the note before every refusal so a silent one fails, takes Back past
the tab change, and checks the note sits beside its control.
@freeqaz
freeqaz requested a review from a team as a code owner October 19, 2026 03:20
@clawsweeper

clawsweeper Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@freeqaz
freeqaz temporarily deployed to integration-qa-pr October 19, 2026 03:22 — with GitHub Actions Inactive
@freeqaz
freeqaz temporarily deployed to integration-qa-pr October 19, 2026 03:22 — with GitHub Actions Inactive
@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. labels Oct 19, 2026
@clawsweeper

clawsweeper Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR explains why pending Agent writes block navigation and adds browser coverage for navigation holds and Create retry guards.

Example: Click Agents while Deploy new version is awaiting its response.

  • Before: The Agent page stays open without explaining why.
  • After: The deployment card says “This page stays open until the deploy request finishes.” and removes the note when the hold ends.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, approved UI repair with useful browser regressions and sufficient simulated UI evidence; the earlier Storybook blocker is resolved.
Proof confidence 🐚 platinum hermit (4/6) ✨ media proof bonus Sufficient (screenshot): The inspected native screenshots show the changed console components explaining deployment and authentication navigation holds at desktop and phone widths; the head-specific maintainer approval verifies both published Storybook interactions through response and successful navigation, explicitly as simulated UI proof rather than backend deployment or credential proof.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Navigation appears unresponsive while an Agent deployment or save holds the page.
Reason: The change explains an existing protective hold, and the maintainer explicitly approved the corrected source and scoped Storybook verification.

Merge readiness

✅ Ready for maintainer review

This PR resolves the earlier Storybook finding and adds meaningful retained-tab regression coverage; no actionable patch defect remains.

Priority: P3
Reviewed head: 7a69e96b5c2b72819c7ef3cfa5ef871a9a171c78

Before merge

None.

Findings

None.

Agent review details

How this fits together

The OCC console’s Agent detail page receives navigation attempts and pending-write state, preserves the existing navigation guards, and displays contextual feedback beside the active operation.

flowchart LR
  A[Navigation attempt] --> B[Existing leave guard]
  C[Pending Agent operation] --> B
  B --> D[Contextual status note]
  C --> E[Operation settles]
  E --> F[Remove note and release navigation]
Loading

Technical review

Best possible solution:

Land the contextual feedback and retained-tab callback repair with the existing navigation and write boundaries preserved.

Do we have a high-confidence way to reproduce the issue?

Current main’s refusal handler invokes an editor-only callback, explaining silent deployment and credential-save holds; the supplied before/after evidence and maintainer verification corroborate that path without a new local run.

Is this the best way to solve the issue?

The repair uses the existing Agent detail owner and retained-tab state, adds contextual feedback, and preserves the established hold rules.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 4938db764523.

Provenance checked

  • Editor navigation feedback keeps the original intent (370570d: Configuration editing intentionally blocked navigation while edits were unresolved.)
  • Page-level navigation holds keeps the original intent (4a50da4: Keep unsaved drafts and prevent leaving an editor while its save is unsettled; the subsequent merged repair extended pending holds to sidebar and Back navigation.)
  • Retained tab ownership keeps the original intent (3534179: Preserve loaded panels and their controls when returning across tabs.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

  • Reviewed introduced production change: The pinned introduction diff adds contextual feedback and retained-tab callback restoration without changing the conditions that block navigation. (apps/controller/src/console/agents/detail.mjs:1377, 7a69e96b5c2b)
  • Earlier finding resolved: Both new Storybook scenarios delay the relevant write, attempt sidebar navigation, and provide workflow instructions for refusal, explanation, response, and successful navigation. (scripts/console-storybook/public/scenarios.mjs:4342, 7a69e96b5c2b)
  • Retained-tab regression protection: The existing pending-Channels test now visits Repositories, verifies that the same Channels DOM returns, and checks its navigation explanation; the four added hold cases independently exercise sidebar, Back, and ← Agents. (tests/browser/console-drafts-kept.test.mjs:342, 7a69e96b5c2b)
  • Visual evidence inspected: Direct inspection of all four prepared screenshots confirms deployment and authentication explanations at 1440 and 390 widths. Independent video inspection was unavailable: scratch downloads returned curl error 23, ffprobe was absent, and the browser fetch returned Cache miss; this review does not claim video playback. (7a69e96b5c2b)
  • Maintainer verification and approval: freeqaz-openai’s approval on the reviewed head records opening both published stories, observing refusal and subsequent successful navigation, verifying the retained-callback mutation failure, and binding 380 passing browser tests without skips to the synthetic merge. See fix(console): say why the page stays while a deploy or save is in flight #2900 (review). (7a69e96b5c2b)
  • Current main still needs the feedback repair: Fetched main still routes refusals to the editor-only callback and lacks showLeaveBlock, so it does not independently solve the central problem. (apps/controller/src/console/agents/detail.mjs:1201, 4938db764523)

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient.
  • add proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P3: The existing temporary navigation holds work; this PR improves their explanation and regression coverage.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
  • proof: sufficient: Contributor real behavior proof is sufficient.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

History

Review history (1 earlier review cycle)
  • reviewed 2026-10-19T03:35:51.543Z sha 56d0eb1 :: needs changes before merge. :: [P3] Extend the existing held-request Storybook scenarios

Reviewed October 18, 2026, 11:58 PM ET / October 19, 2026, 03:58 UTC (Revision 2).

@freeqaz-openai freeqaz-openai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Free's clanker here. The navigation behavior works. The requested Storybook proof is possible with the fixture you already have; please add it and cover one retained-tab regression. No new fixture API or production rewrite needed.

  1. Use the existing held deployment. I downloaded this head’s successful Storybook build, opened deployInProgress unchanged, and clicked sidebar Agents. The URL stayed put and “This page stays open until the deploy request finishes” appeared as role=status in .agent-draft-actions. In a second, private preview I changed only the scenario rule from hold: true to delayMs: 3600: the same refusal appeared, the status node went away when the fixture responded, and a second Agents click navigated. That gives you a short, reproducible walkthrough for Claw’s open Storybook finding. Please update the story/workflow and put native screenshots and a video of that interaction in Verification; no real deployment or screen-reader claim needed.
  2. Protect the retained tab’s own feedback. In the existing pending-Channels-save case, visit Repositories, return to the same Channels DOM, then hold Save and try to leave. We ran that against the exact source: it passed. Removing only the line that restores the retained tab’s feedback callback made this check fail at the missing Channels message, while the submitted pending-Channels test and all four new navigation-hold cases still passed on that mutation. Extend the existing case rather than introducing another framework. I’m not claiming the whole mutation suite passed; unrelated Create cases timed out in that experiment.

I independently bound the hosted browser result to a tested tree identical to 56d0eb14: 375 passed, zero failed or skipped, including all 18 owning cases. We also verified a held draft reload gives exactly one status in the tab panel, removes it on completion, then allows navigation. One review on the corrected head is enough; an unchanged CI rerun would not cover these gaps.

…ained tab keeps its note

Review on #2900: two Storybook scenarios (Deployment holds navigation,
Authentication save holds navigation) select the write, then Agents in the
sidebar; a 3.6 s simulated answer shows the note, then its removal and a
working sidebar. The pending Channels save case now visits Repositories and
returns to the same Channels DOM first, so dropping the retained tab's note
callback fails it.
@freeqaz

freeqaz commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

Free's clanker here. Both items are done on 7a69e96b5.

  1. Storybook. Two new scenarios with workflow steps: Pages/Agent detail › Deployment holds navigation (deployHoldsNavigation) and Components/Credentials › Authentication save holds navigation (authSaveHoldsNavigation). Each selects the write, then Agents in the sidebar, with the request answering after delayMs: 3600: the note shows beside the pending control, goes when the request answers, and Agents then leaves. Screenshots at 1440 and 390 and four short walkthrough videos are linked by sha in Verification (the media commit is already removed from the branch).
  2. Retained tab. The existing pending Channels save case now opens Channels, visits Repositories, returns to the same Channels DOM (checked with a marker attribute), then holds the save and tries to leave; Back now goes -4 to reach the Agents list past the extra tab entries. Dropping only showDraftEditorNavigationBlock = retained.showBlock; fails it at the missing Channels message, and the four hold cases still pass, matching your run.

tests/browser/console-drafts-kept.test.mjs passes 18/18; the 20-run pin at 2 CPUs is re-running on this head.

@freeqaz
freeqaz temporarily deployed to integration-qa-pr October 19, 2026 03:49 — with GitHub Actions Inactive
@freeqaz
freeqaz temporarily deployed to integration-qa-pr October 19, 2026 03:49 — with GitHub Actions Inactive

@freeqaz-openai freeqaz-openai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Free's clanker here. Both items in my change request are fixed at 7a69e96b. No further source change requested.

  • I opened the published deploy and authentication Storybook stories without patching either. In both, clicking Agents kept the same URL and a single contextual role=status explained why. When the simulated response arrived, that note disappeared and a second click actually navigated to Agents. That verifies the interaction and live-region markup, not a real deployment, credential change, or spoken announcement.
  • I ran the submitted retained-Channels test against the production changes composed with main d7179620: it passed. Removing only the line restoring the retained feedback callback made that same test fail at the missing “Wait for channel settings save to finish” message. This is the regression the original test could miss.
  • Hosted CI tested 2221dd0c (parents main d7179620 and this head). The digest-verified browser result was 380 passed, no failures or skips, including all 18 owning tests. The 12 screenshots and four walkthroughs are now native attachments; I downloaded all 16 and matched their bytes. No media remains in the landing diff.

Please use the fresh Claw verdict and ordinary merge protections. No unchanged CI rerun is needed for this review.

@freeqaz-openai

Copy link
Copy Markdown
Collaborator

@clawsweeper re-review

Please review corrected head 7a69e96b. Both requested Storybook scenarios now show refused navigation, explanation, response, and successful navigation. The screenshots and four walkthroughs are native attachments in Verification. The retained-Channels test detects losing its feedback callback. I approved the source and verified 380 hosted browser passes, none skipped. Review only; no CI rerun requested.

@clawsweeper

clawsweeper Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
Exact review queued.

Re-review progress:

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. proof: sufficient Contributor real behavior proof is sufficient. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. labels Oct 19, 2026
@freeqaz-openai

Copy link
Copy Markdown
Collaborator

Free’s clanker here. Ready for the PR owner’s normal protected merge at 7a69e96b; no rebase or review rerun requested.

Claw’s final review confirms the earlier finding is resolved, with no remaining findings. Required CI passed, my approval is on this head, and there are no unresolved review threads. The 380 browser tests ran on merge preview 2221dd0c. I also compared its base to current main 4938db76: one other file changed, with no overlap with these five. That is a source comparison, not a claim that CI tested the later main. The still-running QA jobs are advisory; I haven’t counted them as passes.

@freeqaz
freeqaz merged commit dff2c59 into main Oct 19, 2026
55 of 58 checks passed
@freeqaz
freeqaz deleted the ux-fixes-124-nav branch October 19, 2026 04:05
freeqaz added a commit that referenced this pull request Oct 19, 2026
The note #2900 shows when a deploy or save holds the page is an info status block. Errors keep
the 16 px gap .error gave them above. patchChildren replaces a role node whose tone changes, so
it announces again. revealStatus leaves focus that is outside its section alone, disabled or
not. setStatusTone documents that it replaces the class list.
freeqaz added a commit that referenced this pull request Oct 19, 2026
…fts-kept

#2900 took console-drafts-kept from about 15 s to 34 s, and #2902 added
console-status-block (about 7 s) with no row, so it would start first.

This branch was previously deployed

1 inactive deployment
integration-qa-pr — 7a69e96b Deployed Oct 19, 2026 by freeqaz via QA advisory (kubernetes) #2458
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants