Skip to content

fix(cli): fence legacy automation runtime cutovers - #4579

Closed
yannickmonney wants to merge 1 commit into
mainfrom
fix/automation-cutover-barrier
Closed

yannickmonney wants to merge 1 commit into
mainfrom
fix/automation-cutover-barrier

Conversation

@yannickmonney

Copy link
Copy Markdown
Contributor

A managed protocol-1 upgrade can admit new legacy automation work after an empty census and before its backend containers are replaced. The CLI now holds a bounded PostgreSQL admission lock while verifying an all-organization zero census and gracefully stopping the exact legacy writers. A durable handoff supports verified partial replacement recovery; lost locks, uncertain stops and unknown identities refuse startup. Existing tag deployments must take this managed transition first.

The initial supported legacy source is b4931db4 with its complete migration inventory and bundled database. Existing protocol fences remain intact; the guard does not infer retirement of historical external effects.

Validation: 189 CLI, consumer and CI scope/cache tests; real psql lock and connection-loss tests; real OS-pipe backpressure/output-bound tests; 12 real-Postgres assertions in the automatically required Backend integration helper; causal lock-removal and old-runtime negative controls; CLI and platform typechecks, lint, formatting and manual coverage checks. The full released-container cutover remains release acceptance work.

@yannickmonney

Copy link
Copy Markdown
Contributor Author

REQUEST CHANGES — reviewed head deb1b6ae78cf65b1428168cc4af4059c8456700a (base c986f99db9406abf20fd7f4e90069afcfad6941f). I did not write this implementation. Independent API-only review, both parts; no local tests or deployment execution.

  1. [P2] Explain an inconclusive legacy stop and its recovery action — tools/cli/src/lib/deployment/automation-cutover.ts:327. If Docker throws, times out, or returns failure after stopping only some writers, this new call propagates runtimeCommand's generic “Docker could not complete/refused the managed runtime operation” (runtime-command.ts:82, :87). It omits that this was the automation cutover's backend stop, that shutdown may be partial and the runtime remains pending, and that the operator must retain the bundle/receipt and retry the same managed deployment without manually restarting writers. The successful-stop/lost-lock branches explain this, but the actual stop-failure branch does not; the test at automation-cutover.test.ts:323 currently asserts only “could not complete.” Add a redacted cutover-specific failure boundary (and a progress message before the potentially 60-second stop), with regression assertions for both a thrown executor error and unsuccessful result after a partial stop. Preserve the underlying fail-closed behavior and never expose Docker output.

Part A: The README and manual automation register accurately describe supported source, all-org census, pending recovery, immutable identity checks, forward-only protocol and excluded historical external effects. They also correctly separate release acceptance from synthetic/SQL proof. Interrupted legacy retry takes a fresh locked census; partial replacement requires a stopped receipt, exact target images, complete roles and unchanged database incarnation. The finding above is the operator-message gap.

Part B: Source review found no additional blocking correctness defect. SHARE/NOWAIT precedes READ COMMITTED census; competing INSERT/UPDATE cannot pass the held table lock, prior uncommitted writers refuse acquisition, and lock probes plus exact exited-container readback precede release. SQL uses fixed statements and catalog-quoted ledger identifiers; no user input is executable SQL. Receipt persistence excludes connection credentials and unrelated labels. Boot migrations precede API/worker startup; protocol/image floors retain forward-only rollback refusal. Existing tag upgrades with a protocol-1 floor refuse the unmanaged transition. External deployment serialization remains required as documented.

Regression/CI scope: Removing the SHARE lock makes the shared real-PG helper's competing-admission assertion fail; removing the managed cutover call makes the new unsupported-writer apply regression admit startup instead of refusing. Census/identity/partial-replacement tests distinguish the safety decisions, and real-process transport tests cover backpressure, bounded output and private errors. These are source-level causal assessments, NOT executed mutation proof in this review. .github/ci-scope.yml includes the new shared SQL module; platform test/lint/typecheck inputs and their guard include it. integration-check.ts:60887 invokes the modified helper and Checks runs Backend integration directly with ITEST_REQUIRE_ALL_LANES=1. Unit (workspaces) selects CLI bun test; the physical Docker/psql test is deliberately skipped without TALE_CUTOVER_PG_IMAGE, so ordinary green CI alone will not prove complete released-container shutdown/cutover.

Per-topic dispositions: UX-CLARITY FAIL (1); DOCS PASS (source consistency); REL-MANUAL NOT_RUN (physical/manual journey); CODE PASS (source review, proof pending); SECURITY PASS (source boundaries, completed audit/SAST); DATA BLOCKED (required real-PG check pending); DEPLOYMENT BLOCKED (1 and full deployed cutover NOT_RUN); RECOVERY FAIL (1; safety source checks otherwise sound); CICD BLOCKED (execution pending, selection/cache wiring passes source review); WORKFLOW PASS (independence, scope and forward-only authority preserved; no serving-adoption claim).

Exact-head checks, 2026-10-08 19:09:28 UTC: 65 checks: 36 queued, Backend integration in progress, 28 completed; no failure/timed-out/cancelled/action-required conclusion. Completed results include Integration scope, Build db, Bun audit, Trivy filesystem scan, Opengrep and CI ready (Commitlint) success; Trivy is neutral and candidate/fork-only checks are skipped. Pending: CLI builds (linux, linux-arm64, macos, macos-x64, windows); CI ready (SAST/Security); Type check, Performance, Knip, Lint, Format, Build, Browser; Unit workspaces and platform 1–2; UI platform 1–4; E2E preview build and Playwright docs/web; platform/proxy/sandbox/runtime/egress/buildkit/gateway image builds, Storybook, web/docs/UI-docs/AI-gateway container tests. No failing job exists in this snapshot for ##[error]/FAIL attribution; pending checks cannot be classified as a PR defect, stale-base failure or flake without a failure diagnostic. GitHub reports behind; fresh base/check proof remains required before merge. No reruns/cancels requested.

Held #4540 overlap (contracts only): project-agent completion/slot release, automation/standing-role admission, task-subject exclusion and transaction/lock order. TALE-1290 was read; pulls/4540/files?per_page=100 --paginate --slurp returned HTTP 404, so no direct changed-file intersection is asserted and the held patch was not judged.

Next owner: implementer fixes (1); Fleet coordination retains CI/base and release-acceptance follow-up. This comment does not authorize merge or deployment.

@yannickmonney

Copy link
Copy Markdown
Contributor Author

The cutover implementation, including the subsequent reviewed migration-ledger and interrupted-recovery corrections, is included in #4565, merged as 0ee5fd9656dc12f08976155d956ee76fe39f4ea3. Closing this superseded component PR. Actual release and deployment acceptance remain separate.

@yannickmonney
yannickmonney deleted the fix/automation-cutover-barrier branch October 10, 2026 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant