Skip to content

fix(console): mark an earlier failed version from the revision list after a reload - #3598

Merged
freeqaz merged 9 commits into
mainfrom
fix/d703-list-deployment-outcomes
Oct 25, 2026
Merged

freeqaz merged 9 commits into
mainfrom
fix/d703-list-deployment-outcomes

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

After a reload, the Versions list showed an earlier failed version (v1) without its Failed mark until someone opened it. #3577 marks failed versions from deployment records, but the page reads a record only for the selected version and for versions opened since the page loaded. The revision list carried no outcome.

Change

  • Each item in GET /v1/namespaces/{id}/agents/{agentId}/revisions now carries deploymentStatus (queued, running, succeeded, failed, or null when the version has no deployment work) and deploymentErrorCode (the error.code the deployment record reports, or null). OCC reads both from the version's reconcile work, mapped exactly as the deployment record maps them.
  • The console marks a listed version whose deploymentStatus is failed, unless neutralDeploymentOutcome (list.mts) says a newer version or a Stop ended it (REVISION_SUPERSEDED, REPOSITORY_REVISION_SUPERSEDED, REVISION_STOPPED). So the mark survives a reload with no extra requests, and superseded or stopped versions stay unmarked. The record-based marks from fix(console): mark an earlier failed version after a reload and label the draft read-only for readers #3577 still apply as versions are opened or deployed.
  • Why the code and not a server-side "neutral" status: the deployment record already reports these endings as failed with a code, and the console classifies them in one place. Listing the same status and code keeps list.mts the only source of truth for neutral endings, for both the record and the list.
  • Docs: regenerated API reference and OpenAPI, a sentence in the agent-editing flow, and an Pages → Agent detail → EarlierFailedVersion story. The Storybook list fixture carries the code too, so "Deployment superseded by a newer version" reads as a historical version again.

Trade-off: the list read now does one primary-key work lookup per listed version (at most a page of 100) on the server. The alternative was a deployment-record request per version from the browser on every load, which costs more round trips and more authorization checks.

Verification

  • tests/browser/console-agent-overview.test.mjs: the fix(console): mark an earlier failed version after a reload and label the draft read-only for readers #3577 reload test expects "View version v1, deployment failed" in the Versions list before v1 is opened, with no read of v1's record. A new test lists a version as failed with REVISION_SUPERSEDED and with REVISION_STOPPED and expects it unmarked after a load, reading Superseded or Stopped and "Historical version" when opened. On the previous head the new test fails ("View version v1, deployment failed"). Pinned 20/20 at 2 CPUs on a remote test VM.
  • tests/integration/postgres-agent-provisioning.test.mjs: a first deployment interrupted by a real Stop lists as failed with REVISION_STOPPED.
  • tests/integration/postgres-worker-agent-revision.test.mjs: an older revision the worker really superseded lists as failed with REVISION_SUPERSEDED, the newer one as succeeded with no code.
  • list-summaries, occ-api, configuration-occ and the PostgreSQL platform-state tests expect both fields.

Risks

  • Additive response fields; clients that ignore unknown fields see no change. The summary schema still lists exactly the fields it returns.
  • One extra indexed read per listed version.
  • occ agent revisions (CLI) still fills deploymentStatus from each record; deploymentErrorCode passes through from the list.

…fter a reload

The Versions list read only the latest version's deployment record, so after a
reload an earlier failed version had no mark until it was opened. Revision
list summaries now carry deploymentStatus, read from each listed revision's
deployment work in the same read, and the console marks failed versions from
it. Finding 2422 (D703).
@freeqaz
freeqaz requested a review from a team as a code owner October 25, 2026 07:41
@clawsweeper

clawsweeper Bot commented Oct 25, 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. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. 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. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 25, 2026
@clawsweeper

clawsweeper Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge.

What this changes

The PR adds deployment status to revision summaries so the Console can mark earlier failed versions immediately after a reload.

Example: Version v1 failed its model check, and v2 subsequently deployed successfully.

  • Before: After reloading, v1 has no Failed mark until its deployment record is opened.
  • After: The revision list supplies v1's status, allowing its Failed mark to appear before opening it.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) Useful and focused, but the neutral-outcome regression blocks correctness and the repository-required visual proof is absent; no persisted model or migration changes are introduced.
Proof confidence 🧂 unranked krab (1/6) Real behavior proof is not required for maintainer- or bot-authored pull requests.
Patch quality 🦪 silver shellfish (2/6) 1 actionable review finding remain.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Earlier failed versions look like ordinary history after reloading until the reader opens each one.
Reason: The maintainer-authored direction completes an explicitly identified gap using the existing revision-list API; its bounded additional work lookups are a recorded tradeoff.

Merge readiness

⛔ Needs changes before merge - 2 items remain

Keep this PR open: it addresses a real reload gap, but introduces incorrect Failed labels for stopped and superseded versions.

Priority: P2
Reviewed head: 31ac46d749fb168891342c7886ad7136180fe35f

Before merge

  • Preserve neutral outcomes when marking listed versions (P2) - When v1 is stopped during deployment or superseded before activation, controllerWorkDeploymentStatus reports failed. This new condition permanently adds v1 to failedVersions without the reason-code check used by deploymentOutcome. Once v2 is the latest version, v1 therefore gets a red Failed badge and opening it shows Failed version even while its record correctly says Stopped or Superseded. Reading that record cannot remove the cached mark. Carry enough outcome information in the summary to apply the existing classification, and cover stopped and superseded earlier versions alongside a genuine failure.
  • Explain or restore the original intent of Failure-mark classification - fix(console): mark an earlier failed version after a reload and label the draft read-only for readers #3577: Populate failure marks using deploymentOutcome so intentionally stopped and superseded deployments remain neutral.

Findings

  • [P2] Preserve neutral outcomes when marking listed versions — apps/controller/src/console/agents/detail.mts:534-536

Provenance

Tests

  • Missing end-to-end proof: Supply the required before/after screenshots and walkthrough showing an earlier genuinely failed version marked immediately after reload; the body reports a base-failing browser test, but its listing outcome is intercepted.
Agent review details

How this fits together

OCC authorizes revision-list reads, joins each readable revision to its controller work, and returns summaries through the HTTP API to the Console's Versions list.

flowchart TB
  Browser[Console reload] --> API[Revision list API]
  API --> OCC[OCC authorization and listing]
  OCC --> Revisions[Revision summaries]
  OCC --> Work[Deployment controller work]
  Revisions --> Result[Summaries with deployment status]
  Work --> Result
  Result --> Marks[Console version marks]
Loading

Technical review

Best possible solution:

Return enough outcome information in revision summaries to reuse the existing neutral-outcome classification before adding failure marks.

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

Current main marks earlier failures only after reading their records; the introduced list loop also demonstrably classifies stopped and superseded work as failed. This review used source inspection and ran no target tests.

Is this the best way to solve the issue?

Adding outcome information to the existing paged summary is a focused solution, but raw deployment status loses the reason codes required by the Console's existing classification.

Full review comments:

  • [P2] Preserve neutral outcomes when marking listed versions — apps/controller/src/console/agents/detail.mts:534-536
    When v1 is stopped during deployment or superseded before activation, controllerWorkDeploymentStatus reports failed. This new condition permanently adds v1 to failedVersions without the reason-code check used by deploymentOutcome. Once v2 is the latest version, v1 therefore gets a red Failed badge and opening it shows Failed version even while its record correctly says Stopped or Superseded. Reading that record cannot remove the cached mark. Carry enough outcome information in the summary to apply the existing classification, and cover stopped and superseded earlier versions alongside a genuine failure.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

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

Provenance checked

  • Revision summary shape and pagination keeps the original intent (2a55afc: Keep lists small without decoding saved snapshots, retain revision history, and filter each item through exact-resource read authorization.)
  • Agent-editing flow wording keeps the original intent (5227270: Distinguish recorded deployment evidence from live serving and preserve polling, recovery permissions, and delayed-read behavior.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

Acceptance criteria:

  • node --test tests/integration/list-summaries.test.mjs
  • node --test tests/integration/postgres-agent-provisioning.test.mjs
  • pnpm test:files --test-reporter=spec -- tests/browser/console-agent-overview.test.mjs
  • pnpm openapi:check
  • pnpm lint

What I checked:

Labels

Label changes:

  • add P2: The change addresses version-history confusion with limited blast radius, but must preserve intentional stopped and superseded outcomes.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Label justifications:

  • P2: The change addresses version-history confusion with limited blast radius, but must preserve intentional stopped and superseded outcomes.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

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.

Reviewed October 25, 2026, 4:14 AM ET / 08:14 UTC.

…reload

The revision list carried only the raw deployment status, so a version that
a newer one or a Stop ended (status failed, codes REVISION_SUPERSEDED,
REPOSITORY_REVISION_SUPERSEDED, REVISION_STOPPED) got the Failed mark on a
new page load. List items now carry deploymentErrorCode, the error.code the
deployment record reports, and the console applies neutralDeploymentOutcome
before marking, so list.mts stays the one place that classifies neutral
endings for both the record and the list.

PostgreSQL tests now list real stopped and superseded work with its code; a
browser test proves both codes stay unmarked after a load (fails on the
previous head). The Storybook list fixture carries the code too, so the
superseded scenario reads Historical again.
@freeqaz
freeqaz merged commit b8f02ca into main Oct 25, 2026
43 checks passed
@freeqaz
freeqaz deleted the fix/d703-list-deployment-outcomes branch October 25, 2026 08:36
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: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant