Skip to content

P0: main CI red — Deploy MCP failure on 1743a51 #3122

Description

@github-actions

P0: main went red

Workflow: Deploy MCP concluded failure on main.

Failing run: https://github.com/edobry/minsky/actions/runs/32204282110
HEAD SHA: 1743a51e0f318deaa5782a21507646c732282431
Head commit: fix(mt#4275): Bound the derived timestamp, not another etime field

Summary

mt#4260 shipped three hours ago with a range check on parseElapsedSeconds, and its comment

Note this does NOT fix the underlying flake — it converts an absurd value into a legible null so
the next occurrence is diagnosable.

It does not convert this one. The next occurrence arrived on the next CI run with the identical

(fail) parseElapsedSeconds — ps -o etime= > a live process yields a plausible start time, and a dead pid yields null
      expect(ageMs).toBeLessThan(60_000);
Expected: < 60000
Received: 38109073018720000

An age of ~1.2 million years for a sleep spawned microseconds earlier — byte-identical to the
value that motivated mt#4260.

Why the guard missed it

if (h > 23 || m > 59 || s > 59) return null;
return Number(days ?? 0) * 86_400 + h * 3_600 + m * 60 + s;

Hours, minutes and seconds are bounded. Days is not — deliberately, and the comment says why:
"Days is genuinely unbounded — a process can run for years." That is right about ps and wrong
about this consumer.

Decomposed, the reading was 441077234-00:18:40: 441 million days, with every bounded field in
range. And note the 18:40 — mt#4260's originating occurrence recorded 10585853616:18:40, parsed
then as HOURS. 10,585,853,616 hours is exactly 441,077,234 days. One reading, two capture
groups, and only the group seen first was bounded. A field-by-field guard has as many holes as it
has unbounded fields.

The fix: bound the quantity that actually has a ceiling

parseElapsedSeconds is unchanged — it still reports what the string denotes. A new pure function
converts the reading to an absolute start time and rejects it if that time falls before the UNIX

export function startedAtMsFromElapsed(raw: string, nowMs: number): number | null {
  const elapsedSec = parseElapsedSeconds(raw);
  if (elapsedSec === null) return null;
  const startedAtMs = nowMs - elapsedSec * 1000;
  if (startedAtMs < 0) return null;   // before the epoch is impossible, not merely large
  return startedAtMs;
}

A process cannot have started before the UNIX epoch. That is not a chosen constant — no
threshold to justify per decision-defaults §Thresholds, and nothing to revisit. It rejects 441
million days and still admits a genuinely decade-old process.

nowMs is a parameter rather than a Date.now() call, so the bound is testable without depending
on the host's ps — which is the thing under suspicion. Judging possibility belongs to the
consumer because only the consumer knows the value becomes a timestamp (functional core, imperative
shell).

The test was asserting against the fix

port-incumbent.test.ts:190 was expect(startedAt).not.toBeNull() — which contradicts the code
it guards
. On a host whose ps emits 441 million days, null is the CORRECT answer, and the test
was failing the probe for behaving properly. It now asserts the disjunction the probe actually
promises: a plausible timestamp OR an explicit null.

The end-to-end shell-out is kept — its own comment calls it "the exact check that caught the
etimes bug"
— and it still catches a fabricated age: if the probe returns a number, that
number must be plausible. The original failure was exactly that, and would still be caught.

Testing

Execution evidence:

$ bun test --preload ./tests/setup.ts --timeout=15000 src/cockpit/port-incumbent.test.ts
 52 pass
 0 fail
 63 expect() calls
Ran 52 tests across 1 file. [115.00ms]

$ bun test --preload ./tests/setup.ts --timeout=15000 src/cockpit/
 2003 pass
 0 fail
Ran 2003 tests across 100 files. [24.47s]

Six tests added. AT1 pins that the parse stays faithful
(parseElapsedSeconds("441077234-00:18:40")38109073018720) alongside the rendering mt#4260 DID
bound ("10585853616:18:40" → null) — asserting both is what records the two occurrences as one
reading. AT2 is the epoch rejection. AT4 is the long-lived-process control (3650-00:00:00, ~10
years, still a real timestamp), which is what distinguishes "rejects impossible" from "rejects
large". Plus the exact epoch boundary, an ordinary recent reading, and unparseable input.

Negative control — epoch bound commented out:

Received: -38107285921120000
(fail) startedAtMsFromElapsed — the epoch bound (mt#4275) > rejects a reading that would place the start before the UNIX epoch
 51 pass
 1 fail

A negative timestamp — which is precisely what the caller then rendered as a 1.2-million-year
age. Restored: 52 pass / 0 fail.

Typecheck: 0 errors across 8 projects. Lint: 0 errors, 0 warnings, 3781 files.

What this does NOT fix

Why the runner's ps reports 441 million days for a just-spawned process. Still unexplained,
and explicitly out of scope — the bound is correct whatever the cause, since a pre-epoch start is
wrong under every explanation. The isolation control run locally (5/5 pass) is recorded in the task
spec as weak evidence: it ran on macOS while the failure occurs on Linux runners, so it does not
sample the failing population at all. A Linux-side reproduction is what would turn "intermittent"
into a cause.

With this merged, such a reading becomes a null the caller already handles rather than a fabricated
age downstream logic treats as real — which is what mt#4260 intended and did not achieve.

Deploy verification

[no-deploy-impact]src/cockpit/port-recovery.ts is the local cockpit-daemon port-recovery
path, exercised when a daemon starts and finds a port occupied. It is not on the hosted service's
request path, and this PR changes no deploy config.

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