Tech-debt tracker — non-blocking follow-ups

[SSFSKIM]

The parent for non-blocking follow-ups on the doperpowers plugin itself (same shape as the consumer-repo tech-debt sink: review findings and known costs that don't block a merge land here instead of getting lost).

Open items

(none)

Closed items

• tests/qa-loops/test-review-engine.sh — decision (delegated): review-engine.sh is live and load-bearing for the qa-loops loop on main (PR requesting-review: split the review job out of codex-companion, runtime fail-closed, engine script retired #55's retirement is stale against pre-rename paths), so the suite was rebuilt, not deleted — and re-pinned at the seam the engine owns. It now stubs node and asserts the with-effort.mjs command line the engine composes (verb routing, defaults, lens contract), the env recipe, rc mapping, and the sandbox fail-closed check; the app-server thread protocol stays pinned in tests/codex-companion/ against the protocol-speaking fake codex, so a future transport change can no longer rot this suite. Zero real-node dependency; mutation-checked ×3. Landed in PR Worker-orbit maintenance: unforgeable triage dedup, broker self-reap, review-engine suite re-pin #82.
• Retired-worker sessions linger in the harness session list — closed as upstream-blocked (delegated judgment). The investigation stands: the purge tier already exists (daemon-retire.sh <id> purge), claude agents exposes no delete verb, and an age-gated registry GC would cost the retired-worker audit trail while fixing nothing the report complained about. Nothing buildable in this repo can act, so the checklist line only masquerades as work; the session-list ask belongs upstream (Claude Code). If registry metadata volume ever becomes its own problem — distinct from the session list — that is a fresh, differently-motivated item. Original context: the 2026-07-19 launchd-cwd incident left 20 retired 490-* metas and 9 retired review-pr-574 metas.
• board snapshot caching — a dispatching tick runs ~3 full-board GraphQL snapshots per ticket #25 — board snapshot caching. Already fixed on main before this tracker was swept: _board.py carries a per-process memo plus an opt-in cross-process file cache (BOARD_SNAPSHOT_CACHE), invalidated after every spawn, covered by tests/issue-tracker/test-snapshot-cache.sh. Closed 2026-08-21.
• test gap: hermetic case for the normalize-owners-before-bind preflight #24 — hermetic test for the normalize-owners-before-bind preflight. Landed in tech-debt: hermetic normalize-owners case, two non-hermetic suites, engine:claude wording (dp#24, dp#26) #79, both directions (finished owner → bind succeeds; live owner → bind refuses and the reviewer is retired), mutation-checked.
• tests/qa-loops/test-review-dispatch.sh — the review-pr-3 fixed-string assertion that matched the suite's own checkout path. Landed in tech-debt: hermetic normalize-owners case, two non-hermetic suites, engine:claude wording (dp#24, dp#26) #79.
• tests/issue-tracker/test-board-scripts.sh — the timing-flaky lock-wait race, now a marker handoff instead of a 1.0s timer. Landed in tech-debt: hermetic normalize-owners case, two non-hermetic suites, engine:claude wording (dp#24, dp#26) #79.
• skills/issue-tracker/SKILL.md — the engine:claude "redundant" claim, now qualified against the WORKER_ENGINE override. Landed in tech-debt: hermetic normalize-owners case, two non-hermetic suites, engine:claude wording (dp#24, dp#26) #79.
• tests/issue-tracker/test-board-scripts.sh — board-map.sh --write discarded its rc, making the intermittent interactive-preferred failure undiagnosable. Landed in tech-debt: hermetic normalize-owners case, two non-hermetic suites, engine:claude wording (dp#24, dp#26) #79.
• tests/claude-code/test-subagent-driven-development.sh — model capitalization drift; fixed 2026-07-28 by making the semantic-presence assertions case-insensitive.

Conventions

• One child issue per item once it's actionable; a checklist line is enough while it's just a known cost.
• This issue carries the tech-debt label so review tooling that looks for the standing tech-debt sink finds it.

[SSFSKIM]

Two non-hermetic test suites (found while reviewing PR #35)

Both are pre-existing — the scripts under test and the test files are
byte-identical between origin/main and PR #35's head, so neither is that PR's
doing. Deferred as non-blockers, but both cost this pipeline real time: they made
a clean PR look like it had three regressions, and a review worker that runs the
repo's own suites to verify a PR's Validation Evidence will keep hitting them.

1. tests/reviewing-prs/test-review-dispatch.sh:854 — false failure when the checkout path contains review-pr-3

assert_not_contains "$(cat "$SPAWN_LOG")" "review-pr-3" "failing first PR is not spawned"
is a fixed-string substring match. The spawn log contains the fully rendered
worker prompt, which carries absolute plugin paths — including the directory the
suite is being run from. When that directory is a review worktree named
review-pr-35, the log holds review-pr-35 nine times, and grep -F "review-pr-3" matches it. The negative assertion fails even though PR 3 was
never spawned.

Observed: 2 failures at PR #35's head when run from
.claude/worktrees/review-pr-35 (failing first PR is not spawned and
bad PR after a good one is never spawned (no stale-state dispatch)),
reproducibly, 2 runs of 2. The identical tree run from a scratch checkout at a
path without that substring: all tests passed. Evidence:

$ grep -o "review-pr-3[0-9]*" head-review-dispatch-2.log | sort | uniq -c
   2 review-pr-3
   9 review-pr-35

Severity: low as a product defect, moderate as pipeline friction — it fires for
any review of PR #3, #30–#39, obra#300–obra#399, … i.e. whenever the reviewer's own
worktree name contains the needle. Fix direction: anchor the assertion to a
whole spawn-name token (e.g. match review-pr-3 / spawn:--no-wait review-pr-3
with the trailing delimiter, or assert against just the spawn-name field rather
than the whole log, the way the same file's [model=] bracketing trick already
does).

2. tests/issue-tracker/test-board-scripts.sh:382-396 — timing-flaky lock-wait race

The lock-wait park case holds $DAEMON_HOME/.metalock for a fixed 1.0 s,
backgrounds board-bind.sh at t≈0 s, then runs a real board-transition.sh 999 needs-human at t≈0.2 s and expects the park to land before the lock releases. When
board-transition.sh takes longer than ~0.8 s, the contender acquires the lock
while the ticket is still ready-for-agent, binds legitimately, and all three
assertions fail together:

[FAIL] bind re-reads needs-human after lock acquisition
[FAIL] lock-wait park keeps the original owner
[FAIL] lock-wait contender never acquires the parked ticket

Evidence that it is nondeterministic rather than a real regression: identical
code in the identical directory gave pass / FAIL / pass across three consecutive
runs. Separately, origin/main failed a different assertion on one run
(html payload carries the interactive-preferred class) and passed on four
others, including three under deliberate 8-way CPU load — so the suite has more
than one timing-sensitive spot.

Fix direction: make the handoff explicit instead of timed — have the lock holder
release on a marker file the test writes after board-transition.sh returns,
rather than on time.sleep(1.0).

Note on the third suite

tests/implementing-tickets/test-implement-dispatch.sh aborts immediately
(rc=1, no assertion output) when run from a directory that is not a git work
tree. Not a defect worth fixing — just a trap for anyone verifying a PR from a
git archive snapshot. git init in the snapshot is enough.

(Reviewer: review-pr-35 worker, PR #35 / ticket #33.)

[SSFSKIM]

engine:claude called "redundant" is only true when no global override is set

Finding — skills/issue-tracker/SKILL.md:155-156 (added by PR #35): "Label
engine:codex to put one ticket back on the gateway; engine:claude is
redundant now but still valid."

Severity — low (engine ranked it P3). Deferred as a non-blocker: PR #35 was
at its fix-wave cap, and this is wording precision in operator-facing prose, not
a behavioral defect. Routed here rather than waved because momentum on a correct
change outranks polish.

Why it is still worth fixing — the redundancy claim holds only in the default
configuration. WORKER_ENGINE=codex remains a supported override (the same PR
documents it in references/sweep-setup.md:103, and both the review and land
dispatchers rely on that variable), and labels win over the environment. So the
moment an operator exports WORKER_ENGINE=codex, engine:claude is the only
per-ticket way back onto plain Claude — the opposite of redundant. An operator
who reads "redundant" as "safe to strip" would silently change routing for that
ticket.

Note this is not a defect introduced by the flip itself: it is a statement that
is true by default and false under a documented override.

Fix direction — qualify rather than delete: say engine:claude is redundant
when no WORKER_ENGINE override is set, and remains the per-ticket override
when one is. One clause; the surrounding resolution order (label → $WORKER_ENGINE → default) already states the mechanism correctly one line above.

(Reviewer: review-pr-35 worker, PR #35 / ticket #33, engine round 3.)

[SSFSKIM]

Pre-existing test failure (found 2026-07-28 during multi-lens exit checks, reproduced on clean main): tests/claude-code/test-subagent-driven-development.sh Test 2 'Workflow ordering' fails — assert_order expects the live model's answer to match literal 'First:.*spec.*compliance', current model answers phrase it differently. Severity: low (test brittleness, skill content unchanged). Fix: loosen the pattern or pin the answer structure harder in the test prompt. Evidence: 3 consecutive fails (worktree + main), all other tests in the file pass.

[SSFSKIM]

RESOLVED (2026-07-28, follow-up to the earlier note): the test-subagent-driven-development.sh failure was model capitalization drift — the answer 'First: Spec compliance review' no longer matched the case-sensitive pattern 'First:.*spec.*compliance'. Fix landed on main: assert_contains and assert_order in tests/claude-code/test-helpers.sh are now case-insensitive (semantic-presence assertions over live model prose; assert_not_contains deliberately left case-sensitive), the Test 5 'reviewer reads code' phrasing net was broadened, and the SC2155 shellcheck warnings in the helpers were fixed. Full run-skill-tests.sh suite: STATUS PASSED.

[SSFSKIM]

Tracker sweep — PR #79

PR #79 lands three of the items below. Status of every open item, plus two findings from investigating them.

Landed in #79

• The review-pr-3 substring false failure — negative assertions now match the spawn-name field as a whole token (assert_no_spawn) instead of a bare substring of the whole spawn log. Reproduced from a directory named review-pr-35 (2 failures on the pre-fix suite), fixed and re-verified there (318 pass).
• The timing-flaky lock-wait race — the lock holder now releases on a marker written after board-transition.sh returns, rather than on a fixed 1.0s timer. A/B under the exact failure condition (a 2s stall so the transition finishes well after any fixed hold): old code fails all three assertions, new code passes 505.
• engine:claude called "redundant" — qualified, not deleted: redundant only while no WORKER_ENGINE override is set.

Plus #24 (hermetic normalize-owners-before-bind cases) and one bonus: board-map.sh --write no longer discards its rc, which is what made the intermittent interactive-preferred failure undiagnosable.

#25 — closed as already done

Fixed on main before this sweep; closed with evidence there rather than folded into #79.

---

Finding: the retired-worker session GC is blocked on a harness capability, and the investigation changes its premise

This item asked for "a purge-tier cleanup verb or an age-gated registry GC for retired metas", with "investigation first: what can the harness actually delete?" Answering that first turns out to matter, because it invalidates the framing.

1. The purge tier already exists. daemon-retire.sh <id> purge deletes the registry files (meta, reply, err) and, for codex daemons, the turn scratch its event_log points at. It deliberately never touches the session transcript, so claude --resume <uuid> still works.

2. The harness exposes no way to delete a session. claude agents takes options only — no subcommands, and nothing resembling rm, delete, or remove. The closest thing is --all, which includes completed background sessions in the JSON rather than clearing them.

So the reported symptom — retired workers lingering in the harness session list, which the human ends up closing by hand — cannot be fixed from this repo at all. Nothing we build here clears a session from that list.

That makes the proposed remedy a non-remedy: an age-gated registry GC would purge our own metadata, which is not what is showing up in the session list. It would also destroy the audit trail for retired workers (which worktree, which session, why retired) in exchange for solving nothing the issue actually complained about.

Recommendation: keep this item open but reframed. The registry side is already served by purge. The session-list side is an upstream harness ask, not work for this repo. If registry metadata volume ever becomes a problem in its own right — distinct from the session list — that is a separate, differently-motivated item.

---

New finding: tests/qa-loops/test-review-engine.sh is dead on any machine without a system node

Same class as the two non-hermetic suites already tracked here — a suite that fails for reasons unrelated to the code under test, and costs whoever runs it time. Pre-existing; PR #79 does not touch it.

Symptom. rc=127, zero assertions executed, output is one line: happy path (non-nested):.

Cause 1 — the PATH scrub removes a required dependency. Line 59 sets export PATH="$STUB_BIN:/usr/bin:/bin". The scrub's purpose is to hide a real codex CLI so the engine can only find the stub. But review-engine.sh:38 also requires node, which routinely lives outside /usr/bin (nvm, homebrew) — so the engine exits 127 at its node check before the first assertion.

Cause 2 — the stub no longer matches the engine's invocation shape, and this is the deeper one. Re-adding node's directory to the scrub gets past the check, and the suite then fails silently with rc=1 and no diagnostic. The stub codex logs argv and expects to be called as codex exec review --base ... (line 71 asserts "exec review --base origin/main"). What with-effort.mjs actually does is start an app-server and drive a review thread — a protocol the stub does not implement. Verified against a real codex, where with-effort.mjs works: it prints [codex] Starting Codex review thread. / Thread ready (<uuid>), not an exec review call.

So the suite was written against an earlier with-effort.mjs and has rotted. Note the last commit to touch the test is the vocabulary rename (b9264fc9), i.e. no substantive update since the wrapper changed.

Fix direction. Two independent pieces: (a) re-add node's own directory to the scrubbed PATH — the scrub only needs to hide codex, and the codex missing case at line 127 stays honest because codex is checked before node; (b) rewrite the stub to stand in for the app-server thread protocol with-effort.mjs actually speaks, or move these assertions to whatever surface is still stable. (b) is the real work and needs someone to decide what this suite should be pinning now.

A partial fix — (a) alone — was deliberately not shipped in #79, since it would have added one test edit that cannot be verified green.