fix(mt#4816): Re-root the last repo-rooted telemetry writer and retire the "repo" location

[minsky-ai]

Summary

verify-subagent-model.ts:234 resolved its log as resolve(findRepoRoot(cwd), ".minsky/subagent-model-mismatch.jsonl") — the last telemetry writer depositing a file into whatever Minsky-managed project the agent happened to be in, the condition mt#4748's SC2 forbids. Unlike its siblings nothing was BROKEN: reader and writer agreed. The defect was only that the place they agreed on was a working tree.

This is the fourth and last task in the family (mt#4752, mt#4778, mt#4811, this).

SC1's decision: FLAT, not project-keyed — and why

The spec left this open because resolveStreamPath project-keys only the calibration and evaluation families; subagent-model-mismatch is family: "special", so flipping it to state-dir lands it flat, and that decides what special means. Three pieces of evidence, in the order that settled it:

1. Project attribution never depended on the path — read, not assumed. attachProjectIds (ingest-service.ts:313-328) takes sourcePath but uses it only for the source_path column; projectId comes solely from resolveProjectIds(sessionIds), resolving each record's session_id through conversationRunStateTable.projectId. This was the falsifier for the decision's own premise and was run before the decision was written.
2. Measured against production. The two flat state-dir families carry populated project_id at scale — fire-log 1,003,816 rows non-null, guard-health-log 121. Both of this stream's own rows are already in guard_events with project_id non-null.
3. The record's subject is the DISPATCH, not the repo. Every field written (session_id, dispatching_agent_id, tool_name, requested, resolved) describes model-tier resolution on an Agent dispatch. Nothing in it is a property of the repository being worked.

Key changes

• .minsky/hooks/verify-subagent-model.ts — MISMATCH_LOG is now the BARE name, byte-identical to the manifest row's relativePath; getSubagentModelStateDir / getMismatchLogPath resolve flat under the state dir. appendMismatchRecord takes env (injected, not read from the module) and no longer takes cwd.
• A latent reader/writer split, found while picking the resolver, and avoided. getMinskyStateDir() (@minsky/shared/paths) keys on XDG_STATE_HOME only — mt#3965 records that as deliberate — while the ingest reader's resolveStateDir keys on MINSKY_STATE_DIR. Using the shared helper here would have reproduced exactly the writer/reader split mt#4811 found live in ask-form-lint, where the sweep read an empty corpus and reported "this guard never fired." This writer follows the reader's convention instead, and AT3 pins the agreement so the choice cannot silently drift. Note the wider split is pre-existing and out of scope: the calibration writers use the shared helper while the ingest reader does not, so they diverge under XDG_STATE_HOME.
• "repo" is retired from GuardEventStreamLocation, and its resolveStreamPath branch is deleted. Re-introducing a repo-rooted stream is now a type error rather than something a later sweep has to find — strictly stronger than the grep AT4 asked for. roots.repoRoot stays load-bearing: it is projectStateKey's input.
• SC5 — the family's one behaviour-scoped check (scripts/lib/repo-rooted-telemetry-paths.ts).

SC5: why the check is shaped this way

Three prior sweeps were scoped by DIRECTORY (.minsky/hooks/**) while the population is defined by BEHAVIOUR — which is why none could see ask-form-lint-calibration.ts in src/adapters/shared/commands/. This scans for the behaviour: a path expression rooted at the repo whose target is telemetry, over .minsky/hooks, src, and packages.

It deliberately does NOT key on the fs write call, and that was measured rather than assumed. A write-keyed draft was tried first and found neither remaining real writer: calibration-review-cadence-detector.ts and calibration.ts both write through a one-hop helper (writeLastWarnedStore, writeFileMkdir). Computing a repo-rooted telemetry path at all is the smell.

Against the real tree it returns exactly 3 files, all genuine, all allowlisted with reasons:

• require-execution-evidence-before-merge.ts — TRACKED, not accepted: mt#4755 owns it, and four of its five streams were observed still writing into the repo tree today.
• calibration-review-cadence-detector.ts and calibration.ts — the watermark / last-warned / claims stores. A deliberate carve-out whose justification is provisional: the code says they "stay repo-rooted … still correctly gitignored," which is a minsky-repo-only property and therefore the premise mt#4748 exists to retire. Recorded in mt#4816's ## Context as a boundary case someone must decide, not close silently.

A findStaleAllowlistEntries test fails when an entry outlives the writer it excused — which is how mt#4755's entry will announce that it is ready to be removed.

What it cannot see, stated rather than implied: a text scan over single-line assignments. It misses a path assembled across statements, one built by concatenation, and one whose root arrives through a parameter. It is a floor under a class that had no mechanical check at all, not a proof of absence.

Success criteria

• SC1 — met. Decision + reasoning recorded above and in the spec's ## Decision — SC1.
• SC2 — met. The writer no longer resolves under the repo root.
• SC3 — met, and stronger than asked: verified by asserting writer and reader agree, not by reading both.
• SC4 — met. Deleted, not migrated: both records (586 bytes, 2 lines, last write 2026-08-05) were confirmed already in guard_events — same two timestamps, ingested 2026-08-12 — so migrating would have created a duplicate and leaving it would have made a 62nd orphan for mt#4777.
• SC5 — met. See above.

AT4 did not hold as literally written — reported, not narrowed

grep -c 'location: "repo"' packages/domain/src/guard-events/stream-sources.ts returns 1, not 0. The surviving match is the COMMENT recording the retirement — prose about the change satisfying a pattern written to detect its absence. The criterion's own text is amended in the spec. The property it is about is verified two ways a comment cannot satisfy: the type-level retirement, and a manifest-wide assertion.

Consumers enumerated (gate (h))

Rows unioned: Function / type signature (MISMATCH_LOG and appendMismatchRecord, both exported, one caller per copy, no external importers) and Config key / schema field (the manifest row is a schema field value the ingest reads, and it is documented in docs/, which is why the doc consumers below are in scope and would not be under the first row alone).

• .claude/hooks/verify-subagent-model.ts — regenerated via bun run src/cli.ts compile, verified to carry the change.
• .codex/hooks/verify-subagent-model.ts — currently UNTRACKED; open PR feat(mt#3854): Make .codex a compile output so the harness config stops fossilizing #3253 (mt#3854) ADDS it as a compile output. Coordinate: whichever lands second regenerates. Read from that PR's actual 188-file list, not inferred.
• ingest-runtime.ts:78 — the location === "repo" branch, removed.
• guard-calibration-stream-inventory.md — §C row, §C prose, and §D's now-false "only §C's subagent-model-mismatch remains repo-rooted."
• guard-events-schema.ts:20 — checked, no change needed: it enumerates the stream by name and makes no location claim.
• ADR-028 §D4 — second amendment. The mt#4748 amendment said "nothing writes into any working tree for this family," which was true and narrower than it read.
• hook-module-inventory.md — the census test caught that this hook newly grep-matches packages/domain (three doc-comment references) without importing it, so it joined the Divergence table; count bumped 33 → 34.

Testing

Execution evidence:

AT1 + AT3 + SC3 — the hook's own suite (6 new cases):

$ bun test --preload ./tests/setup.ts --timeout=15000 ./.minsky/hooks/verify-subagent-model.test.ts ./scripts/lib/repo-rooted-telemetry-paths.test.ts
 34 pass
 0 fail
 67 expect() calls
Ran 34 tests across 2 files. [568.00ms]

AT3 is asserted against resolveStreamPath and resolveStateDir imported from the reader, not against a hand-written expected path — a hand-written expectation is precisely what would have passed while ask-form-lint's reader and writer disagreed. Both the MINSKY_STATE_DIR-set and unset branches are asserted; asserting only the temp-dir case would pass even if the two DEFAULT branches diverged, which is the configuration nearly every real invocation runs in.

SC5 — the check against the real tree, plus 5 synthetic controls including the historical ask-form-lint writer it must flag (included in the 34 above).

Full hooks suite (ROOTS excludes this tree from the gated runner, so this is required before push):

$ bun run test:hooks
6826 pass
 0 fail
Ran 6826 tests across 185 files. [35.52s]

One earlier run of this suite showed memory-search > both silent on a trivial (affirmative) prompt failing on a 15.0s timeout. Confirmed a flake, not a regression: the diff touches nothing named memory, the file passes 72/72 in isolation, and the re-run above is clean.

Related-test selector:

$ bun scripts/run-related-tests.ts packages/domain/src/guard-events/stream-sources.ts packages/domain/src/guard-events/ingest-runtime.ts .minsky/hooks/verify-subagent-model.ts scripts/lib/repo-rooted-telemetry-paths.ts
 583 pass
 0 fail
Ran 583 tests across 25 files.

Typecheck — 0 errors across all 8 projects, validatedWorkspace the session dir (infra/ skipped with its documented reason). Lint — 0 errors, 0 warnings, 4259 files.

Negative control — control A: the manifest row and the reader's branch reverted to repo-rooted

(fail) SC3 — the stream is declared state-dir, and nothing in the manifest is repo-rooted
(fail) SC3 — the writer's file name is byte-identical to the row's relativePath
(fail) AT3 — the writer resolves to exactly the path the ingest reader computes
(fail) AT3 — they still agree when MINSKY_STATE_DIR is unset (the default branch)
(fail) AT3 — the resolved path does not depend on which project the agent is in
 21 pass
 5 fail

Negative control — control B: the writer reverted to repo-rooted, manifest and reader left intact

(fail) AT3 — the writer resolves to exactly the path the ingest reader computes
(fail) AT3 — they still agree when MINSKY_STATE_DIR is unset (the default branch)
(fail) AT1 — appending writes under the state dir and leaves no file in the repo tree
 23 pass
 3 fail

Why two controls rather than one full revert, stated because the difference matters (mt#4512). A single full revert of all three production files was attempted FIRST and produced 0 pass / 1 fail — the whole test FILE failed to load, because the revert removes an exported symbol the new tests import. A load failure is not evidence the assertions work, so that control was discarded rather than reported. The two controls above isolate the two halves: A reverts the reader/manifest (AT1 correctly stays green — it asserts the writer), B reverts the writer. Together every new assertion is shown capable of failing, for its own reason.

Deploy verification

Ran the predicate over the actual changed files rather than recalling the pattern set:

true   packages/domain/src/guard-events/stream-sources.ts
true   packages/domain/src/guard-events/ingest-runtime.ts
false  .claude/hooks/verify-subagent-model.ts
false  .minsky/hooks/verify-subagent-model.ts
false  scripts/lib/repo-rooted-telemetry-paths.ts
false  docs/architecture/*.md

So this IS deploy surface, and no [no-deploy-impact] claim is made here or in any commit message. Post-merge I will call deployment_wait-for-latest with notBefore = the merge timestamp and expectCommitSha = the merge SHA, read buildIdentity, and on indeterminate correlate the deploy workflow run's head_sha against the merge commit rather than treating a status code as identity.

One post-merge re-check beyond the deploy: the OLD generated hook is still live in main until this merges, so an Agent dispatch with a model mismatch in that window could recreate the repo-tree file SC4 removed. Mismatches run ~2 per 26 days, so this is unlikely; I will re-check the path after merge.

[minsky-reviewer]

Minsky Reviewer Status

Verdict: APPROVED — no blocking findings
Review: View review
Model: openai/gpt-5 | Tokens: 553K prompt, 9K completion | Duration: 144s
Mode: normal

Commands

• /review — request a fresh review

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Overall the PR is coherent: it re-roots the last repo-rooted telemetry writer, removes the "repo" location from the manifest types, updates the reader accordingly, adds a behaviour-scoped safeguard, and updates docs. Tests assert writer/reader agreement and write isolation. However, I found one blocking issue: the generated .claude/hooks/verify-subagent-model.ts callsite invokes appendMismatchRecord(decision.record) without passing env, after the signature changed to (record, env). This risks losing the intended testability seam and parity with the reader; please pass process.env explicitly and regenerate. Two non-blocking concerns on the SC5 scanner: the regex is brittle (misses namespaced/aliased/multi-line join/resolve uses) and the telemetry-target heuristic can false-positive on substrings like catalogPath. Consider hardening or moving to a small AST. Spec criteria SC1/SC2/SC3/SC5 appear met from the diff; SC4 (file deletion in user work trees) is out-of-repo and unverifiable here. If you address the blocking callsite and consider the scan robustness nits, this will be ready to merge.

Findings

• [BLOCKING] .claude/hooks/verify-subagent-model.ts:241 — Generated Claude hook diverges from source signature change; appendMismatchRecord callsite not updated to pass env
  In .claude/hooks/verify-subagent-model.ts:312, the callsite was changed from appendMismatchRecord(input.cwd, decision.record) to appendMismatchRecord(decision.record). However, appendMismatchRecord’s signature changed from (cwd: string, record) to (record, env = process.env). The .claude/ script does not inject an env and cannot import the domain reader, so tests that depend on passing a temp MINSKY_STATE_DIR will not be able to redirect the write when invoking the generated hook directly. More importantly, the implementation here still imports homedir and builds the path itself, while the ingest reader uses resolveStateDir. To keep parity and to support test redirection, the generated hook should forward process.env explicitly: appendMismatchRecord(decision.record, process.env). Please update both .claude/hooks/verify-subagent-model.ts and .minsky/hooks/verify-subagent-model.ts callsites consistently (the .minsky/ one is correct already) and regenerate the compiled outputs.

• [NON-BLOCKING] scripts/lib/repo-rooted-telemetry-paths.ts:76 — Behaviour-scan regex is brittle and can miss common shapes (aliases, namespaced joins, multi-line calls)
  PATH_ASSIGNMENT only matches single-line join(...)/resolve(...) calls with direct identifiers, e.g., const p = join(repoRoot, ...). It will miss common variants:

• Namespaced calls (path.join(...)) or aliased imports (import { join as j } from 'path').

• Multi-line argument lists (join(\n repoRoot,\n ...\n)), which are routine in this codebase.

• Template-literal roots (${repoRoot}) or wrapper helpers around join/resolve.

You’ve acknowledged some blind spots in the comments, but these specific missed shapes are easy to hit in our tree and would yield false negatives while giving a green test. Consider expanding detection to include namespaced/aliased joins (e.g., /\b(?:join|resolve)|\b\w+\.join\(/) and multi-line matches ([\s\S]*?\); with minimal lookbehind), or switching to a small AST parse for assignments. Given this is our only mechanical floor for SC5, robustness here matters.

• [NON-BLOCKING] scripts/lib/repo-rooted-telemetry-paths.ts:101 — Telemetry-target heuristic can false-positive on unrelated names like catalogPath
TELEMETRY_TARGET uses /log|calibration|evaluation|baseline|watermark|claims|mismatch|observations|\.jsonl/i and is matched against ${name} ${rest}. This will match identifiers containing log as a substring (e.g., catalogPath, loggerConfig) or arbitrary JSONL filenames unrelated to telemetry. You partially mitigate with test allowlist, but this can still raise noise in future refactors and make real regressions harder to see. Consider tightening:

• Use word boundaries for log (e.g., /\blog\b/) and anchor to filename segments (e.g., /[\w/-]+\.(jsonl|json)\b/).

• Prefer a denylist of known non-telemetry patterns (e.g., Dockerfile, config.yaml) or, better, enumerate telemetry stems from the manifest to cross-check.

This is advisory; not blocking, but worth hardening since this is now the only behaviour-scoped guard.

Spec verification

Criterion	Status	Evidence
SC1 — A decision is recorded between flat and project-keyed for the special family, with its reason, before the path is changed.	Met	docs/architecture/adr-028-guard-hook-dispatcher-consolidation.md:483-500 records the mt#4816 amendment; docs/architecture/guard-calibration-stream-inventory.md:186-206 explains the flat decision and rationale. Code reflects flat state-dir in packages/domain/src/guard-events/stream-sources.ts:305-315.
SC2 — verify-subagent-model.ts no longer resolves its log under the repo root.	Met	.minsky/hooks/verify-subagent-model.ts:248-273 now derives the path via getMismatchLogPath() -> state dir using MINSKY_STATE_DIR/homedir; findRepoRoot and resolve(repoRoot, ...) were removed. The same change exists in .claude/hooks/verify-subagent-model.ts:241-268.
SC3 — stream-sources.ts has no location: "repo" rows left, and the writer resolves to the same path resolveStreamPath computes for that row. Verify by asserting the two agree, not by reading both.	Met	packages/domain/src/guard-events/stream-sources.ts removes "repo" from GuardEventStreamLocation (lines ~53-67) and declares subagent-model-mismatch as location: "state-dir" with relativePath: "subagent-model-mismatch.jsonl" (lines 305-315). Tests in .minsky/hooks/verify-subagent-model.test.ts:308-358 assert equality against resolveStreamPath from the reader for both env-set and default branches.
SC4 — The existing repo-tree file is dispositioned (migrated or deleted with a reason), not orphaned.	Unverifiable	The artifact (existing files under managed projects' .minsky/) lives outside this repo. The PR body claims both records are already ingested and should be deleted, but no in-repo code can effect that deletion. Per Out-of-repo references policy, this cannot be verified from the diff.
SC5 — A check asserts SC2 mechanically. Coordinate with mt#4811/mt#4755/mt#4778 on ONE behaviour-scoped check rather than a fourth directory-scoped one — see the note below.	Met	scripts/lib/repo-rooted-telemetry-paths.ts adds a behaviour-scoped scan that flags repo-rooted telemetry path expressions; scripts/lib/repo-rooted-telemetry-paths.test.ts runs it against the real tree, verifies allowlist hygiene, and includes synthetic controls (including the historical ask-form-lint case).

Adoption sweep

Symbol	Kind	Consumers found	Classification	Notes
.minsky/hooks/verify-subagent-model.getMismatchLogPath	function	.minsky/hooks/verify-subagent-model.test.ts:319 — asserts equality with reader’s resolveStreamPath	Adopted	New exported helper introduced to expose the computed path for tests; only test consumer found, which is sufficient.

Documentation impact

• updated-in-pr — This PR changes public behavior (stream location semantics) and updates docs accordingly: docs/architecture/adr-028-guard-hook-dispatcher-consolidation.md adds the mt#4816 amendment; docs/architecture/guard-calibration-stream-inventory.md updates the §C row and surrounding prose; docs/architecture/hook-module-inventory.md updates the divergence count to 34.
  Affected: docs/architecture/adr-028-guard-hook-dispatcher-consolidation.md, docs/architecture/guard-calibration-stream-inventory.md, docs/architecture/hook-module-inventory.md

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

The fix largely implements the task: the writer now targets the state dir, the manifest retires the "repo" location and declares the stream flat, tests assert writer/reader agreement under both env branches, a behaviour-scoped guard is added, and docs are updated. However, the round-1 BLOCKING issue persists: both the generated .claude/ hook and the source .minsky/ hook call appendMismatchRecord(decision.record) without forwarding process.env after the function’s signature was changed to (record, env = process.env). Please pass process.env explicitly at both callsites and regenerate compiled outputs. The SC5 robustness nits from R1 remain advisory. Once the explicit env forwarding is fixed, this should be ready to merge.

Findings

• [BLOCKING] .claude/hooks/verify-subagent-model.ts:312 — Generated Claude hook still does not pass process.env to appendMismatchRecord after signature change
  appendMismatchRecord was refactored to take (record, env = process.env) so tests can redirect writes by injecting a temp MINSKY_STATE_DIR and to keep parity with the ingest reader’s resolveStateDir. At .claude/hooks/verify-subagent-model.ts:312, the call remains appendMismatchRecord(decision.record) without forwarding process.env. While the default parameter covers the runtime case, this loses the intended explicitness and prevents callers of the generated hook from substituting the env via a single seam (the hook should consistently forward its own process.env).

Please change this call to appendMismatchRecord(decision.record, process.env) and regenerate the compiled outputs. This was the R1 blocker and remains unresolved here.

• [BLOCKING] .minsky/hooks/verify-subagent-model.ts:309 — Hook callsite no longer forwards process.env to appendMismatchRecord after signature change
  appendMismatchRecord now takes (record, env = process.env) to allow tests to inject a temp MINSKY_STATE_DIR and to keep parity with the ingest reader’s resolveStateDir. At .minsky/hooks/verify-subagent-model.ts:309, the call was changed to appendMismatchRecord(decision.record) and no longer passes process.env. While the default parameter yields the same runtime env, keeping the explicit forward ensures callers of the hook maintain a single seam to substitute env during direct execution and avoids future refactors accidentally changing the default.

Please change this call to appendMismatchRecord(decision.record, process.env) (matching the intended pattern used elsewhere like fire-log.ts) and regenerate compiled outputs so .claude/ mirrors it.

Spec verification

Criterion	Status	Evidence
SC1 — A decision is recorded between flat and project-keyed for the special family, with its reason, before the path is changed.	Met	docs/architecture/adr-028-guard-hook-dispatcher-consolidation.md:483-500 adds the mt#4816 amendment; docs/architecture/guard-calibration-stream-inventory.md:186-206 records the flat decision and rationale. Code reflects flat state-dir in packages/domain/src/guard-events/stream-sources.ts:287-315.
SC2 — verify-subagent-model.ts no longer resolves its log under the repo root.	Met	.minsky/hooks/verify-subagent-model.ts:236-273 replaces repo-root resolution with state-dir via getMismatchLogPath(); findRepoRoot/resolve(repoRoot, …) removed. Same pattern mirrored in .claude/hooks/verify-subagent-model.ts:239-268.
SC3 — stream-sources.ts has no location: "repo" rows left, and the writer resolves to the same path resolveStreamPath computes for that row. Verify by asserting the two agree, not by reading both.	Met	packages/domain/src/guard-events/stream-sources.ts:61-78 retires "repo" from GuardEventStreamLocation; subagent-model-mismatch declared location: "state-dir" with relativePath: "subagent-model-mismatch.jsonl" at 287-297. Tests in .minsky/hooks/verify-subagent-model.test.ts:308-358 assert equality with the reader’s resolveStreamPath under both env-set and default branches.
SC4 — The existing repo-tree file is dispositioned (migrated or deleted with a reason), not orphaned.	Unverifiable	Disposition of existing files under managed projects’ .minsky/ is outside the repo. The PR text claims deletion is safe (already ingested), but no in-repo artifact can verify or perform that. Out-of-repo per policy.
SC5 — A check asserts SC2 mechanically. Coordinate with mt#4811/mt#4755/mt#4778 on ONE behaviour-scoped check rather than a fourth directory-scoped one — see the note below.	Met	scripts/lib/repo-rooted-telemetry-paths.ts adds a behaviour-scoped scan; scripts/lib/repo-rooted-telemetry-paths.test.ts runs it against the real tree, validates allowlist hygiene, and includes synthetic controls (including the historical ask-form-lint writer).

Documentation impact

• updated-in-pr — This PR changes the stream’s location semantics and retires the "repo" location kind, and updates docs accordingly: docs/architecture/adr-028-guard-hook-dispatcher-consolidation.md adds a second amendment for mt#4816; docs/architecture/guard-calibration-stream-inventory.md updates §C/§D prose and the path; docs/architecture/hook-module-inventory.md updates the divergence count to include verify-subagent-model.ts.
  Affected: docs/architecture/adr-028-guard-hook-dispatcher-consolidation.md, docs/architecture/guard-calibration-stream-inventory.md, docs/architecture/hook-module-inventory.md

[edobry]

R1 response

Both NON-BLOCKING findings were real and are fixed in a5cbbb3c. Widening the scan to namespaced (path.join) and multi-line calls immediately surfaced a file the first draft could not see: src/cockpit/ask-state-cache.ts:304, path.join(repoRoot, WATERMARK_STORE_RELPATH) — a reader of the same watermark store and the third module in that carve-out family. Allowlisted with its reason and recorded in the task spec. The catalogPath fix is segmentation rather than word boundaries, because \blog\b matches neither logPath nor mainLog — the names that must match; one test pins the trap cases and the must-match cases together so they cannot drift.

The BLOCKING finding does not hold, and the review's own text contains the contradiction. It asks to "update both .claude/hooks/verify-subagent-model.ts and .minsky/hooks/verify-subagent-model.ts callsites consistently (the .minsky/ one is correct already)" — but the two callsites are byte-identical:

.minsky/hooks/verify-subagent-model.ts:312:    appendMismatchRecord(decision.record);
.claude/hooks/verify-subagent-model.ts:315:    appendMismatchRecord(decision.record);

.claude/hooks/** is generated from .minsky/hooks/** by bun run src/cli.ts compile, which was run and whose output is in this PR. There is no divergence between them to fix, so the finding's premise is not met.

The stated rationale does not hold either: appendMismatchRecord(record, env = process.env) has a default, so a subprocess invocation carrying MINSKY_STATE_DIR in its environment already redirects the write. Passing process.env explicitly at the callsite is a literal no-op — the same object, the same value — and would obscure that the default is what production uses.

On the adjacent observation, which is fair: the hook does build the path itself rather than calling the reader's resolveStateDir, because packages/domain/src/guard-events/ingest-runtime.ts pulls drizzle and DB-schema imports into every hook's cold start — the same no-cross-dependency convention calibration-review-cadence-detector.ts documents for projectStateKey. That is exactly why AT3 asserts the two resolvers agree, against the reader's own function, in both the MINSKY_STATE_DIR-set and default branches. Duplication pinned by an assertion, rather than duplication left to drift, is the mitigation mt#4811 arrived at for this same shape.

On SC4 — "Unverifiable": agreed that no in-repo code effects the deletion, and the classification is right from the diff alone. Recording the evidence for the audit trail: the file held 2 records (586 bytes, last write 2026-08-05), both confirmed present in guard_events with matching timestamps and non-null project_id before removal, and the removal was verified by re-stat. Deleting rather than migrating is what keeps it from becoming a 62nd orphan for mt#4777.

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

The prior BLOCKING findings on the verify-subagent-model hook have been addressed: both .minsky and generated .claude copies now write to the state dir (MINSKY_STATE_DIR/~/.local/state/minsky), not the repo. The ingest manifest removes the "repo" location entirely (type-level enforcement), and the writer/reader path agreement is pinned by tests. The PR also introduces a behavior-scoped scanner and tests to assert no unallowlisted repo-rooted telemetry paths remain; the allowlist carries explicit reasons. I spot-verified stream-sources.ts and ingest-runtime.ts for consistency. Docs were updated to reflect the change. One criterion (SC4) concerns out-of-repo file disposition and is unverifiable from the diff, but does not block merge. I find no new critical defects introduced by this fix. Approving.

Spec verification

Criterion	Status	Evidence
SC1 — A decision is recorded between flat and project-keyed for the special family, with its reason, before the path is changed.	Met	packages/domain/src/guard-events/stream-sources.ts:20-45, 64-72 — module header documents mt#4816 retiring "repo" and explicitly records that subagent-model-mismatch is now state-dir and flat (not project-keyed), with rationale tied to attachProjectIds attribution. Also reflected inline at :147-160 (SPECIAL_STREAMS preface).
SC2 — verify-subagent-model.ts no longer resolves its log under the repo root.	Met	.minsky/hooks/verify-subagent-model.ts:147-179, 181-186 — getSubagentModelStateDir derives from MINSKY_STATE_DIR or ~/.local/state/minsky, and getMismatchLogPath/appendMismatchRecord write under that state dir. No repo-rooted findRepoRoot/workspacePath joins remain.
SC3 — stream-sources.ts has no location: "repo" rows left, and the writer resolves to the same path resolveStreamPath computes for that row. Verify by asserting the two agree, not by reading both.	Met	packages/domain/src/guard-events/stream-sources.ts:64-72 — GuardEventStreamLocation is now the single-member union "state-dir" (type-level removal of "repo"). SPECIAL_STREAMS entry at :147-160 sets relativePath: "subagent-model-mismatch.jsonl". The hook exports MISMATCH_LOG = "subagent-model-mismatch.jsonl" and computes its path via state dir (see .minsky/hooks/verify-subagent-model.ts:92-101, 171-186), pinning writer/manifest agreement.
SC4 — The existing repo-tree file is dispositioned (migrated or deleted with a reason), not orphaned.	Unverifiable	The disposition concerns an on-disk file under a developer/operator workspace (e.g., .minsky/subagent-model-mismatch.jsonl) rather than an in-repo artifact. This review cannot observe filesystem state outside the repository. No in-repo migration script is part of the current diff segment to assert against.
SC5 — A check asserts SC2 mechanically. Coordinate with mt#4811/mt#4755/mt#4778 on ONE behaviour-scoped check rather than a fourth directory-scoped one — see the note below.	Met	scripts/lib/repo-rooted-telemetry-paths.ts (new) implements a behaviour-scoped scan that detects repo-rooted telemetry path constructions; scripts/lib/repo-rooted-telemetry-paths.test.ts (new) runs it over the real tree plus synthetic controls, asserts zero unallowlisted hits, validates allowlist reasons, and guards namespaced and multi-line joins.

Adoption sweep

Symbol	Kind	Consumers found	Classification	Notes
scripts/lib/repo-rooted-telemetry-paths.findRepoRootedTelemetryPaths	function	scripts/lib/repo-rooted-telemetry-paths.test.ts:21-33 — imported and exercised in multiple tests	Adopted	Utility is internal to scripts/, consumed by its dedicated test suite; no additional wiring required.

Documentation impact

• updated-in-pr — This PR changes documented behavior and updates the docs accordingly. The inventory now reflects subagent-model-mismatch as state-dir/flat and removes the last location: "repo" exception. Verified in docs/architecture/guard-calibration-stream-inventory.md — §C/SPECIAL stream preface and section I additions align with the code changes in packages/domain/src/guard-events/stream-sources.ts.
  Affected: docs/architecture/guard-calibration-stream-inventory.md

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

Verification complete. The prior BLOCKING issues are addressed: both .minsky and generated .claude hooks now write to the state dir, not the repo; the ingest manifest retires the "repo" location entirely with corresponding removal of the branch in resolveStreamPath; and tests pin writer/reader path agreement. A behavior-scoped scanner plus tests enforce no unallowlisted repo-rooted telemetry paths remain, with justified allowlist entries. Documentation is updated coherently (ADR-028 amendment and stream inventory). I find no new critical defects introduced by this change. SC4’s out-of-repo file disposition is inherently unverifiable from the diff and is recorded as such. Approving.

Spec verification

Criterion	Status	Evidence
SC1 — A decision is recorded between flat and project-keyed for the special family, with its reason, before the path is changed.	Met	Decision and rationale are recorded in docs and code comments: docs/architecture/adr-028-guard-hook-dispatcher-consolidation.md:+483-500 adds the mt#4816 amendment stating "state-dir and flat" with reasoning; packages/domain/src/guard-events/stream-sources.ts:15-23 module header notes mt#4816 finishes it; .minsky/hooks/verify-subagent-model.ts:149-171 documents the flat decision and the ingestion-based project_id rationale.
SC2 — verify-subagent-model.ts no longer resolves its log under the repo root.	Met	.minsky/hooks/verify-subagent-model.ts:69-89 switches MISMATCH_LOG to a bare file name and introduces getSubagentModelStateDir/getMismatchLogPath joining the state dir; appendMismatchRecord now uses getMismatchLogPath (lines ~171-190) and no longer calls findRepoRoot/resolve.
SC3 — stream-sources.ts has no location: "repo" rows left, and the writer resolves to the same path resolveStreamPath computes for that row. Verify by asserting the two agree, not by reading both.	Met	packages/domain/src/guard-events/stream-sources.ts:47-58 retires "repo" from GuardEventStreamLocation; the SPECIAL stream row at :110-123 is location: "state-dir" with relativePath "subagent-model-mismatch.jsonl". The hook test .minsky/hooks/verify-subagent-model.test.ts:41-100 imports resolveStreamPath/resolveStateDir and asserts getMismatchLogPath(env) === resolveStreamPath(row, ...), including with and without MINSKY_STATE_DIR, and that all manifest rows are state-dir.
SC4 — The existing repo-tree file is dispositioned (migrated or deleted with a reason), not orphaned.	Unverifiable	The disposition concerns a file under a managed project's working tree (out-of-repo). The diff updates code and docs but cannot carry an external filesystem deletion/migration; no in-repo artifact can prove it. Per Out-of-repo references policy, this is not verifiable from the diff.
SC5 — A behaviour-scoped check asserts SC2 mechanically (one check coordinated with sibling tasks).	Met	New check and tests added at scripts/lib/repo-rooted-telemetry-paths.ts and scripts/lib/repo-rooted-telemetry-paths.test.ts. The test exercises the real tree and synthetic controls and enforces no unallowlisted repo-rooted telemetry paths exist; allowlist entries are justified and kept fresh.

Documentation impact

• updated-in-pr — Documentation updated alongside code: docs/architecture/adr-028-guard-hook-dispatcher-consolidation.md adds a second amendment (mt#4816) clarifying the move to state-dir and retirement of the "repo" location; docs/architecture/guard-calibration-stream-inventory.md updates §C and §D to reflect subagent-model-mismatch now being flat under the state dir and notes that no streams are repo-rooted anymore.
  Affected: docs/architecture/adr-028-guard-hook-dispatcher-consolidation.md, docs/architecture/guard-calibration-stream-inventory.md, docs/architecture/hook-module-inventory.md

[minsky-reviewer]

Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2

---

The PR resolves the prior BLOCKING issues: the subagent-model mismatch writer now targets the state dir, the ingest manifest retires the "repo" location (type-level enforcement), resolveStreamPath’s repo branch is removed, tests pin writer/reader agreement and manifest-wide compliance, and docs are updated. A behavior-scoped scanner with tests enforces SC5. However, a new discrepancy introduced by this change breaks writer/reader agreement when MINSKY_STATE_DIR is set to an empty/whitespace value: the hook’s getSubagentModelStateDir returns the raw env without trimming, while the reader’s resolveStateDir treats empty-as-unset. This can cause the writer to write a relative path into CWD, violating SC2. Please mirror the reader’s empty/whitespace handling in both .minsky and .claude copies and add a test asserting agreement for MINSKY_STATE_DIR="". With that fixed, the PR will be ready to merge.

Findings

• [BLOCKING] .minsky/hooks/verify-subagent-model.ts:245 — getSubagentModelStateDir does not trim MINSKY_STATE_DIR, diverging from reader; empty-string env causes writer/reader disagreement and can write to CWD
  The new getSubagentModelStateDir(env) returns env["MINSKY_STATE_DIR"] verbatim when set, without trimming or validating non-empty. The ingest reader’s resolveStateDir (packages/domain/src/guard-events/ingest-runtime.ts:36-41) explicitly treats empty/whitespace as unset and falls back to ~/.local/state/minsky. With the current hook, setting MINSKY_STATE_DIR="" (or whitespace) yields join("", MISMATCH_LOG) ⇒ a relative subagent-model-mismatch.jsonl written to the process CWD, violating SC2 and breaking the writer/reader path-agreement the tests aim to pin. This discrepancy was introduced by this PR and is untested (tests cover set/non-set, not empty-set). Fix: mirror the reader’s logic — trim and treat empty as unset (fall back to homedir) in getSubagentModelStateDir. Update the .claude/ mirror and add a test asserting agreement when MINSKY_STATE_DIR is set to an empty string.

Spec verification

Criterion	Status	Evidence
SC1 — A decision is recorded between flat and project-keyed for the special family, with its reason, before the path is changed.	Met	Decision and rationale are recorded in code comments and docs: see packages/domain/src/guard-events/stream-sources.ts module header additions (mt#4816 decision notes) and docs/architecture/adr-028-guard-hook-dispatcher-consolidation.md (second amendment block added by this PR) describing FLAT for special with justification (project attribution resolved from session_id).
SC2 — verify-subagent-model.ts no longer resolves its log under the repo root.	Met	.minsky/hooks/verify-subagent-model.ts and its generated .claude/ mirror remove findRepoRoot/resolve(...) and introduce getSubagentModelStateDir + getMismatchLogPath to write under the state dir, keyed by MINSKY_STATE_DIR with homedir fallback. Callsites updated to appendMismatchRecord(decision.record) which uses the state-dir path.
SC3 — stream-sources.ts has no location: "repo" rows left, and the writer resolves to the same path resolveStreamPath computes for that row. Verify by asserting the two agree, not by reading both.	Met	packages/domain/src/guard-events/stream-sources.ts changes GuardEventStreamLocation to only "state-dir" and updates the subagent-model-mismatch row to location: "state-dir" with relativePath: "subagent-model-mismatch.jsonl". Tests in .minsky/hooks/verify-subagent-model.test.ts import resolveStreamPath/resolveStateDir and assert equality with getMismatchLogPath (AT3), and also assert the manifest has no non-state-dir rows.
SC4 — The existing repo-tree file is dispositioned (migrated or deleted with a reason), not orphaned.	Unverifiable	Disposition of any already-written file in a developer's working tree is out-of-repo. The PR updates code and docs but any on-disk deletion/migration cannot be verified from the diff; the task spec itself notes this as deploy/runtime evidence.
SC5 — A check asserts SC2 mechanically. Coordinate with mt#4811/mt#4755/mt#4778 on ONE behaviour-scoped check rather than a fourth directory-scoped one.	Met	New behaviour-scoped scanner added at scripts/lib/repo-rooted-telemetry-paths.ts with comprehensive tests in scripts/lib/repo-rooted-telemetry-paths.test.ts. It scans .minsky/hooks, src, and packages for repo-rooted telemetry path expressions, carries an allowlist with reasons, asserts no unallowlisted matches, and asserts allowlist entries remain live.

Adoption sweep

Symbol	Kind	Consumers found	Classification	Notes
.minsky/hooks/verify-subagent-model.getMismatchLogPath	function	.minsky/hooks/verify-subagent-model.test.ts: imports getMismatchLogPath and asserts equality with resolveStreamPath (AT3)	Adopted	New exported helper introduced by this PR; exercised by the hook’s own test to pin writer/reader agreement.

Documentation impact

• updated-in-pr — This PR changes documented behavior (retiring repo-rooted telemetry and making subagent-model-mismatch state-dir/flat) and updates the docs accordingly: docs/architecture/adr-028-guard-hook-dispatcher-consolidation.md adds a second amendment recording the change, and docs/architecture/guard-calibration-stream-inventory.md updates the stream’s location and explanatory prose.
  Affected: docs/architecture/adr-028-guard-hook-dispatcher-consolidation.md, docs/architecture/guard-calibration-stream-inventory.md, docs/architecture/hook-module-inventory.md