feat(cogos-harness): threads registry + warn-tier hook (v0.3.0)

[chazmaniandinkle]

Summary

• Adds a threads registry (bin/threads add|list|check|close, state at ~/.cog/status/threads.json) so a seat's stated open-ended wait ("I'll hold until the verdict lands") survives compaction and session end instead of dying silently in conversation context — the failure this fixes happened on 2026-08-07 (a session watched a CI run and reported "done" when the process exited, before the actual review verdict had propagated).
• A thread is a resolution predicate, not a completion signal: a bounded shell command whose exit code answers "is the condition true", checkable by anyone at any time with no memory of who registered it. Declared fields only (id/what/why/predicate/opened_at/expected_by/owner/closed_at/closed_reason) — resolved/orphaned/overdue/age are always derived by re-running the predicate, never stored or hand-set.
• hooks/threads-warn.py (UserPromptSubmit): silent-by-default, per-turn check. It only actually runs a thread's predicate once that thread is past its expected_by (skip_predicate_if_not_due) — a thread with a distant deadline costs nothing turn after turn until it's actually due. Speaks for orphaned threads (unresolved AND past expected_by), and, distinctly, whenever its overall wall-clock budget (COGOS_THREADS_TOTAL_BUDGET) runs out before every open thread could be checked — that case renders an explicit "N not checked this turn — over budget" note rather than staying silent, with scan order rotated across invocations so the same thread never starves the budget check forever. Fail-open on any missing/corrupt state.
• hooks/threads-gate-pr.py (PreToolUse/Bash): scaffolded but disabled by default — can require an open thread before gh pr create runs, armed only via enforce_pr_create_thread: true in ~/.cog/status/threads-config.json. Nothing in this PR sets that key. Command matching tokenizes the shell command line (splitting on ;/&&/||/|/$(/newline, respecting quotes) and checks whether any segment's argv starts with gh pr create, rather than a bare substring match — so the phrase appearing inside a quoted git commit -m/grep argument doesn't trip the gate. Documented, in-code limit: shell variable-substitution evasion (C=create; gh pr $C) is out of scope for a warn-tier gate that must never itself run a shell.
• skills/threads/SKILL.md: the procedure, including a worked good-vs-bad predicate example, the skip_predicate_if_not_due behavior, and the budget-notice behavior above — and a real footgun caught empirically while building this (gh ... --jq '.x == "y"' prints false but still exits 0; predicates need [ "$(...)" = "..." ] to be unambiguous).
• plugin.json + marketplace.json bumped to 0.3.0 (cascaded together, both carried duplicate version/description/keywords for this plugin).

Follow-up commit (f054faa): four blocking hazards closed

An adversarial review of the initial design found four issues, all fixed in a follow-up commit before this PR was reviewed further:

1. Unbounded predicate output blew the per-turn budget — a predicate like cat /dev/zero cost 10.7s / 7.68GB against a hook with a 4s budget. Fixed by discarding predicate stdin/stdout/stderr (DEVNULL) instead of capturing output nothing reads.
2. Concurrent threads add silently lost registrations — an unguarded read-modify-write dropped entries under concurrent CLI invocations. Fixed with lib.locked_state(), an flock-guarded context manager used by add/close.
3. Empty/missing predicate self-reported RESOLVED — /bin/sh -c '' exits 0. threads add now rejects a blank --predicate; derive_status() additionally treats a missing/blank predicate as explicit-unresolved as defense in depth for hand-edited state.
4. The hook ran the predicate before checking whether it could matter — orphaned requires overdue, so derive_status always ran the predicate first even for a not-yet-due thread, discarding the result every time. Added skip_predicate_if_not_due (warn-hook only, never threads check).

First review pass: eight findings from an independent blind review, all fixed

A second independent review (2026-08-07) returned REQUEST_CHANGES with eight findings — F1 through F8. All eight are addressed on this branch:

• F1 (HIGH) — budget exhaustion could silently suppress the entire orphan report when a genuine orphan sat behind enough non-orphaned-but-slow predicates in registry order to exhaust the wall-clock budget (reproduced 5/5 in review). Fixed two ways: the budget note now renders unconditionally, even when zero orphans were found among the threads actually checked; and scan order rotates across invocations (a small persisted counter next to the state file) so the same thread doesn't starve forever. Covered by a deterministic, clock-driven reproduction of the exact starvation scenario, plus a test confirming rotation eventually surfaces the orphan.
• F2 — the PR-create gate matched a bare substring of the whole command, so git commit -m "docs: ... gh pr create ..." and grep -rn "gh pr create" . both tripped a false deny. Replaced with a tokenizer that splits the command line on shell separators (respecting quotes) and checks whether any segment's argv actually starts with gh pr create. Variable-substitution evasion is explicitly out of scope and documented as such — a warn-tier gate must never run a shell to catch it.
• F3 — this PR body itself: the original description understated the diff (stale test count, no mention of the f054faa hazard fixes or the skip-if-not-due behavior). Corrected in this same body.
• F4 — skills/threads/SKILL.md and README.md corrected to describe actual behavior: predicates only run for due/overdue threads, and the budget-notice behavior from F1 is documented.
• F5 — gate tests now set COGOS_THREADS_CONFIG to a fixture path unconditionally, so they never resolve to the operator's real ~/.cog/status/threads-config.json.
• F6 — an unparseable opened_at paired with a duration expected_by used to recompute now + duration fresh every call, a deadline that recedes forever and can never be reached. Same defense-in-depth class as the blank-predicate case: treated as already overdue-eligible instead.
• F7 — SCHEMA_VERSION is now enforced on load: a state file with a newer version than this code supports fails loudly (CorruptStateError) rather than parsing silently under the wrong assumptions.
• F8 — predicate timeout now kills the whole process group (start_new_session=True + os.killpg), not just the immediate /bin/sh child — a predicate that backgrounds or forks something the shell doesn't wait on no longer leaks a reparented process per turn per overdue slow/hanging thread.

Delta pass: independent re-review returned APPROVE, four new residuals closed

The same independent instance re-verified the fixes above with its own reproductions and returned APPROVE (all eight findings CONFIRMED fixed, F1's starvation scenario 5/5 → 0/5 silent, F2 adversarially swept, F8's leak reproduced dead on both trees). It surfaced four new LOW/INFO residuals in the new code, plus a lingering type-confusion gap in F7's fix — all closed in this same pass:

• NEW-1 — the F2 tokenizer regressed one case the OLD substring match caught for free: GH_TOKEN=x gh pr create (a leading env-assignment token shifts argv, so argv[:3] == ["gh","pr","create"] no longer matched). Fixed by stripping leading NAME=value tokens and a fused subshell ( before the compare; also added single & to the separator set (gh pr create & sleep 1 was previously one un-splittable segment). Backtick substitution and a backslash-escaped quote inside single quotes remain documented, accepted limits. Second-pass residual (NEW-5): the mirror case, a trailing ) fused to create with no flags after it ((gh pr create), FOO=$(gh pr create), (GH_TOKEN=x gh pr create)), also defeated the match — fixed with a per-token rstrip(")") on the first three tokens before the compare.
• NEW-2 — an F6 thread (unparseable opened_at + duration expected_by) rendered self-contradicting evidence on the triage surface: (overdue) next to age 0s, expected_by <tomorrow>, both synthesized from now standing in for the real timestamp. derive_status() now flags age_unknown/expected_by_unknown and adds an explicit unparseable_opened_at reason; both the warn hook's rendered line and threads check's output now show age ? / expected_by ? instead of the synthesized values.
• NEW-3 — documented the .threads.json.lock (flock-guarded CLI write lock) and .threads.json.rotation (F1's scan-order counter) sidecar files in the README's env/file reference; previously undocumented.
• NEW-4 — no code change needed; added a one-line comment at the process-group reap site (proc.wait(timeout=2) after killpg) noting the up-to-2s wall-clock overshoot it can add and that shipped defaults keep the warn hook's worst case under hooks.json's 8s timeout for it.
• F7 residual — the version guard was isinstance(version, int), which lets a non-int version (a hand-edited "2", a float, true) slide through unchecked, since the newer-than-supported comparison simply never fires for it. Now rejected the same way as a too-new version: CorruptStateError for any present-but-non-int version (bool explicitly excluded from counting as int, despite being a Python int subclass).

Test plan

• python3 tests/test_threads.py -v — 109/109 passing (37 at initial PR open → 48 after the four-hazard follow-up → 82 after the first review's eight-finding fixes → 102 after the delta pass's four-residual + F7-residual fixes → 109 after the second delta pass's NEW-5 trailing-paren fix)
• Covers, in addition to the original suite: F1's budget-notice-and-rotation fix (deterministic clock-driven scenarios), F2's command tokenizer (false-positive/negative cases via subprocess and direct unit tests, now including NEW-1's env-assignment/&/subshell-paren cases with DENY-expected armed-gate assertions), F5's hermetic gate-test config, F6's unparseable-opened_at defense-in-depth plus NEW-2's age_unknown/expected_by_unknown rendering (both the warn hook line and threads check's output), F7's schema-version enforcement plus its non-int-version residual, F8's process-group-kill (spawns a real backgrounded descendant, confirms via pgrep that it doesn't survive the timeout)
• Manually verified hook output for the three required scenarios (nothing registered / one healthy thread / one orphan) — see the SKILL.md worked example
• Confirmed hooks.json and both .claude-plugin/marketplace.json manifests still parse as valid JSON
• Not installed or activated anywhere — this PR only ships code; no settings.json/settings.local.json touched

[chazmaniandinkle]

Pushed a follow-up commit (f054faa) closing 4 blocking hazards an adversarial review found in the original design:

1. Unbounded predicate output blew the per-turn budget — run_predicate captured stdout/stderr but nothing read them; a predicate like cat /dev/zero cost 10.7s / 7.68GB on a hook with a 4s budget and an 8s hooks.json timeout. Fixed with DEVNULL on stdin/stdout/stderr. Reproduced fixed: same predicate now finishes in ~3s (budget-bounded) at ~17MB RSS.
2. Concurrent threads add silently lost registrations — unguarded read-modify-write; atomic_write's tempfile+replace only makes the final rename atomic, not the surrounding read-modify-write. Added lib.locked_state(), an flock-guarded context manager, used by add/close. Reproduced fixed: 8 concurrent adds (previously dropped to as few as 3) now consistently register all 8.
3. Empty/missing predicate self-reported RESOLVED — /bin/sh -c '' exits 0. threads add now rejects a blank --predicate; derive_status() additionally treats a missing/blank predicate as explicit-unresolved as defense in depth for hand-edited state.
4. The hook ran the predicate before checking whether it could matter — orphaned requires overdue, but derive_status always ran the predicate first, so an open thread with a multi-day expected_by paid full predicate latency (and, for the documented gh-based predicates, a network round trip) every turn of every session, discarded every time. Added skip_predicate_if_not_due (warn-hook only, never threads check): skips invoking the predicate while the thread isn't yet past its deadline, since the exit code can't change the answer in that case. A thread becomes exactly as checkable as before the instant it goes overdue.

14 new tests cover all four directly; existing 34 pass unmodified. Full suite: 48/48 (python3 tests/test_threads.py -v).

[chazmaniandinkle]

🕶 Independent blind review — REQUEST_CHANGES

Reviewed by a dispatched independent instance (Opus) with review-only authority; artifacts only, no author commentary in scope. Posted by the seat verbatim.

Basis: the warn hook can silently drop a genuine orphan forever when its wall-clock budget is consumed by non-orphaned threads — reproduced 5/5 — which is the exact "wait dies silently" failure class this PR exists to eliminate, and it is the one major branch in the hook with zero test coverage.

F1 — HIGH — Budget exhaustion silently suppresses the entire orphan report — CONFIRMED

hooks/threads-warn.py:128-129 with :104-106, :136-137. The skipped_for_budget note only renders inside the non-empty orphan_lines path. With budget consumed by slow non-orphaned predicates, remaining threads drop at the loop-top check and the hook emits nothing. Reproduced: 3 overdue threads (two slow-resolving, one truly orphaned), COGOS_THREADS_TOTAL_BUDGET=1.0 → byte-empty stdout, 5/5 runs. Scan order is plain registry order, so the same thread starves every turn, deterministically. Fixes (either sufficient): render the over-budget note unconditionally; rotate scan order.

F2 — MEDIUM — PR gate matches a bare substring of the whole command — CONFIRMED

hooks/threads-gate-pr.py:39,69. Armed with zero open threads: git commit -m "docs: describe gh pr create gating" → deny (false positive); grep -rn "gh pr create" ./docs → deny; C=create; gh pr $C --title x → allow (false negative). The plugin's own SKILL.md/README contain the literal string, so editing this feature's docs trips the gate. Also: "≥1 open thread exists anywhere" satisfies the check — any stale unrelated thread passes it.

F3 — MEDIUM — PR description understates and misreports the diff — CONFIRMED

Test plan says 37/37; branch head has 48/48 (f054faa added 14). Summary never mentions the four blocking-hazard fixes or that predicates are not run at all for not-yet-due threads — a material behavioral property.

F4 — MEDIUM — Shipped docs contradict shipped behavior — CONFIRMED

skills/threads/SKILL.md:141-142,155-156, README.md:38 still describe every-turn predicate checks and omit skip_predicate_if_not_due. SKILL.md is the agent-facing surface with a MUST-level invocation block; an agent reading it will believe its threads are actively re-checked every turn when they are not.

F5 — LOW — Gate tests not hermetic w.r.t. config — CONFIRMED

tests/test_threads.py:540-543: COGOS_THREADS_CONFIG inherited unset → resolves to the operator's real ~/.cog/status/threads-config.json. Arm the flag for real and the "disabled by default" test fails against the live machine.

F6 — LOW — Unparseable opened_at + duration expected_by → deadline recedes forever — PLAUSIBLE

lib/threads_core.py:371-372: opened = parse_ts(...) or now recomputes now + 1d every call; thread permanently invisible to the warn hook. Loads cleanly (load_state validates only truthy id). Same hazard class as the blank-predicate case that did get defense-in-depth.

F7 — LOW — SCHEMA_VERSION written, never read — PLAUSIBLE

threads_core.py:66 vs :177-184. A future v2 state file parses silently under v1 code.

F8 — LOW — Predicate timeout kills the shell, not descendants — PLAUSIBLE

threads_core.py:306-315: subprocess.run(timeout=) kills /bin/sh only; a gh/curl grandchild survives detached. Process leak, one per turn per overdue slow-network thread.

Checked and cleared

bin/threads PATH wiring (CC adds plugin bin/ to PATH; mode 100755) · concurrency (flock + atomic replace; 8-way test passes; lock-free hook reads safe) · fail-open posture verified empirically (missing/corrupt/empty state → empty stdout, exit 0) · _fail() inside locked_state releases cleanly · 48/48 tests pass locally.

Test adequacy

Strong where it aims (byte-exact assertions against real hook stdout). The gap is precisely F1: no test exercises COGOS_THREADS_TOTAL_BUDGET exhaustion at all.

[chazmaniandinkle]

🕶 Independent blind review — delta pass — APPROVE (head 74a7728)

Same independent instance as the first review, re-verifying with its own reproductions. Posted by the seat verbatim (condensed; full report held by the seat).

Basis: all eight findings substantively fixed — F1's starvation goes from 5/5 silent to 0/5 silent with both the unconditional budget notice and scan rotation independently verified; F2's false positives gone with every true positive preserved; F8's process leak reproduced dead — 82/82 tests, no regression to the silence contract, residuals fail-open or cosmetic.

finding	verdict	evidence
F1 (HIGH)	FIXED — CONFIRMED	Identical repro, 5 turns: before = 5×[SILENT]; after = budget notice turn 1, orphan surfaced turns 2/3/5 via rotation. TestWarnHookBudgetFairness drives the exact scenario with an injectable clock.
F2	FIXED — CONFIRMED	Full table re-run: both false positives now ALLOW, all seven true positives still DENY. Tokenizer survived adversarial sweep (unbalanced quotes, nested $( ), 200 KB command, 50k ;) — no raise, no hang, ≤0.57 s.
F3	FIXED — CONFIRMED	Body's 82/82 matches measured; f054faa hazards disclosed; skip-if-not-due described. Nit: "eight findings, all fixed" section enumerates seven — F3 itself absent (self-evidencing).
F4	FIXED — CONFIRMED	All three doc sites checked against actual behavior.
F5	FIXED — CONFIRMED	Gate tests hermetic via fixture COGOS_THREADS_CONFIG.
F6	FIXED (detection) — CONFIRMED	Correct overdue-eligibility; see NEW-2 for the rendering side effect.
F7	FIXED — CONFIRMED	version: 99 → warn hook silent (fail-open surface), threads list loud, exit 1. Residual: isinstance(int) guard lets string "2" load.
F8	FIXED — CONFIRMED	Same probe both trees: OLD leaked the backgrounded descendant, NEW did not. killpg-on-zombie race safe by construction.

Regression check: silence contract re-verified directly (missing/empty/corrupt state, no threads, future schema, healthy not-yet-due → all SILENT rc=0). Rotation counter survives every hostile state constructed (non-integer, negative, directory, unwritable parent) → offset-0 fallback, all orphans still reported.

New findings (all LOW/INFO, fail-open)

• NEW-1 — residual gate false negatives: GH_TOKEN=x gh pr create (leading env assignment — the old regex caught this; ordinary usage, not evasion), single & not a separator, (subshell) paren fusing, backticks, backslash-in-single-quotes. Fix: strip leading NAME=value tokens; add & to separators.
• NEW-2 — F6 threads render self-contradicting evidence: (overdue) beside age 0s, expected_by <tomorrow> because opened falls back to now. Suggest an explicit reason token and age ? rendering.
• NEW-3 — INFO: sidecar files .threads.json.rotation / .threads.json.lock in ~/.cog/status/ absent from the README file reference.
• NEW-4 — PLAUSIBLE: killpg reap (proc.wait(timeout=2)) can overshoot the wall-clock budget by 2 s per timing-out predicate; safe at shipped defaults (~5.2 s vs 8 s hooks.json timeout), reachable only by env misconfiguration.

[chazmaniandinkle]

🕶 Independent blind review — final pass — APPROVE (head 2bdbfb2, NEW-5 closed at 929cbbe)

Same independent instance, third pass. Posted by the seat verbatim (condensed).

Basis: all five residuals substantively fixed and verified by reproduction — every NEW-1 case now denies while all F2 false positives stay allowed; NEW-2 renders age ? / expected_by ? with an explicit unparseable_opened_at reason on all three surfaces; F7 rejects every non-int version — 102/102 passing at review head, silence contract intact.

• NEW-1 FIXED — full table re-run: GH_TOKEN=x …, A=1 B=2 …, single &, subshell parens all DENY; documented limits (var-substitution, backticks, backslash-in-single-quotes) correctly remain and are now enumerated in the docstring. No F2 regression in either direction; &&/|| precedence over &/| verified correct.
• NEW-2 FIXED — before/after render confirmed on warn hook, threads check, threads list; the narrower flag is right: unparseable opened_at + absolute expected_by keeps the real deadline (a measurement, not a guess) and no false reason.
• NEW-3 FIXED — sidecars documented incl. delete-safety and read-failure degradation.
• NEW-4 FIXED as specified — and the comment's arithmetic verified empirically: worst-case 4-thread hang+background scenario measured 4.24s against the 8s hooks.json ceiling.
• F7 residual FIXED — "2", 2.0, true, [1], {} all rejected loud via CLI / silent via hook; bool explicitly excluded despite subclassing int.

One further LOW found and since closed (929cbbe, 109/109): trailing ) fused to create defeated the subshell strip — (gh pr create) allowed. Prescribed one-line per-token paren strip applied with DENY tests for the three exact cases; seat spot-verified post-push (three MATCH, false-positive case still clean).

Arc: initial blind review (REQUEST_CHANGES, F1–F8) → 8 fixes → delta re-review with independent reproductions (APPROVE + 4 residuals) → residual fixes → targeted final verify (APPROVE + NEW-5) → NEW-5 closed. Tests 37 → 48 → 82 → 102 → 109.