Skip to content

P0: main CI red — Deploy MCP failure on d58ca47 #3135

Description

@github-actions

P0: main went red

Workflow: Deploy MCP concluded failure on main.

Failing run: https://github.com/edobry/minsky/actions/runs/32206053915
HEAD SHA: d58ca47e6b80aeb9fd7fdd1e12185cbe06d33826
Head commit: fix(mt#4255): Retire a driven_sessions row whose actuator is verifiably gone

Summary

23 driven_sessions rows have sat non-terminal since late July, so the cockpit registers each one
as reconnecting at every daemon boot — the surface whose job is to say what is running is
confidently wrong, and got more wrong over three weeks (2 rows on 2026-07-25, 23 on 2026-08-18).

The diagnosis in the task spec is not the one it was filed with. The rows are not escaping a
sweep; they are outside the reach of the boot reconciler's two retirement tests by construction.

harness_session_id is non-null on all 23 and every cwd exists (21 are /Users/edobry/Projects/minsky,
which never disappears), so neither test can ever fire on any of them — no number of reboots would
have retired one.

The reframe that makes the fix small: driven_sessions.status carries two different facts.
spawned/running are claims about an actuator process; unrecoverable is a claim about the
conversation. The governing RFC ("Conversation-first drive", Accepted 2026-07-22) names those
layers and says they must be "rendered honestly". These rows claim running for processes that are
gone — a false actuator-layer fact. Writing exited makes it true, and asserts nothing about the
conversation.

Key changes

src/cockpit/process-identity.ts — a four-way probe beside the existing predicates.
verifyProcessIdentity returns false for every reason not to kill, which is right for the kill
path and unusable here: "the process is gone" and "ps could not answer" are the same false, so a
sweep built on it would retire the whole table the first time ps misbehaved. probeProcessIdentity
returns ours / not-ours / gone / unknown, establishing presence via kill(pid, 0) first
which is what makes the rest unambiguous, since a command line that then fails to read is a failure of
our probe rather than evidence about the process. The existing functions are untouched.

src/cockpit/driven-session-launch.ts — a third retirement test in the loop that already runs.
No new sweeper, no new scheduler, no new caller: boot reconciliation already iterates every
non-terminal row, already probes, and already writes verdicts back. A row whose verdict is gone or
not-ours is persisted exited and not registered, so the phantom disappears on the boot that
detects it. Fails open on a null pid, an unknown verdict, or a per-row timeout — a row is retired
only on a definitive answer, and (after R1) only once the write is confirmed to have landed.

Identity rather than bare liveness is load-bearing, not a nicety. Of the 23 rows, 22 pids are
dead and one is ALIVE — pid 1119, held by BeeperDesktop's ContactsServer, which inherited the
number over an 18-day gap. A kill(pid, 0) check alone calls that row live forever. The same
property makes the sanctioned dual-daemon dev loop safe: a second daemon finds the first's children
alive and matching, and leaves every row alone.

Retiring costs nothing in resumability, which is what makes this safe without a staleness policy.
orchestrateDrivenSessionResume reads the row by localId and refuses only on !harnessSessionId,
status === "unrecoverable", and a missing cwd — it never inspects exited/crashed. All three
callers consult it unconditionally; entity-thread-launch.ts's mt#4093 comment names "a row whose
terminal status excluded it from that read" as the case it fixes. So principal-channel-standing
retired here resumes on the next message exactly as before. unrecoverableReason is deliberately
left untouched: that column is the conversation-layer fact.

R1 — reviewer round 1 (BLOCKING, taken)

retired was incremented and registration skipped regardless of whether the exited write
actually persisted
. The write is best-effort and swallows its own error, so a timed-out or failed
upsert still produced "retired N" in the operator's one boot line for a row the database still held
non-terminal and would re-read on the very next boot.

persistBootTerminalVerdict now returns whether the write landed — non-throwing is a property of
the control flow, not a licence to withhold the outcome — and the loop counts the row and skips
registration only on a confirmed write. On failure it falls through to ordinary registration and the
stage lands in degraded, so the registry stays consistent with what is actually stored.

The reviewer also marked two criteria Not Met. SC10 (cockpit no longer lists retired rows) is now
verified at the API layer rather than argued — see Live verification. SC2 (reuse
verifyProcessIdentity) was a real deviation for the reason in Key changes; the spec criterion was
amended with that reason, in the same session, before the review landed.

Judgment call

This reverses the blocking finding of the 2026-08-18 gate run, which held that no signal in the
schema distinguishes a dead conversation from a long-idle resumable one, and that a sweep would
retire the principal's standing channel. Both halves are falsified above — by the identity pair, and
by reading all three resume call sites. The prior gap report is retained in the spec, marked RESOLVED
with a per-action discharge record.

Testing

Execution evidence:

$ bun test --preload ./tests/setup.ts --timeout=15000 src/cockpit/
 2030 pass
 0 fail
 4504 expect() calls
Ran 2030 tests across 101 files. [24.62s]

Acceptance tests, by the spec's own order — all now discharged:

  • AT1 dead pid → persisted exited, not registered, counted — AT1.
  • AT2 live actuator that still matches → untouched (the dual-daemon case) — AT2.
  • AT3 pid alive but reused by an unrelated process → retired — AT3.
  • AT4 no recorded pid → never probed, never retired — AT4.
  • AT5 probe throws / exceeds its bound → untouched, stage in degradedAT5a, AT5b.
  • AT6 second run over the same table retires nothing — AT6.
  • AT7 describeReconciliationOutcome renders the retired count — the two SC7 describer tests.
  • AT8 dry-run against the live table reports a count and mutates nothing — live run below.
  • AT9 the cockpit no longer lists a retired row — the two API-level tests below.

Beyond the spec's list: a mixed batch (3 rows, 1 retired) covering the count/retired arithmetic
that no single-row test can exercise; an ordering guard that a no-transcript row stays
unrecoverable rather than being downgraded to exited; and the two R1 failed-write cases.

Negative control: neutralized the retirement branch in reconcilePersistedDrivenSessions (if (false && ...)), reverting to pre-change behaviour, and observed 5 of the new tests fail.

(fail) ... > AT1 — a dead pid is persisted `exited`, not registered, and counted
(fail) ... > AT3 — a pid ALIVE but reused by an unrelated process is retired
(fail) ... > AT5b — a probe that exceeds its bound fails OPEN and names the stage
(fail) ... > AT6 — the second run over the same table retires nothing
(fail) ... > the write records a verdict, not a rewrite — and preserves its own evidence
 63 pass
 5 fail

Negative control — probe fail-open: collapsed unknown into gone in probeProcessIdentity, the exact defect the four-way return exists to prevent.

Expected: "unknown"
Received: "gone"
(fail) probeProcessIdentity > presence the kernel could not determine is `unknown`, not `gone`
 10 pass
 1 fail

Negative control — R1 fix: restored the pre-R1 unconditional retired += 1 and observed the three tests that guard it fail, including the API-level one.

(fail) ... > R1 — a persist that FAILS is not counted retired, and the row is registered
(fail) ... > R1 — a persist that TIMES OUT is not counted retired either
(fail) GET /api/driven-session ... > a row whose retirement write FAILED is still listed
 98 pass
 3 fail

All three controls were reverted before committing.

Live verification

scripts/verify-actuator-gone-retirement.ts — dual-mode, and both branches were exercised, since
a passing dry-run is no evidence about the branch that writes.

Dry run (writes nothing), against the live table — AT8:

Read 23 non-terminal row(s) — the set boot reconciliation loads.
  RETIRE   principal-channel-standing  (status=spawned, pid=50233, verdict=gone)
  RETIRE   telegram-topic:167346572:749709  (status=running, pid=1119, verdict=not-ours)
  ... (21 more)
Would retire 23 of 23; 0 stay registered, 0 skipped for want of a pid.

22 gone, 1 not-ours — the BeeperDesktop pid-reuse row, caught by the identity check exactly as
designed. Scope-match: 23 predicted, 23 proposed, 1:1.

Seeded write probe (--seed-probe). Real database, real probe, real write, with the row list
narrowed to one throwaway row so the write binding is exercised without touching any real row:

[2/4] reconciling — real db, real probe, real write; row list narrowed to it
info: [driven-session] boot reconciliation: persisted actuator-gone verdict (no process at recorded pid 1073741824) for mt4255-actuator-probe-... (it will no longer be re-read at boot)
      outcome={"kind":"loaded","count":0,"retired":1,"degraded":[]}
[3/4] re-reading the row and the boot query
      status=exited, appears in next boot's read=false, registered=false
PASS: a dead-actuator row is persisted `exited`, drops out of the boot query,
is not registered, and every other column survived the write.

Confirmed self-contained afterwards by re-reading the table: 23 non-terminal rows still present and
untouched, 0 leftover probe rows.

AT9 at the API layer (R1). src/cockpit/routes/driven-sessions.test.ts mounts the real Express
route over a real HTTP server, runs reconciliation across a retired row and a live one, and asserts
GET /api/driven-session omits the retired id while still listing the live one as reconnecting.
A second case asserts a row whose retirement write FAILED is still listed — the surface-level
companion to the R1 fix. The first pass offered a code-path argument here instead; the reviewer
correctly called that Not Met.

Deploy verification

Correcting an earlier claim in this body: src/cockpit/** IS deploy surface. A prior revision
said no deploy-surface path was touched, reasoning that the cockpit daemon runs locally under the
tray. That is only half true — the same source is also built into the deployed cockpit image, so all
five changed files are deploy-surface and the merge gate was right to say so.

Committing to the post-merge check: after merge I will run
mcp__minsky__deployment_wait-for-latest with notBefore set to this PR's merge timestamp, require
SUCCESS, and confirm the runtime actually started rather than treating the deploy call's exit as the
outcome. A tool or auth flake is a blocker to reconnect and retry, not a licence to defer.

Known external condition at merge time. Railway's upload endpoint has been returning
500 Internal Server Error at railway up since ~00:57Z, failing every Cockpit Preview run across
every branch in the repo (mt-4274, mt-4250, mt-4275, and this one) — the red deploy check here.
It fails before the workflow reaches any of our code and is unrelated to this change; four other PRs
have merged through the same window tonight. If the post-merge deploy is still blocked by that
outage, the verification stays open and is reported as such — not recorded as passed.

Usability

The rows are retired by whichever cockpit daemon boots this code. For the local tray daemon this is a
build/install deliverable, not an auto-usable one: merging does not retire the 23 rows. They are
retired at the first cockpit daemon restart running this code.

Spec, the reproduced diagnosis, and the full gate record: mt#4255.

Co-Authored-By: minsky-ai[bot] <minsky-ai[bot]@users.noreply.github.com>

What this means

A push to main triggered CI and the workflow above did not conclude success. Per
CLAUDE.md user preference ("main must never be broken"), this is severity-1.

Diagnostic checklist

  1. Open the failing run URL above; identify which job/step failed.
  2. Check whether the offending PR was merged with a known-failing required check
    (operator-API bypass via gh api PUT /merge despite enforce_admins).
  3. Confirm enforce_admins is currently enabled:
    gh api repos/edobry/minsky/branches/main/protection --jq .enforce_admins.enabled
    
    Expected: true post-mt#1938. If false, that is itself a separate finding.

Recovery

  1. Open a hotfix branch off current main.
  2. Apply the smallest fix that turns CI green (often a formatter pass or a config
    flip).
  3. Land via the standard Minsky session flow:
    tasks_create → session_start → session_commit → session_pr_create → /review-pr → session_pr_merge.
  4. Verify the post-merge main build is green within ~5 minutes.
  5. Close this issue with a link to the hotfix PR.

Cross-references

Metadata

Metadata

Assignees

No one assigned

    Labels

    main-redAuto-filed by .github/workflows/main-watch.yml when main CI fails (mt#1938)p0Severity 0: production breakage requiring immediate attention

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions