Repository navigation
Conversation
…ead is late A first Deployment activity record that lands after the bounded reveal fills the folded one-row stand-in instead of opening; the summary note, stat values and the version record keep their rows. Browser asserts on the activity outcome go through one helper module (console-deployment-activity-helpers.mjs) that encodes the fold rule once. Finding 2163.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
…view - Set detailShown when the page actually shows (after any minimum loading time), so a record landing while the loading page is held still opens. - The summary note gives its line back when it is already empty as the page first shows; only a still-loading note keeps the reserved line. - Keep the loading row to one line on narrow phones. - Regression test: add a timely control case (a pending record read in time opens, the empty note takes no space) and ignore polls from a replaced page. - Escape the label in the summary match.
|
Codex review: needs maintainer review before merge. What this changesThis PR stabilizes Agent version controls when the first deployment-status response arrives after the page appears. Example: At 1440 px, an Agent draft’s first running-deployment response arrives after the 2-second reveal.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review Keep open: this PR addresses a layout shift still present on main, and the review found no blocking introduced defect. Priority: P2 Before mergeNone. FindingsNone. Agent review detailsHow this fits togetherThe controller-served Console reads authorized Agent and deployment records, then renders summary facts, deployment details, and version controls in the browser. flowchart TB
A[Agent details browser page] --> B[Controller resource APIs]
B --> C[Agent and deployment records]
C --> D[Summary outcome badge]
C --> E[Deployment disclosure]
D --> F[Version controls]
E --> F
Technical reviewBest possible solution: Keep the repair in the existing deployment panel and preserve explicit expansion for subsequent outcomes, warnings, cleanup, and recovery. Do we have a high-confidence way to reproduce the issue? Main’s loading-card replacement and initial disclosure expansion explain the reported movement, supported by the supplied before screenshots; no target code was executed during this review. Is this the best way to solve the issue? The patch repairs the existing presentation owner, reserves the affected rows, and distinguishes first arrival from later changes without changing API or deployment behavior. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against e5fc76148cf8. Provenance checked
TestingProof path: shipped entry point. SecurityNone. EvidenceWhat I checked:
Review metrics
LabelsLabel changes:
Label justifications:
Rating scale6/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. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 21, 2026, 9:30 PM ET / October 22, 2026, 01:30 UTC. |
Problem
When the Agent page's first Deployment activity read outlasts
FIRST_ACTIVITY_WAIT_MS(2 s), the page shows with a 128 px "Loading the most recent visible deployment…" card. The panel that replaces it has a different height, so everything below it moves after the page is already on screen (finding 2163). Measured with the read held past the wait (version tabs' top edge, 1440 / 390):Three smaller pieces add to it: the summary's "Reading recorded deployment status." note disappears (-20 px at 390), a stat value grows by 3 px when its loading text becomes a badge, and the version's deployment record grows from one loading line to two rows (+36 px).
The browser tests also wait for "Recorded status: X" in about 30 places, each with its own copy of the fold rule (two files even had the same
openFoldedDetailshelper). Changing the rule meant touching every one of them.Change
Product (
apps/controller/src/console/agents/detail.mjs,console.css):Tests:
tests/browser/console-deployment-activity-helpers.mjsholds the rule once:expectDeploymentOutcome(page, label, { folded }),openFailureDetails,openDeploymentActivity,deploymentDetailsOpen,expectVersionRecordanddeploymentRecordUrl. Withfoldedunset (a page's first record), a folded panel is accepted only when the summary shows the same outcome, and the helper then opens it.folded: true|falseasserts the state where the rule fixes it (every change after the first record, a routine completion, a failure under its card).openFoldedDetails(2 files) andnextFrames(2 files; now inconsole-agents-browser-helpers.mjs) are gone, as isconsole-agent-overview's localdeploymentUrl.a late first deployment read moves nothing above the version tabs(console-agent-overview): one page, fake clock, a control load (a pending record read in time still opens, and the empty note takes no space) plus 8 late loads (4 outcomes x draft/version view), measured at 1440 and 390. It replaces "Deployment activity appears once with its recorded status" (console-agent-state), whose stand-in checks it keeps. It fails on main.console-retained-revalidationwaited for.deployment-evidenceto mean "the panel replaced the placeholder"; the stand-in now has that class, so it waits for the panel'sdetails.Docs: flow (agent-editing + history), reference (versions-and-channel-drafts) and guide (agent-details) describe the late-first-record rule. Storybook: Pages/Agent detail / Deployment activity loading now uses an in-progress record and describes the still layout.
Verification
Measured after the change, with the read held past the wait (version tabs move, 1440 / 390):
The regression test asserts exactly that: the overview never changes height; the tabs move only by the card above and the viewed version's record; and they do not move at all for Completed and In progress (both views) or a read error on the draft.
Before / after, In progress on the draft at 1440, page shown with the read held, then after it lands:
Completed on the version view at 1440:
Video (In progress, draft, 1440; the read lands about 1.5 s after the page shows): before, after. Screenshots and video come from the real controller with a held, contract-shaped deployment record.
Storybook Pages/Agent detail / Deployment activity loading (simulated API, built by this PR's Console Storybook job; record reads take 4 s), page shown and then landed:
Storybook video, 1440.
Checks run locally:
eslint and prettier on the touched files: clean.
Touched browser files, one run each, main and this head (a2622f8) alternating file by file on a quiet host: all pass. Wall time:
console-agent-stateconsole-agent-overviewconsole-agent-detailconsole-agent-detail-2console-agentsconsole-repository-dedicatedconsole-agent-deploy-againconsole-retained-revalidationconsole-agent-overviewgains the new regression test (about 9 s: a control load plus eight late loads);console-agent-stateloses the test it replaces (about 3 s). The other files are unchanged within noise.Pin runs, 20 per touched browser file, each pinned to 2 CPUs, on a2622f8: 20/20 for all eight files (
console-agent-state,console-agent-overview,console-agent-detail,console-agent-detail-2,console-agents,console-repository-dedicated,console-agent-deploy-again,console-retained-revalidation). A firstconsole-agent-statepin round went 19/20 while the host's load average spiked past 500: that run took 1047 s instead of about 40 s and timed out inpage.gotobefore any changed code ran. A rerun once the load had settled went 20/20.Whole console browser suite (
node --test --test-concurrency=6 tests/browser/*.test.mjs) on a2622f8: 856 of 857 pass in 188 s. The one failure,docs-compute-matrix, is local only:github-sluggeris not installed in this checkout's docs-site tooling, and main fails the same way here. CI's Console Browser jobs pass it.New-conflict check against the 16 open PRs touching these files (
git merge-treeagainst main and against this head): no new conflicted files.Risks
expectDeploymentOutcomeopens a folded first record for the caller. Sites that assert a specific fold state passfolded; the regression test's control load pins the in-time case.