Files
stack/agents/rocko/work/queue-56/BUILD.md
T

13 KiB
Raw Blame History

Row 56 (#1545): runs and releases reader module, candidate packet, round 2

Author: Rocko. Reviewers: Darkwing and Filbert. Brief: docs/plans/2026-10-10_design-implementation.md, section "Runs and releases reader module". Base: 75ab1646 (base.txt), the origin/refactor Sage pushed with the round 1 review records. Round 1 was candidate ed3c5392 over dc96f87b. No file in this candidate changed upstream between the two bases. The packet is uncommitted. I made no commits, pushes or Gitea calls, and read no token or private binding.

The round 2 section comes first. The round 1 text follows and still holds except where round 2 corrects it; the corrections are marked in place.

Round 2 changes (against ed3c5392)

Round 1 was approved by Darkwing (#1545 comment 27115, rev 348) and Filbert (comment 27116). Sage ruled round 2 in: tests and wording only. No src/ logic changes. scripts/mosaic-task.mjs is byte-identical to round 1. out/round2-delta.diff is the exact diff of the six files that changed:

File Change Answers
packages/runs/tests/task-cli.test.mjs +2 tests: show ../x with a missing config exits 4 "invalid run id", not 3; list shows r-.bad as before Darkwing 1 (M31), Darkwing 2
packages/runs/tests/runs.test.mjs +1 test: listRunIds and listRunRecords on r-.bad give ["r-.bad"] and [{ runId: "r-.bad", result: {} }], the same as base Darkwing 2 (D17, D20)
packages/runs/tests/state.test.mjs malformed log lines gain an entry with no imageTag and { ...good[1], release: 5 }, so malformed goes 5 → 7; +1 test: a state/ link out to nothing reads as no release and an empty log Darkwing 3 (D22), Filbert N2, Darkwing 4a
packages/runs/src/state.mjs comment above readActivationLog only: says what is skipped and that rollback skips only lines that aren't JSON Filbert N1, Darkwing 4b
packages/runs/README.md links with nothing behind them read as missing, state/ included; the activation log bullet says what the module skips against rollback; a delta row for show on a mode-000 run directory Darkwing 4a, 4b, 4c; Filbert N1
agents/rocko/work/queue-56/delta-check.sh + the case "run directory unreadable: show" Darkwing 4c

Rollback's actual rule, from scripts/release.sh lines 91–99: it drops empty lines, skips lines that JSON.parse rejects, and acts on any parsed activate or rollback entry with a truthy imageTag. The module keeps its strict entry shape, so a line rollback would use can count as malformed here.

Round 2 evidence (scratch worktree of the round 2 candidate at 75ab1646)

  • packages/runs: 41/41 (out/runs-tests.log).
  • test-task: 98 passed, 0 failed, with Docker (out/gate/test-task.log). The other suites aren't rerun, because round 2 changes only package tests, comments, docs and the check script; round 1's results for them stand.
  • byte-check against a base worktree at 75ab1646: 16 cases, stdout, stderr and exit identical; data root unchanged (out/byte-check.txt).
  • delta-check: 13 cases now (out/delta-check.txt). The new one: base show printed run: and result.json: (missing or unreadable) and then crashed in readdirSync, exit 1; the candidate refuses with cannot read …: EACCES, exit 4.
  • mutants: 37 (out/mutants.sh, out/mutants.txt). Round 1's 32, plus Darkwing's D17, D20 and D22, Filbert's N2, and S1 (the dangling state/ link reading as missing). 36 are killed. Only M04 survives, and it is equivalent (below). Each new kill comes from an assertion in the new tests, not a syntax or reference error.
  • build.patch: git diff --cached --binary 75ab1646, 15 files, +1068/−27. It applies to 75ab1646 and the manifest matches (out/apply-75ab1646.txt).

Correction: M31 was never equivalent

Round 1 called M31 equivalent: "readRunRecord throws the same message". That is wrong. The isRunId check in showRun is the only id check that runs before loadConfig. Without it, show ../x with a missing config exits 3 with a configuration problem instead of 4 "invalid run id" (Darkwing's probe P1). The round 1 paragraph even said the check stays "so that an invalid id refuses before the config loads", which contradicted its own verdict. No test covered that case. The new CLI test does, and M31 is killed.

Round 1 (candidate ed3c5392 over dc96f87b)

Round 1 evidence is under out/round1/. Paths below that start out/ mean out/round1/ in this packet. The round 1 packet files are in r1/, unchanged, so the posted hashes still check: r1/build.patch dd7b469b…, r1/candidate-manifest.sha256 ed3c5392… and r1/packet-manifest.sha256 e0497ad1…. That manifest lists the round 1 paths, so it checks against the round 1 tree, not this directory.

Files (files.txt, 15)

File Change
packages/runs/package.json new: @mosaic/runs, private, no dependencies
packages/runs/src/errors.mjs new: RunsError(message, exitCode = 4)
packages/runs/src/paths.mjs new: resolveInside containment, readJsonObject
packages/runs/src/runs.mjs new: isRunId, listRunIds, listRunRecords, readRunDocument, readRunRecord
packages/runs/src/state.mjs new: readActivePointer, readActivationLog
packages/runs/src/index.mjs new: re-exports
packages/runs/README.md new: what it reads, the limits, the deliberate deltas
packages/runs/tests/*.mjs new: 37 tests in round 1 (runs 22, state 10, task-cli 5) plus helpers; 41 in round 2 (runs 23, state 11, task-cli 7)
scripts/mosaic-task.mjs list and show read through the module, +18/−27
agents/rocko/work/queue-56/{seed,byte-check,delta-check}.sh the byte-identity and delta checks

build.patch is git diff --cached --binary dc96f87b over those files: +1021/−27 in round 1. Round 2's build.patch is over 75ab1646, +1068/−27. candidate-manifest.sha256 hashes the 15 files.

Q14 freeze: nothing under packages/bus, packages/tasks, packages/cli, scripts/bus-service.sh or scripts/mosaic changes. Apart from the import, scripts/mosaic-task.mjs is the only file under scripts/ that changes. The module covers the runs view's needs, but the view itself, release.sh and pruning are untouched.

Design

  • Containment. resolveInside runs realpathSync on the data root and the target, then applies a path.relative check:
    • .., ../… or an absolute relative path counts as outside;
    • the configured data root is trusted even when it is itself a link;
    • links that stay inside it are followed.
  • Outside paths.
    • runs/, state/, a run directory read by readRunRecord, active.json and the activation log refuse when they resolve outside.
    • A run document that resolves outside reads as null, the way an unreadable one always has.
  • Missing versus broken. ENOENT and ENOTDIR read as missing. Anything else (ELOOP, EACCES) refuses with the error code instead of reading as empty.
  • Documents. A run document is a JSON object or null. Run records are not validated further, because older records carry older shapes.
  • Release pointer. active.json is strict: exactly the version 1 shape release.sh writes. Bad data exits 2, and a read error exits 4.
  • Activation log. The log is lenient per line, like release.sh rollback. It returns { entries, malformed }, and last keeps the newest entries. Corrected in round 2: not "like rollback". Rollback skips only lines that aren't JSON; this module also counts wrong-shaped entries.
  • Errors. Every refusal is a RunsError with exit code 2 or 4. mosaic-task.mjs maps it to fail(exitCode, message) through readRuns.
  • Read-only. No function writes. The "readers write nothing" test and byte-check's before/after snapshot of the data root both check this.

Gate (out/gate/, sequential, scratch worktree of the candidate)

Suite Result
packages/runs tests 34/34 at gate time; 37/37 after the mutant round (out/runs-tests-final.log)
test-auth 15 passed, 0 failed
test-conductor 17 passed, 0 failed
test-config 24 passed, 0 failed
test-discord first run 57 passed, 1 failed; rerun 66 passed, 0 failed (below)
test-extension-package 18 passed, 0 failed
test-foundation 44 passed, 0 failed
test-queue 27 passed, 0 failed
test-release 14 passed, 0 failed
test-task 98 passed, 0 failed

test-discord's first-run failure was environmental. The failing case was "pi binary present at node_modules/.bin/pi for the extension checks". The scratch worktree has no node_modules, and the base worktree lacks it too. I symlinked the checkout's node_modules into the scratch tree and reran the suite: 66 passed, 0 failed. The extra 8 cases are the extension checks that the missing binary had skipped. Then I removed the symlink (out/gate/test-discord-r2.log). Nothing in this candidate touches packages/discord. Sage's rerun in a tree that has node_modules should see 66.

After the gate, the only changes were tests, the README table row and the check scripts. No src/ or scripts/mosaic-task.mjs change came after it.

Byte identity (out/byte-check.txt)

byte-check.sh BASE CAND WORK seeds a data root with seed.sh. The seed holds:

  • runs a–f;
  • a run that is a file;
  • an internal link and a dangling link;
  • r-zz.weird_name-1;
  • a non-r- name, .pruned.log and a stray file.

It runs 16 cases from both trees: list on seeded, empty and absent roots, and show on each run, a missing run, ../escape, no argument and an absent root. It diffs stdout, stderr and exit codes.

byte-check: 16 cases, stdout, stderr and exit identical
byte-check: data root unchanged

Correction to my own harness. My first rerun in this round passed a relative WORK_DIR. Every redirect inside the cd subshell failed, both trees recorded exit 1, and the diff still reported "identical": a false pass. Earlier runs used absolute paths and were not affected. Both scripts now make WORK_DIR absolute before doing anything else. byte-check.sh also exits 4 if any case leaves no stdout or stderr file. The output above comes from the fixed script.

Deliberate deltas (out/delta-check.txt)

These cases fall outside the byte-identity domain: a link out of the data root, a document that is JSON but not an object, a run that is a file, a link loop, and an unreadable directory. Each one changes on purpose. The README table lists them, and delta-check.sh prints base against candidate:

  • run directory linked out: list shows unknown; show refuses, exit 4.
  • result.json linked out: list shows unknown; show prints (missing or unreadable).
  • runs/ linked out: list and show refuse, exit 4.
  • result.json is 5: base list crashed (exit 1); base show printed undefined fields. The candidate gives unknown and (missing or unreadable).
  • task.json is []: base printed the snapshot line; the candidate omits it.
  • run is a regular file: base show crashed in readdirSync; the candidate gives run not found, exit 4.
  • link loop: base said run not found; the candidate refuses with ELOOP, exit 4.
  • runs/ unreadable: base list printed nothing, exit 0; the candidate refuses with EACCES, exit 4.

Mutants (out/mutants.sh, out/mutants.txt)

I ran 32 single-line mutants of src/ and scripts/mosaic-task.mjs against the packages/runs tests.

First round: 26 killed, 4 survived and 2 didn't apply. Three of the survivors were real gaps, and I added a test for each:

  • M02 dropped the exact .. check. A link to the data root's parent got through. New test: "a link to the data root's parent is outside it".
  • M09 dropped the sort. Node returned this directory already sorted, so no fixture could catch it. New test: it mocks fs.readdirSync to return the names reversed.
  • M15 read every readdir error on a run directory as missing. New test: "an unreadable run directory refuses instead of reading as missing".

Final round: 30 killed, 2 survived. Both survivors are equivalent:

  • M04 dropped !path.isAbsolute(relative). On POSIX, path.relative between two absolute paths is never absolute, so the branch is unreachable on Linux. I kept it as a guard for other platforms.

  • M31 dropped the CLI's isRunId pre-check in show. readRunRecord throws the same message with exit 4, and readRuns maps it to the same fail. The pre-check stays so that an invalid id refuses before the config loads, as it did before.

    Corrected in round 2: M31 was not equivalent. See "Correction: M31 was never equivalent" above; it is killed in round 2.

Follow-up (not fixed, out of scope)

This defect predates the candidate and is unchanged: list crashes with a padEnd TypeError when result.json is an object without a string status or taskId. Base and candidate behave the same here, so byte-identity holds. A fix would change list output and belongs in its own row.

Reproduce

git worktree add --detach /tmp/r56-base 75ab1646
git worktree add --detach /tmp/r56-cand 75ab1646
(cd /tmp/r56-cand && git apply --index <packet>/build.patch && sha256sum -c <packet>/candidate-manifest.sha256)
env -u NODE_TEST_CONTEXT node --test '/tmp/r56-cand/packages/runs/tests/*.test.mjs'
bash /tmp/r56-cand/agents/rocko/work/queue-56/byte-check.sh /tmp/r56-base /tmp/r56-cand /tmp/r56-bc
bash /tmp/r56-cand/agents/rocko/work/queue-56/delta-check.sh /tmp/r56-base /tmp/r56-cand /tmp/r56-dc