Skip to content

fix(console): show a watched deploy failure as a reload does - #3008

Merged
freeqaz merged 11 commits into
mainfrom
fdr32-fail
Oct 20, 2026
Merged

freeqaz merged 11 commits into
mainfrom
fdr32-fail

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Dogfood round 32 (finding 2094, D664, medium): when a deploy fails while the Agent page is open, the "Deployment failed" card lands below the overview and below the auto-expanded Deployment details, about 960 px down at 1440 (1,550 px at 390), and the failure's error text shows three times (card, Deployment details, the version's record). A reload of the same page puts the card first, under the header. Finding 1655 had moved the live card down there to avoid pushing the overview while the reader watched; with the details expanded that put the next step below the fold.

Change

  • agents/detail.mjs: a failure that lands while the page shows keeps the card where a load puts it, first in the page. The first time the card shows for a failed version, the Deployment details fold: the card already carries the code, the API's message and its next step. The reader can open the details again and they stay open. The card sits inside the view's polite live region, so its appearance is announced; the details' alert no longer repeats it.
  • Tests: the 1655 test becomes "a watched deploy that fails shows its recovery as a reload does" (1440 and 390). It records the card's position, the details' open state and how many visible nodes carry the error, live and after a reload, and requires them equal: card first, details folded, error twice (the card and the selected version's own record), action above the fold. Tests that read the details' contents after a failure now open them first.
  • Docs and Storybook: docs/reference/console/versions-and-channel-drafts.md (where docs(console): split near-cap console reference and flow pages #3013 moved the section) and docs/guides/console/agent-details.md say a failure's card leads the page and folds the details; npm run docs:check-length and the docs link check pass. The deploymentFailsWhileWatching scenario's description and steps describe the new placement and the fold.

This deliberately reverses 1655's placement: the overview moves down once when the failure lands, which is what the brief for 2094 asks ("render the same as a fresh load"). The folded details give most of that height back to the cards below.

  • Synced with main after feat(console): move Share Agent into an Access tab #2999 (Share Agent on its own tab): the page is shorter, so the error now shows once (on the card) and at 1440 the version detail can start in the lower half of the first screen. The watched-failure test expects one; the four recovery-link tests require the detail to start outside the upper half before the click, which their in-view check after it needs.
  • Review fixes: the fold skips a panel that shows a revision cleanup, startup warnings or the "could not be refreshed" warning, because the card carries none of them. Tests: the cleanup test asserts the status is visible before any click, and a new deploy-again test covers a Refresh that reads the failure while its Agent read fails. Both fail on the head before the fix.

Verification

  • New test fails on main (watched: {leadsOverview:false, top:957, detailsOpen:true, errorShown:3} vs reload {true, 180, true, 3} at 1440; top:1553 at 390) and passes with the fix; 20/20 at 2 CPUs.
  • Touched and related browser files green locally on the merged head: console-agent-overview, -deploy-again, -detail, -detail-2, -state, -draft-guards, console-agents, console-button-hierarchy, console-retained-revalidation, console-repository-dedicated.
  • prettier and eslint clean on touched files.

Media (fixture: v1 current, v2 deploying, then its model check fails and Refresh deployment reads it; first screen, and the whole page at 1440):

before (main) after
1440, first screen fail-before-1440-fold fail-after-1440-fold
390, first screen fail-before-390-fold fail-after-390-fold
1440, full page fail-before-1440-full fail-after-1440-full

Risks

A deploy that failed while the Agent page was open put the 'Deployment
needs attention' card below the auto-expanded Deployment details, below
the fold at 1440, and the error showed three times; a reload put the
card first (finding 2094). The card now keeps its place at the top on a
live failure too, and the first time it shows for a failed version the
Deployment details fold, since the card carries the error and its next
step. This reverses finding 1655's placement: the overview moves down
once when the failure lands, as the brief for 2094 asks.
@freeqaz
freeqaz requested a review from a team as a code owner October 20, 2026 05:06
@clawsweeper

clawsweeper Bot commented Oct 20, 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.

@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. P2 Normal priority bug or improvement with limited blast radius. 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 20, 2026
@clawsweeper

clawsweeper Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR puts watched deployment failures first on the Agent page and folds redundant details while preserving cleanup, startup warnings, and refresh errors.

Example: Version v2 fails its startup model check while v1 remains selected.

  • Before: Recovery appears below the overview and expanded deployment details, unlike after a reload.
  • After: Recovery leads the page, with ordinary failure details folded and Open v2 Logs visible.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The focused patch and regression coverage support correctness; supplied static evidence only partially satisfies the repository's UI verification requirement.
Proof confidence 🦐 gold shrimp (3/6) Real behavior proof is not required for maintainer- or bot-authored pull requests.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: A watched deployment failure places its recovery action below expanded details, while reloading the same failure puts recovery first.
Reason: The maintainer-authored direction resolves a demonstrated inconsistency and explicitly accepts the layout movement, while keeping consequential evidence visible.

Merge readiness

✅ Ready for maintainer review

Keep open: this PR fixes a concrete console inconsistency that remains on main. The previous refresh-warning finding is resolved, and no remaining code defect was identified.

Priority: P2
Reviewed head: d0503a3e671bac6b5d1e75d6b03546df62be2148

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: Provide current-revision visual walkthrough evidence of watched failure, reload parity, and preserved cleanup/refresh warnings; supplied static screenshots do not establish those transitions.
Agent review details

How this fits together

The console Agent detail view consumes deployment records and Agent/version reads, then renders recorded outcomes, recovery actions, and supporting evidence.

flowchart TB
  A[Deployment poll or Refresh] --> B[Deployment and Agent reads]
  B --> C[Deployment status panel]
  C --> D[Agent overview]
  C --> E[Failure recovery card]
  E --> F[Conditional details fold]
Loading

Technical review

Best possible solution:

Keep the focused presentation fix and its exceptions for evidence that the recovery card does not contain.

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

Current main explicitly relocates watched failure recovery below deployment activity; the supplied screenshots and reported browser regression demonstrate the corresponding layout difference, although no target code was run during this review.

Is this the best way to solve the issue?

The patch uses the existing recovery card and disclosure owners, preserves distinct cleanup and warning evidence, and replaces the old placement assertion with watched-versus-reloaded coverage.

AGENTS.md: found and applied where relevant.

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

Provenance checked

  • Watched failure card placement changes intended behavior with a stated reason (ce11728: Keep newly failed deployment recovery near the activity being watched without inserting a card above the overview.)
  • Deployment details visibility changes intended behavior with a stated reason (4bf2532: Fold routine completion while showing changed outcomes, pending work, read errors, startup warnings, and cleanup.)

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

Likely related people:

  • kevinlin-openai: Raw commit cbcb8c0 adds apps/controller/src/console/agents/detail.mjs:52 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: medium; commits: cbcb8c0a5935; files: apps/controller/src/console/agents/detail.mjs)

Labels

Label changes:

  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp 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: 🦐 gold shrimp, 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:

  • P2: Failure recovery is harder to find while watching a deployment, but remains reachable by scrolling or reloading.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Complete the current-revision screenshot and walkthrough evidence described in realBehaviorProof.

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 (6 earlier review cycles)
  • reviewed 2026-10-20T05:21:40.719Z sha b37829a :: needs changes before merge. :: [P3] Update the walkthrough and guidance for folded failures
  • reviewed 2026-10-20T05:30:51.673Z sha 4b53dc0 :: needs maintainer review before merge. :: none
  • reviewed 2026-10-20T05:48:03.824Z sha 02879de :: needs maintainer review before merge. :: none
  • reviewed 2026-10-20T06:02:57.726Z sha 15f77ea :: needs maintainer review before merge. :: none
  • reviewed 2026-10-20T06:21:33.029Z sha 2c6388d :: needs maintainer review before merge. :: none
  • reviewed 2026-10-20T06:36:15.321Z sha b553fb9 :: needs changes before merge. :: [P2] Preserve the failed-refresh warning when folding failure details

Reviewed October 20, 2026, 2:58 AM ET / 06:58 UTC (Revision 7).

@freeqaz

freeqaz commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Oct 20, 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. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed 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 20, 2026
With Share Agent on its own tab (#2999) and the details folded, the error
shows once, on the card, and the version detail can start in the lower half
of the first screen at 1440. The recovery link tests now require only that it
starts outside the upper half, which their in-view check needs.
@freeqaz

freeqaz commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Oct 20, 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. labels Oct 20, 2026
@freeqaz

freeqaz commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Oct 20, 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. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 20, 2026
A Refresh deployment that read a failure while its Agent or versions read
failed opened the details for the 'could not be refreshed' warning, and the
card's fold closed them at once. A panel showing that warning now stays open,
like one with a cleanup or startup warnings.
@freeqaz

freeqaz commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

Fixed the later P2 in d0503a3: a failed record read by a Refresh whose Agent or versions read fails left the "Current version and version history could not be refreshed" warning folded away.

  • The fold now also skips a panel showing that refresh warning, alongside cleanup and startup warnings. The panel's own evidence rule still opens the details for anything recorded later.
  • New test "A failure read by a refresh that cannot reread the Agent keeps its warning in view" (console-agent-deploy-again.test.mjs) checks the details are open and the warning visible before any click. It fails on b553fb9 and passes now (20/20 pinned). Related console browser files: 314/314.

@freeqaz

freeqaz commented Oct 20, 2026

Copy link
Copy Markdown
Collaborator Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Oct 20, 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. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 20, 2026

@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.

Verified the cleanup visibility fix I requested, and the partial-refresh warning fix, on d0503a3. Thank you.

In the browser, a watched deployment failed while the Agent overview read returned a controlled 503. On b553fb9 the warning was present but hidden; on this commit the details open and the warning is available without a click. I also checked that pending cleanup stays visible, an ordinary failure starts folded, and refreshing after I open the details respects that choice.

This resolves my specific objection in review 5581365985. These were controlled Console checks, not a live deployment or a substitute for the required human review. I would not hold this fix for a separate disclosure-state refactor.

@freeqaz-openai
freeqaz-openai dismissed their stale review October 20, 2026 07:17

Withdrawing my obsolete change request on 15f77ea. Free fixed it in d0503a3. I verified cleanup-pending and partial-refresh warnings remain visible. My exact-head verification is review 5581784575. This is not approval; human review of the current head is still required.

@freeqaz
freeqaz merged commit 75ebcc2 into main Oct 20, 2026
76 of 77 checks passed
@freeqaz
freeqaz deleted the fdr32-fail branch October 20, 2026 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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