Reduce the codebase by 40% and fix the defects found in the cleanliness audit

[nnennandukwe]

Goal

Reduce the TypeScript surface by 40% while fixing the correctness and security defects found in the same audit.
Keep every published contract and every guarantee the lifecycle graph makes.

"Cleaner" here means four concrete things:

• Less code. Delete code that exists only for states that cannot occur. Examples are migrations for databases
  that were never deployed, stored copies of derivable facts, and checks that cannot fire.
• Right-sized abstractions. One implementation where there are now two or three near-copies. No new layer
  unless it removes more than it adds.
• One source of truth per fact. Schemas and types are declared once, and validators, JSON Schema, and DDL are
  derived from them.
• Tests that catch bugs. Keep one test per behavior at the cheapest level that proves it. Remove tests that only
  restate docs, pin formatting, or repeat a stronger test.

Metric

The metric is lines in src/, tests/, and scripts/, counting *.ts and *.mjs only. JSON and YAML fixtures
are excluded.

Area	Baseline (357fc58)	Target
src/	14,292	~8,800
tests/	20,008	~12,000
scripts/	6,700	~3,800
Total	41,000	≤ 24,600 (−40%)

Measure it with this command:

git ls-files 'src/*.ts' 'tests/*.ts' 'scripts/*.ts' 'scripts/*.mjs' | xargs cat | wc -l

Each PR in this effort reports its before and after totals.

Constraints

• Published contracts are frozen. The schemas, fixtures, and vectors under docs/contracts/**, the
  threadloop protocol --json output, and the receipt formats in docs/attestations/ stay byte-for-byte identical
  unless a PR says otherwise and explains why.
• Pilot databases must keep working. Issue Storage schema versioning makes additive changes as expensive as breaking ones #85 records a consumer pilot that ran storage v7 and then v8. Storage
  v7 is the oldest schema this effort keeps an upgrade path for. See Decisions.
• Owner's storage direction. Storage schema versioning makes additive changes as expensive as breaking ones #85 favors capability and shape checks over an ordinal version. The storage
  cleanup derives those shape checks from one table definition. It does not delete them.
• Merge gate. Every PR must pass npm run check, all required CI checks, and a Qodo review with every thread
  resolved before merge.
• Overlap. PR Diagnose setup and gate repository mutations #99 touches gate-runner.ts, session-service.ts, and session-gate.test.ts. The PRs here are
  ordered so the small correctness fixes land before the large rewrites of those files.

Findings

This comes from a full read-only audit of src/, tests/, scripts/, and the workflows. (V) marks findings
verified against the code, and most of those were also reproduced.

Correctness and security defects

#	Severity	Finding	Location
1	Critical	(V) Signing fails for every gate that declares setup. The sign step re-validates the gate inside a synthetic unversioned plan (allowSetup: false), while the run step admits it. It throws setup[0] must contain exactly: id, command, working_directory, timeout_ms.	scripts/sign-ci-gate-receipt.ts:55
2	Critical	(V) A timeout does not stop a gate that forks. Only the direct child is signalled, and the runner waits for close, which needs every pipe holder to exit. sh -c 'sleep 8 & sleep 8' with a 500 ms timeout returned after 8 s and left an orphaned process behind.	src/adapters/process/gate-runner.ts:232
3	High	(V) An old approval can be replayed over newer blockers. Review evidence is taken from the receipt with the highest import sequence, and observed_at is validated but never used. Import A (approved), then B (blocking thread), then re-import A, and the session reads as clear.	src/domain/review.ts:359
4	High	(V) Guard evidence is read outside the write transaction. The transaction compares only state_version, and receipt appends never bump it. A failing gate run or a CHANGES_REQUESTED import can therefore race a transition that was approved on stale evidence.	src/services/session-service.ts:338, src/adapters/fs/sqlite-store.ts:1174
5	Medium	(V) getChangedFiles mangles paths. trim().slice(3) turns M foo.txt into oo.txt and renames into "a" -> "b". The mangled names reach the change-brief and PR-summary artifacts.	src/adapters/git/client.ts:153
6	Medium	(V) threadloop and threadloop session print nothing and exit 0. Help-on-error goes to a swallowed writeErr, and commander.help is rewritten to exit 0.	src/cli-program.ts:103, src/cli.ts:74
7	Medium	A repair entered from reviewing can get stuck. If the newest review evidence is corrupt, repairBasis is undefined and repairing → verifying is denied forever.	src/services/session-service.ts:1688
8	Medium	The repair or implementation basis can be a commit that never ran. latestFailure includes invalidated and setup_failed receipts, whose head_after then becomes the basis.	src/services/session-service.ts:1645
9	Medium	session next can offer a candidate that the apply step rejects. planVerifyingTransition hard-codes executable: true for post-PR verifying → repairing without evaluating the guard.	src/domain/session-transition.ts:1022
10	Medium	Errors are classified by matching message prefixes. Several store errors (Invalid schema trigger, CAS failures, new Error(corruption)) match no prefix and escape as raw crashes.	src/services/session-service.ts:2180
11	Medium	Reads and writes disagree about corruption. A write silently rebuilds a broken active-session projection that read-only commands report as STATE_CORRUPTED, and two tests pin the opposite behaviors.	src/adapters/fs/sqlite-store.ts:1922
12	Low	The PR number and base ref in a review receipt are never bound to the session, so any PR whose head is the current HEAD qualifies.	src/services/signed-receipt-import.ts:564
13	Low	The review sensor throws on author: null (a deleted account) for approvals, and a non-JSON error response hides the HTTP status.	src/adapters/github/review-sensor.ts:127,236
14	Low	canonicalJson is not RFC 8785 (integer-like keys sort numerically), and it serializes a Date as {}.	src/domain/canonical-json.ts
15	Low	Guard messages that operators see still say "not available in M002-2" / IMPLEMENT_ISSUE_40.	src/domain/session-transition.ts:1098
16	Low	A promoted receipt package is written to disk inside the SQLite transaction, so a failed COMMIT leaves an orphaned file.	src/adapters/fs/sqlite-store.ts:1360

Checked and not bugs. The audit flagged these two, but both hold up. Both import paths reject a receipt whose
head is not the live HEAD before persisting, so the status: current reported by session review import and the
CI-proof status reported by session gate import are accurate at import time.

Where the size comes from

Storage: sqlite-store.ts, 3,569 lines, target ~1,500

• Migration paths for schemas that were never deployed. Storage v2–v8 landed between 2026-07-23 and 2026-07-31,
  and the JSON store existed for a few hours on 2026-03-14. The JSON import, v1–v6 upgrades, audit-floor logic, and
  version branches in the services add up to ~500 lines. The version branches include schemaVersion >= 4 checks
  that are always true after ensureStateDatabase.
• Active sessions are stored three times. active_sessions, the active_state singleton, and
  StateData.active are all copies of tasks.status plus sessions.ended_at. About 150 lines exist to keep the
  copies in sync, and that sync logic causes defect 11 and a whole-repo scan on every write.
• 24 hand-written immutability triggers. They are listed again in four constant arrays and re-checked by four
  hand-maintained assert*SchemaShape functions (~350 lines). These should all be generated from one table
  definition.
• 13 hand-written *Row types with field-by-field camelCase mapping. The proof-plan query is also duplicated.
  Together that is ~250 lines.
• Three receipt-append functions that are 95% identical, plus a promise write queue that serializes
  already-synchronous node:sqlite calls (~250 lines).

Services and transitions: 4,185 lines, target ~2,700

• Two parallel signed-import flows (gate and review). They share every step except the parser and the context
  assertion (~200 lines).
• 33 inline deniedGuards(...) literals. Each is 12–15 lines, and the same pairs repeat up to 7 times. A code
  table plus a candidate() helper removes ~450 lines.
• State-error mapping copy-pasted into 6 entry points (~100 lines).
• Three copies of stored-proof-plan verification, four of path containment, and three of the
  evaluateSessionProof fallback object (~125 lines).

Domain: attestation.ts, proof.ts, review.ts, sigstore.ts, ~3,300 lines, target ~2,000

• Every receipt, plan, and payload validator is hand-written, even though zod is already a dependency. The
  interfaces are then maintained a second time by hand, and a z.infer would replace them.
• The helpers are copied three times. Object, exact-keys, SHA, timestamp, identifier, and repository-regex
  helpers exist in three files, and the copies have drifted. Timestamps are strict ISO in one and Date.parse in
  another; identifier length is 160 in one and 256 in another.
• Gate and setup validation is re-implemented in attestation.ts, separately from proof.ts, and the two
  already disagree on setup_failed with zero steps.
• The gate and review signed-package pipelines are line-for-line mirrors.
• assertSignerProjection can never fail. It compares values that verifySigstoreReceipt copies from the
  policy back to that same policy.

CLI, commands, and scripts: ~4,400 lines, target ~3,000

• Legacy start, status, and capture aliases, and a daemon sleep loop. The runner skill uses none of
  them. Removing them also removes their allowLegacy* flags, duplicated option blocks, and tests.
• 14 command files that are pure pass-through. Every command name is also listed in four places: the handler
  interface, the no-op factory, the wiring, and detectInvokedCommand. One command table replaces all of them.
• A 421-line re-implementation of GitHub issue-form validation in check:static.
• The gate-sensor preamble was copied into both sensor scripts, and the copies drifted, which caused defect 1.

Contract tooling: scripts/*-contract, workflow-graph, controller-conformance, 5,759 lines plus 6,671 lines of tests and fixtures

Nothing in src/ imports this layer, and CI never runs the spec:* scripts; its consumer is RunInvariant, through
docs/contracts/**. Those published bytes are frozen. That includes 18 schemas sha256-pinned in compatibility.json,
the conformance corpus digest, the vectors, and every fixture. Because zod instance identity and $defs names show
up in the emitted schemas, shared primitives have to come from a factory called once per module, not from shared
instances.

• Tests that pass for the wrong reason.
  • gaap-ledger-review.test.ts:160 gets the same diagnostic with and without the budget change, so the
    completed-over-budget check has no negative test.
  • Four of the executor-admission.test.ts:94 cases fail on REQUEST_IDENTITY_MISMATCH before reaching the check
    they name.
  • About 124 bare .ok).toBe(false) assertions check no diagnostic code.
• A validation gap. validateSubjectResponse accepts a blanket blocked{NO_APPLICABLE_REMEDY} answer on 24 of
  25 decide cases. compareCaseResult catches the mismatch, but the checker's ok means nothing.
• A drift risk. zod is ^4.1.11 while emitter output is byte-pinned, so a lockfile refresh can break the frozen
  schemas. It should be pinned exactly.
• Dead checks. Timestamp re-checks run after z.iso.datetime({precision:3}) and duplicate-policy checks run after
  zod refines that already reject the same input.
• Duplication.
  • Six diagnostic constructors, six digest helpers, and three bounded-JSON walkers.
  • Three copies of the subject-identity comparison and five near-identical published*Schemas() bodies.
  • The shared kernel lives inside workflow-graph/contracts.ts.
  • The journal is re-parsed and replayed repeatedly, which makes replay O(n²).
• Test redundancy.
  • Hand tests restate corpus fixtures.
  • The codec is tested three times, and schema parity is implemented five times.
  • Several files are organized by review round rather than by behavior.

The estimated consolidation is about 12.4k → 9.1k lines (−26%). Going past that would mean archiving the TypeScript
reference semantics that give the 38 frozen conformance expectations their only executable check. That is a capability
decision and is not part of this cleanup.

Test suite: 20,008 lines

• Assertions that cannot fail. session-gate-import.test.ts:303,979 assert inside an injected reader whose
  throws production code maps to the expected error. cli.test.ts:1452 resets connections in the test process
  while every operation runs in a subprocess.
• Contradictory tests. cli.test.ts:649 and session-service.test.ts:122 apply the same corruption and expect
  opposite outcomes.
• Duplicated infrastructure.
  • 34 inline copies of the 15-line session transition argv.
  • 117 hand-written DatabaseSync open/close blocks.
  • Repo, session, and plan fixtures duplicated three times.
  • Signed-package builders duplicated three times.
  • Legacy DDL duplicated twice.
  • Estimated ~1,500 lines in total.
• The same guard is proven end-to-end in 2–5 places at 2–8 s each, although it is already unit-tested.
• Change-detector tests.
  • Pinned action SHAs; every Dependabot bump edits ci-workflow.test.ts.
  • A literal commit SHA and prose in a markdown doc.
  • SKILL.md sentences.
  • 70 lines of --help substrings.
  • Handoff headings.
• packaging.test.ts (12 s) duplicates scripts/smoke-pack.mjs, and CI runs both.
• Leaked temp directories. Several files never delete their temp dirs; 614 threadloop-* directories had
  accumulated in $TMPDIR.

Decisions

These are recorded here so review can challenge them.

1. Review ordering (defect 3). An import is rejected when its observed_at is older than the newest stored
   review snapshot for the same session. This fails loudly and keeps "latest" meaningful. Ranking by observed_at
   instead would tolerate out-of-order delivery but make evidence harder to reason about.
2. Evidence race (defect 4). Guard evaluation records an evidence watermark (the newest receipt sequences) and
   re-checks it inside the write transaction, failing with a retryable conflict. state_version keeps its current
   meaning for callers.
3. Legacy storage. v7 is the oldest schema with an upgrade path. Older databases get an explicit
   STATE_MIGRATION_UNSUPPORTED error that tells the operator to upgrade with an older ThreadLoop release. No data
   is deleted.
4. CLI surface. start, status, capture, and daemon are removed. The documented surface is
   init, session *, artifact generate, audit *, and protocol, and it does not change otherwise.

Plan

Each step is one PR (or a small set of PRs). Each is reviewed by Qodo and merged before the next begins.

• 1. Correctness fixes: defects 1, 2, 5, and 6, plus the two vacuous bounded-read tests, each with a
  regression test. Done in Fix setup-gate signing, gate timeouts, changed-file paths, and silent CLI usage #134 (merged).
• 2. Evidence integrity: defects 3, 4, and 12 (Reject replayed review snapshots and evidence that changes mid-transition #135, merged). Defects 7–9 move to step 6, which restructures the code
  they live in. Defect 11 goes away in step 4 once the active-session copies are removed.
• 3. CLI surface: remove the legacy aliases and the daemon, move to one command tree, fold the command files,
  and fold packaging.test.ts into smoke-pack (Remove legacy root commands and the daemon; wire one command tree #136, merged).
• 4. Storage (Drop unshipped storage migrations and derive active sessions #137, merged): drop pre-v7 migration paths, derive active sessions (fixes defect 11), generate triggers and
  shape checks from one table definition, use a generic row mapper, have one receipt-append path, and remove the
  write queue.
• 5. Domain validation (Validate domain receipts and plans through shared schemas and one signed-package pipeline #139, merged): strict zod schemas with z.infer types, one shared helper module, one signed-package
  pipeline, and removal of dead signer checks.
• 6. Services and transitions (Fix repair-basis, planner, and corruption-classification defects; unify service flows #140, merged): fix defects 7–10 with regression tests, add a guard denial table, one
  signed-import flow, one state-error mapper (with typed errors replacing message matching), and one proof-state
  projection.
• 7. Contract tooling (Consolidate contract tooling onto one kernel with published bytes unchanged #138, merged): a shared contract kernel with the published output byte-identical. Also fix the
  wrong-reason tests, add diagnostic-code assertions, pin zod exactly, and merge the review-round test files by
  behavior.
• 8. Test suite (Drop change-detector tests and shrink the community-file checker #141 and test: shared integration helpers and duplicate-test removal (#133) #144, merged): shared helpers, removal of duplicate and change-detector tests, and temp-dir cleanup.

Progress

PR	Lines before	Lines after	Change
baseline	41,000
#134 correctness fixes (merged)	41,000	41,094	+94
#135 evidence integrity (merged)	41,094	41,378	+284
#136 CLI surface (merged)	41,378	40,638	−740
#137 storage (merged)	40,638	38,450	−2,188
#138 contract tooling (merged)	41,378	39,843	−1,535
#139 domain validation (merged)	38,450	37,675	−775
#140 services and transitions (merged)	37,675	37,497	−178
#141 change detectors (merged)	38,450	38,111	−339
#142 adapters and sensors (merged, measured on main)	35,989	36,032	+43
#144 test infrastructure (merged)	35,715	32,261	−3,454
#145 contract layer moved to threadloop-contracts (merged)	32,261	21,448	−10,813
main at 10cabbd (#134–#145 merged)	41,000	21,448	−19,552 (−47.7%)

Rows measure each PR against its own base, so they overlap and do not sum. The main row is the running total.

Outcome

Target met: main is at 21,448 lines, against a target of ≤ 24,600. By area: src/ 11,534, tests/ 9,211, scripts/ 703.

• Steps 1–8 removed 8,739 lines while preserving behavior, and fixed defects 1–16.
• The contract reference layer (Controller Contract v0.1: specifications, conformance corpus, validators, and 398 tests) was nothing the runtime imported. The owner decided to keep it for the controller, GAAP, and Rust roadmap (Phase 5B: Execute the GAAP process adapter and verify Terminal Run Receipts #111, Phase 7B: Add generated controller, Execution Claim, and receipt state-machine tests #112), in its own repository: nnennandukwe/threadloop-contracts, tagged v0.1.0 with history preserved (refactor: move the Controller Contract layer to threadloop-contracts (#133) #145).
  • Its CI regenerates every published schema and fails on drift.
  • The corpus digest is unchanged: 11620588….
• ThreadLoop keeps tests/unit/lifecycle-contract-drift.test.ts. It checks the runtime lifecycle against a vendored copy of the v0.1 Governed PR profile: states, transitions, blocking and recovery, phases, and the repair budget.
• RunInvariant still pins a ThreadLoop commit, so nothing broke. Re-pin the ThreadLoop conformance corpus to threadloop-contracts on next update run-invariant#6 tracks its next re-pin.