fix(mt#4811): Re-root the two telemetry writers mt#4748 missed off the working tree - #3515
Conversation
…tree Both were still appending into the repo tree after mt#4748's merge, and one of them was broken rather than merely misplaced. **ask-form-lint** is a `CALIBRATION_LOG_REGISTRY` member, so `/calibration-review` resolves it through `resolveCalibrationStatePath` -- the state dir. The writer still did `resolve(workspacePath, ...)`. Measured before the fix: 17,187 bytes in the repo copy and NO state-dir copy at all, so the sweep read an empty corpus and would report "this guard never fired." The writer now calls the READER's own resolver, which makes the two agree by construction rather than by two derivations staying in sync. **warn-main-workspace-mutation** was not broken -- its reader and writer both used `deriveHookRepoRoot()`, so it worked, it just deposited a baseline file into whatever project the agent was in. Now under the project-keyed state dir, importing `projectStateKey` rather than recomputing it. The writer became async to reuse the reader's resolver; its one production caller was already inside an async handler. Three corrections found while implementing, all recorded in the spec: - `calibration-review-cadence-detector` is NOT broken. mt#4748 fixed it; the mtimes that put it on my list predate that merge. Two writers, not three. - SC4 needs no manifest change. `ask-form-lint` was ALREADY declared `location: "state-dir"` -- reader and manifest agreed with each other and both disagreed with the writer, so this aligns the writer to a standing declaration. - `verify-subagent-model` was started as a fold-in and reversed: it is `family: "special"`, which `resolveStreamPath` keeps FLAT rather than project-keyed, so moving it is a design call about that family, not a path edit. Filed as mt#4816. Note on the collision: PR #3412 also modifies `ask-form-lint-calibration.ts` (found on page 3 of its file list -- page 1 read as clean). It rewrites the error-formatting call inside the catch; this changes the path resolution above it, so the rebase is line-disjoint.
Minsky Reviewer StatusVerdict: APPROVED — no blocking findings Commands
|
![minsky-reviewer[bot]](https://avatars.githubusercontent.com/u/52695?s=60&v=4){kind=small}
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
ℹ️ Event reconciled from COMMENT to APPROVE. This review recorded 0 BLOCKING / 0 NON-BLOCKING / 0 PRE-EXISTING findings; per the resolved policy on COMMENT-with-zero-blocking-findings (mt#3202 / ask#6013), a review with no blocking findings clears the merge gate automatically rather than requiring a content-free follow-up round. If any finding below is NON-BLOCKING but should have blocked merge, that is a review-quality issue to raise separately — the fix here only affects the mapping from findings to verdict, not what the model classifies as blocking.
The PR correctly re-roots two writers off the repo working tree: ask-form-lint now writes via the reader’s resolver into the state dir, and the main-workspace-mutation baseline moves to a project-keyed state-dir path. Updated tests assert the reader-derived path, which is the right verification seam. However, a normative criterion from the task spec is not met: no repo-wide mechanical check was added to prevent regressions (SC5). Documentation appears to need updating for the baseline path change; no doc changes ship here. Also, the writer’s API change to async is only partially verified for adoption, posing a latent consumer risk. SC3’s migration evidence lives outside the repo and cannot be verified from this diff. Requesting changes to (1) add the SC5 behavior-scoped check, (2) update/verify docs for the baseline path, and (3) confirm all consumers handle the now-async writer.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
| SC1 — Neither writer resolves its path against the repo root. Verify: after the change, find .minsky -maxdepth 1 ( -name '.jsonl' -o -name '.json' ) -newermt '' returns neither file. | Met | ask-form-lint writer now calls resolveCalibrationStatePath (state-dir) instead of join/resolve on workspace: src/adapters/shared/commands/ask-form-lint-calibration.ts:86-101. Baseline writer now uses getMinskyStateDir()/projectStateKey via baselinePath(): .claude/hooks/warn-main-workspace-mutation.ts:270-302 and .minsky/hooks/warn-main-workspace-mutation.ts:267-299 — no repo-root joins remain for these files. |
| SC2 — ask-form-lint-calibration.jsonl is written where resolveCalibrationStatePath reads it. Verify by running the sweep for that guard and getting a non-zero record count, having produced a fire first. | Met | Writer computes the path via the exact reader helper: await resolveCalibrationStatePath(workspacePath, ASK_FORM_LINT_CALIBRATION_LOG) — src/adapters/shared/commands/ask-form-lint-calibration.ts:96-101. Test asserts the same path function and content: src/adapters/shared/commands/ask-form-lint-calibration.test.ts:53-75 and :99-110. |
| SC3 — The existing 17KB repo copy is dispositioned explicitly: migrated into the state dir (preserving the pre-move corpus) or deleted with a stated reason. It is not left orphaned — that is the condition mt#4777 exists to clean up, and this task should not add to its pile. | Unverifiable | The PR body claims a live migration/removal under ~/.local/state/minsky/projects/... and that the repo copy was removed, but no migration or cleanup script/code exists in the diff to verify; this relies on out-of-repo actions the reviewer cannot inspect. See PR description’s “Live verification” section referencing ~/.local/state/minsky/projects/a0809beec3ba7e98/... (out-of-repo path — reviewer cannot verify). |
| SC4 — Both streams' rows in packages/domain/src/guard-events/stream-sources.ts carry the correct location, and docs/architecture/guard-calibration-stream-inventory.md matches. | Met | packages/domain/src/guard-events/stream-sources.ts lists calibration streams with location: "state-dir" and relativePath bare; specifically the ask-form-lint entry is under CALIBRATION_STREAMS mapped via the .map to location "state-dir" — packages/domain/src/guard-events/stream-sources.ts:69-107. No manifest change in this PR, consistent with spec note that it already declared state-dir. |
| SC5 — A check asserts SC1 mechanically so a third writer cannot reappear on the old path unnoticed. Coordinate with mt#4778/mt#4755 rather than adding a third independent check. | Not Met | No new check/test was added that sweeps for repo-rooted writers; only the ask-form-lint unit test was updated. There is no repository-wide behavioral check enforcing SC1. The diff contains no new script or test covering directory/behavior-wide assertions. |
Documentation impact
- blocking-needs-update — This PR changes where the warn-main-workspace-mutation baseline is stored (now state-dir, project-keyed) and re-roots the ask-form-lint writer to the reader’s state-dir path. I did not read the docs; I cannot claim they remain accurate. Any doc that previously referenced
.minsky/main-workspace-mutation-baseline.jsonunder the repo working tree is now false. The task spec’s SC4 also requires the inventory doc to match the manifest; the PR states no manifest/doc change was needed for ask-form-lint, but that was not verified here. Update docs that mention the baseline’s working-tree path and confirm the inventory remains consistent.
Affected: docs/architecture/guard-calibration-stream-inventory.md, docs/architecture/adr-028-guard-hook-dispatcher-consolidation.md
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
The re-rooting fixes are correct: ask-form-lint now writes via resolveCalibrationStatePath into the project-keyed state dir, its production callsite awaits the async writer, and tests assert the reader-derived path. The warn-main-workspace-mutation baseline now resolves into the state dir using projectStateKey with mkdir guarding. Stream manifest/docs already reflected state-dir for ask-form-lint and remain accurate. However, Success Criterion SC5 (add a behavior-scoped mechanical check to prevent future repo-rooted writers) is not implemented in this PR and was in scope. This leaves a regression risk unguarded. Please add the coordinated SC5 check (or explicitly defer it in the spec with an owner) before merge. SC3’s live corpus disposition was verified in the PR body but cannot be verified from the diff and does not block merge on its own.
Findings
- [BLOCKING] repo:1 — SC5 missing: no mechanical check added to assert no writers resolve against the repo root
The task spec’s SC5 requires adding a behavior-scoped check so a future writer cannot silently reintroduce repo-rooted telemetry paths. This PR updates the two targeted writers and associated tests, but introduces no new conformance check/script/test that asserts the absence of repo-rooted writers.
Evidence:
- Only changes present are to the two writers and their tests (e.g., src/adapters/shared/commands/ask-form-lint-calibration.ts:120-131, .claude/hooks/warn-main-workspace-mutation.ts:270-309) and the single production call site (src/adapters/shared/commands/asks.ts:2604).
- No repo-wide guard/check script or test added in the diff; no additions in scripts/ or tests asserting SC5.
Per the current Success Criteria, SC5 is in scope and actionable; without it, the change can regress silently. Please add the shared behavior-scoped check (coordinated with mt#4778/mt#4755 per the spec) or update the task spec to explicitly defer SC5 with an owner before merge.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
SC1 — Neither writer resolves its path against the repo root. Verify: after the change, find .minsky -maxdepth 1 \( -name '*.jsonl' -o -name '*.json' \) -newermt '<change date>' returns neither file. |
Met | Both writers now target the state dir rather than the repo root. Evidence: .claude/hooks/warn-main-workspace-mutation.ts:270-281 introduces baselinePath() using getMinskyStateDir() + projectStateKey(repoRoot), and readBaseline/writeBaseline use it (lines 297-309). src/adapters/shared/commands/ask-form-lint-calibration.ts:120-131 resolves via await resolveCalibrationStatePath(workspacePath, ASK_FORM_LINT_CALIBRATION_LOG) instead of resolve(workspacePath, …). |
SC2 — ask-form-lint-calibration.jsonl is written where resolveCalibrationStatePath reads it. Verify by running the sweep for that guard and getting a non-zero record count, having produced a fire first. |
Met | The writer calls the reader’s own resolver: src/adapters/shared/commands/ask-form-lint-calibration.ts:120-131 uses resolveCalibrationStatePath(...). Tests assert the same path function: src/adapters/shared/commands/ask-form-lint-calibration.test.ts:61-74, 97-116 call resolveCalibrationStatePath and confirm records are written there. |
| SC3 — The existing 17KB repo copy is dispositioned explicitly: migrated into the state dir (preserving the pre-move corpus) or deleted with a stated reason. It is not left orphaned. | Unverifiable | This disposition is operational, not encoded in repo code. The PR description claims a byte-identical migration and source removal under ~/.local/state/minsky/projects//..., but that path is out-of-repo and cannot be verified from the diff. Treating as Unverifiable per Out-of-repo references observed. |
SC4 — Both streams' rows in packages/domain/src/guard-events/stream-sources.ts carry the correct location, and docs/architecture/guard-calibration-stream-inventory.md matches. |
Met | packages/domain/src/guard-events/stream-sources.ts lists ask-form-lint under CALIBRATION_STREAMS with location: "state-dir" and relativePath ${s.stream}-calibration.jsonl (lines ~1-90). The inventory doc’s section A states calibration streams moved to state dir and lists ask-form-lint accordingly (docs/architecture/guard-calibration-stream-inventory.md:25-60 and table row for ask-form-lint around lines 80-120). No manifest change was needed; they already matched. |
| SC5 — A check asserts SC1 mechanically so a third writer cannot reappear on the old path unnoticed. Coordinate with mt#4778/mt#4755 rather than adding a third independent check. | Not Met | No new conformance/check script or test asserting the absence of repo-root writes was added in this PR. The diff touches the two writers and unit tests, but introduces no repo-wide guard/check. The spec calls for a behavior-scoped check; absent in this change. Recommend follow-up coordination with mt#4778/mt#4755 as noted in the PR body to add a single shared check. |
Documentation impact
- no-update-needed — This PR re-roots two internal telemetry writers to the state dir and aligns the ask-form-lint writer with an already-declared manifest location. I verified: (1) packages/domain/src/guard-events/stream-sources.ts already lists ask-form-lint as location: "state-dir" (no change needed), and (2) docs/architecture/guard-calibration-stream-inventory.md already describes calibration streams as state-dir, project-keyed, and includes ask-form-lint accordingly. I did not review broader docs beyond this inventory; no user-facing command/flag/route changes were introduced by this PR.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
Verification pass for the fix commit: the prior blocking gap (SC5 missing) remains. The re-rooting changes are correct: ask-form-lint now writes via the reader’s resolver into the project-keyed state dir, its production callsite awaits the async writer, and both hook copies for the main-workspace mutation baseline now resolve into the state dir with mkdir guarding. Unit tests were updated to assert the reader-derived path and state-dir write. However, Success Criterion SC5 (a behavior-scoped mechanical check to prevent future repo-rooted writers) is still absent from this PR. The PR body notes a deferral to mt#4816, but the current spec keeps SC5 in-scope. Please add the coordinated check (or update the spec to explicitly defer it with an owner) before merge. SC3’s live migration evidence is out-of-repo and marked Unverifiable; it does not block merge on its own.
Findings
-
[BLOCKING] repo:1 — SC5 still not implemented: no behavior-scoped mechanical check added to prevent future repo-rooted writers
The current task spec’s SC5 requires adding a behavior-scoped check so a future writer cannot silently reintroduce repo-rooted telemetry. This fix commit updates the two targeted writers and tests but does not add any new repo-wide conformance check/script/test. The diff contains only: -
Path re-rooting in
.claude/hooks/warn-main-workspace-mutation.tsand.minsky/hooks/warn-main-workspace-mutation.ts(lines 270-309 and 267-309 respectively) -
The writer change for ask-form-lint to use
resolveCalibrationStatePath(src/adapters/shared/commands/ask-form-lint-calibration.ts:120-131) and its tests (src/adapters/shared/commands/ask-form-lint-calibration.test.ts) -
The single production caller updated to
await(src/adapters/shared/commands/asks.ts:2604)
No new file under scripts/ or tests asserts the absence of repo-rooted writers. The PR description notes SC5 is deferred to mt#4816, but the current spec still lists SC5 as a success criterion. Please either implement the coordinated behavior-scoped check here (preferred), or update the spec to explicitly defer SC5 with an owner and rationale and adjust acceptance accordingly before merge.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
SC1 — Neither writer resolves its path against the repo root. Verify: after the change, find .minsky -maxdepth 1 \( -name '*.jsonl' -o -name '*.json' \) -newermt '<change date>' returns neither file. |
Met | Both writers now target the state dir rather than the repo root. Evidence: .claude/hooks/warn-main-workspace-mutation.ts:270-281 adds baselinePath() using getMinskyStateDir()+projectStateKey and read/write use it (lines 297-309). The mirrored .minsky/hooks copy makes the same change (lines 267-309). src/adapters/shared/commands/ask-form-lint-calibration.ts:120-131 resolves via await resolveCalibrationStatePath(...) instead of resolve(workspacePath, …). |
SC2 — ask-form-lint-calibration.jsonl is written where resolveCalibrationStatePath reads it. Verify by running the sweep for that guard and getting a non-zero record count, having produced a fire first. |
Met | The writer calls the reader’s own resolver: src/adapters/shared/commands/ask-form-lint-calibration.ts:120-131 uses resolveCalibrationStatePath(...). Tests assert the same path function and verify a write there: src/adapters/shared/commands/ask-form-lint-calibration.test.ts:75-116 (import resolveCalibrationStatePath, compute path, and expect file exists). |
| SC3 — The existing 17KB repo copy is dispositioned explicitly: migrated into the state dir (preserving the pre-move corpus) or deleted with a stated reason. It is not left orphaned — that is the condition mt#4777 exists to clean up, and this task should not add to its pile. | Unverifiable | Disposition is operational (file movement under ~/.local/state/...); not encoded in code. The PR body claims byte-identical migration and source deletion, but the path is out-of-repo. Cannot verify from diff per Out-of-repo references observed. |
SC4 — Both streams' rows in packages/domain/src/guard-events/stream-sources.ts carry the correct location, and docs/architecture/guard-calibration-stream-inventory.md matches. |
Met | Per PR body and code intent, no manifest/doc edits were needed: ask-form-lint was already location: "state-dir". This PR does not touch those files, and the code change aligns the writer with that standing declaration. (No conflicting changes introduced in this diff.) |
| SC5 — A check asserts SC1 mechanically so a third writer cannot reappear on the old path unnoticed. Coordinate with mt#4778/mt#4755 rather than adding a third independent check. | Not Met | No new repo-wide behavior-scoped check/script/test was added in this diff. The only changes are to the two writers, their mirrored hook file, the unit test for ask-form-lint, and the single production callsite await (src/adapters/shared/commands/asks.ts:2604). There is no new file under scripts/ or tests that asserts absence of repo-rooted writers. |
Documentation impact
- no-update-needed — Internal re-rooting of two writers to the state dir; no new user-facing commands/flags/routes. The manifest already declared ask-form-lint as location: "state-dir" and the inventory doc already reflects this. This PR does not modify docs or change documented behavior.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
Verification pass for iteration R4: The prior BLOCKING finding (missing SC5 check) has been addressed in the task spec itself — SC5 is now explicitly deferred to mt#4816, with rationale and owner named — so it is no longer a requirement for this PR. The code changes correctly re-root both writers: ask-form-lint now writes via the reader’s own resolveCalibrationStatePath into the state dir (writer made async and the sole callsite awaits it), and both copies of the warn-main-workspace-mutation hook now resolve/read/write the baseline under the project-keyed state dir with mkdir guarding. Tests assert the reader-derived path and ensure nothing is left in the workspace. SC3’s live corpus migration is out-of-repo and marked Unverifiable, which is acceptable. I found only a minor nit: an unused resolve import in ask-form-lint-calibration.ts. With no remaining blocking issues, this PR is approved.
Findings
- [NON-BLOCKING] src/adapters/shared/commands/ask-form-lint-calibration.ts:40 — Unused import:
resolvefromnode:pathis no longer used
The refactor to useresolveCalibrationStatePath(...)removed the need forresolve(workspacePath, …), but the importimport { dirname, resolve } from "node:path";remains. This is a minor cleanup; consider removingresolveto satisfy linters and reduce noise.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
SC1 — Neither writer resolves its path against the repo root. Verify: after the change, find .minsky -maxdepth 1 \( -name '*.jsonl' -o -name '*.json' \) -newermt '<change date>' returns neither file. |
Met | Both writers now resolve to the state dir. Evidence: .claude/hooks/warn-main-workspace-mutation.ts:270-309 introduces baselinePath() using getMinskyStateDir() + projectStateKey(repoRoot) and updates readBaseline/writeBaseline to use it; the mirrored .minsky/hooks file makes the same change at :267-309. src/adapters/shared/commands/ask-form-lint-calibration.ts:120-131 now calls await resolveCalibrationStatePath(workspacePath, ASK_FORM_LINT_CALIBRATION_LOG) instead of resolve(workspacePath, …). |
SC2 — ask-form-lint-calibration.jsonl is written where resolveCalibrationStatePath reads it. Verify by running the sweep for that guard and getting a non-zero record count, having produced a fire first. |
Met | The writer uses the reader’s resolver: src/adapters/shared/commands/ask-form-lint-calibration.ts:120-131 calls resolveCalibrationStatePath(...). Tests assert via the same resolver and verify the write: src/adapters/shared/commands/ask-form-lint-calibration.test.ts:95-116 and :123-146 compute the path with resolveCalibrationStatePath and check file existence/contents there, and also assert no file is left under the workspace path. |
| SC3 — The existing 17KB repo copy is dispositioned explicitly: migrated into the state dir (preserving the pre-move corpus) or deleted with a stated reason. It is not left orphaned — that is the condition mt#4777 exists to clean up, and this task should not add to its pile. | Unverifiable | Disposition is an operational step outside the repo. The PR description claims a byte-identical migration and source removal under ~/.local/state/minsky/projects/<key>/..., but this is an out-of-repo path and cannot be verified from the diff. |
SC4 — Both streams' rows in packages/domain/src/guard-events/stream-sources.ts carry the correct location, and docs/architecture/guard-calibration-stream-inventory.md matches. |
Met | packages/domain/src/guard-events/stream-sources.ts already lists calibration streams (including ask-form-lint) with location: "state-dir" (see module header and CALIBRATION_STREAMS block near top). This PR does not change that file; the writer now aligns to the declared location. The inventory doc was not modified and already described this state, per spec; no contradiction introduced by this change. |
| SC5 — DEFERRED to mt#4816 (2026-08-31), owner named, not dropped. A check must assert SC1 mechanically so a third writer cannot reappear on the old path unnoticed. It is NOT shipped in this task's PR, deliberately: coordinate one behaviour-scoped check in mt#4816. | Met | The current spec explicitly defers SC5 to mt#4816 and records the rationale and owner in the criterion text. This PR does not introduce a new check, consistent with the deferral stated in the canonical Success Criteria. |
Documentation impact
- no-update-needed — Internal re-rooting of two telemetry writers and aligning ask-form-lint writer with an already-declared state-dir location. No user-facing commands, flags, or routes changed. Verified manifest already had ask-form-lint as location: "state-dir" (packages/domain/src/guard-events/stream-sources.ts), so no docs changed or invalidated by this PR.
There was a problem hiding this comment.
Independent adversarial review (Chinese-wall)
Reviewer: minsky-reviewer[bot] via openai:gpt-5
Tier: 2
Verification pass: the prior BLOCKING gap (SC5 missing) has been addressed by the canonical task spec deferring SC5 to mt#4816 with rationale and owner, so it is no longer required in this PR. The code changes correctly re-root both writers: ask-form-lint now uses resolveCalibrationStatePath (writer made async and its production callsite updated to await), and both hook copies of the main-workspace mutation baseline now resolve/read/write under the project-keyed state dir with mkdir guarding. Tests assert the reader-derived path and ensure nothing is left under the workspace. I found only a minor nit (unused resolve import) and no new critical defects introduced by the fix. Approving to merge.
Findings
- [NON-BLOCKING] src/adapters/shared/commands/ask-form-lint-calibration.ts:40 — Unused import:
resolvefromnode:pathis no longer used after switching toresolveCalibrationStatePath
The refactor to compute the log path viaresolveCalibrationStatePath(...)removed the need forresolve(workspacePath, …), but the import still readsimport { dirname, resolve } from "node:path";.resolveis not referenced in this module anymore. Suggest removing it to satisfy linters and keep imports tidy.
Spec verification
| Criterion | Status | Evidence |
|---|---|---|
SC1 — Neither writer resolves its path against the repo root. Verify: after the change, find .minsky -maxdepth 1 \( -name '*.jsonl' -o -name '*.json' \) -newermt '<change date>' returns neither file. |
Met | Both writers now resolve to the state dir. Evidence in diff: .claude/hooks/warn-main-workspace-mutation.ts:270-309 introduces baselinePath() using getMinskyStateDir() + projectStateKey(repoRoot) and updates readBaseline/writeBaseline to use it; the mirrored .minsky/hooks file makes the same change at :267-309. src/adapters/shared/commands/ask-form-lint-calibration.ts:120-131 now calls await resolveCalibrationStatePath(workspacePath, ASK_FORM_LINT_CALIBRATION_LOG) instead of resolve(workspacePath, …). |
SC2 — ask-form-lint-calibration.jsonl is written where resolveCalibrationStatePath reads it. Verify by running the sweep for that guard and getting a non-zero record count, having produced a fire first. |
Met | The writer uses the reader’s resolver. src/adapters/shared/commands/ask-form-lint-calibration.ts:120-131 calls resolveCalibrationStatePath(...). Tests assert via the same resolver and verify the write and absence under the workspace: src/adapters/shared/commands/ask-form-lint-calibration.test.ts:95-116, 123-146 compute the path with resolveCalibrationStatePath and check file existence/contents there, and also assert no file under the workspace path. |
| SC3 — The existing 17KB repo copy is dispositioned explicitly: migrated into the state dir (preserving the pre-move corpus) or deleted with a stated reason. It is not left orphaned — that is the condition mt#4777 exists to clean up, and this task should not add to its pile. | Unverifiable | Disposition is an operational step outside the repo. The PR body claims byte-identical migration and source removal under ~/.local/state/minsky/projects/<key>/..., but this is an out-of-repo path and cannot be verified from the diff. No code in this PR performs the migration; treating as Unverifiable per Out-of-repo references observed. |
SC4 — Both streams' rows in packages/domain/src/guard-events/stream-sources.ts carry the correct location, and docs/architecture/guard-calibration-stream-inventory.md matches. |
Met | No manifest/doc edits were required by this PR; ask-form-lint was already declared location: "state-dir". This PR aligns the writer to that standing declaration (see src/adapters/shared/commands/ask-form-lint-calibration.ts:120-131). No contradictions introduced in docs by this diff. |
| SC5 — DEFERRED to mt#4816 (2026-08-31), owner named, not dropped. A check must assert SC1 mechanically so a third writer cannot reappear on the old path unnoticed. It is NOT shipped in this task's PR, deliberately: coordinate one behaviour-scoped check in mt#4816. | Met | The canonical Success Criteria in this task spec explicitly defer SC5 to mt#4816 with rationale and owner. This PR does not add a new check, consistent with the spec’s deferral. |
Documentation impact
- no-update-needed — Internal re-rooting of two telemetry writers with no user-visible commands, flags, or routes changed. The manifest already declared ask-form-lint as location: "state-dir"; this PR aligns the writer to that standing declaration. No docs in this repo were modified by the PR, and no existing inventory prose is invalidated by these code-only path changes.
1 participant
Summary
Two writers were still appending into the repo working tree after mt#4748's merge (2026-08-30
19:44), with mtimes to prove it — one of them advancing during the session that found it. One was
broken rather than merely misplaced.
ask-form-lintwas broken. It is aCALIBRATION_LOG_REGISTRYmember(
calibration-sweep.ts:255), so/calibration-reviewresolves it throughresolveCalibrationStatePath— the state dir. The writer still didresolve(workspacePath, ...). Measured before the fix: 17,187 bytes in the repo copy and nostate-dir copy at all, so the sweep read an empty corpus and would report "this guard never
fired." Same silent-zero shape mt#4780 fixed on the docs side, arriving from the write side, which
mt#4780 explicitly scoped out.
warn-main-workspace-mutationwas not broken. Reader and writer both usedderiveHookRepoRoot(), so it worked — it just deposited a baseline file into whatever project theagent was in, which is the condition mt#4748's SC2 forbids.
Key changes
appendAskFormLintCalibrationRecordcalls the reader's own resolver, so writer and readeragree by construction rather than by two derivations staying in sync. It became async for that;
its one production caller was already inside an async handler.
warn-main-workspace-mutationresolves via the project-keyed state dir, importingprojectStateKeyrather than recomputing it.dispatcher.ts:344,ingest-runtime.ts:70,calibration.ts:236, and one insidecalibration-review-cadence-detector.ts:134whose comment explains the choice.)Execution evidence:
Typecheck clean across 8 projects; lint 0 errors / 0 warnings over 4,246 files.
SC1 — neither writer resolves against the repo root;
find .minsky -newermt <merge>no longerreturns either file.
SC2 — verified live, below.
SC3 — migrated, see Live verification.
SC4 — no manifest change needed, and the reason is the finding:
ask-form-lintwas alreadydeclared
location: "state-dir"instream-sources.ts:89. Reader and manifest agreed with eachother and both disagreed with the writer, so this aligns the writer to a standing declaration.
main-workspace-mutation-baselineis not an ingested stream and has no row.SC5 — NOT met in this PR.
[sc5-deferred: mt#4816]. The criterion itself says to coordinate onecheck rather than add a third independent one, and the implementation findings sharpened why: the
population is defined by BEHAVIOUR (resolves a repo-rooted telemetry path and appends to it), not by
directory, so a check scoped to
.minsky/hooks/**would reproduce the blind spot that hidask-form-lint-calibration.tsinsrc/adapters/shared/commands/from every prior sweep. mt#4816 isthe last task in the family and carries the coordinated check in its own SC5. Deferring rather than
shipping a fourth directory-scoped check is the point, not an omission.
Negative control. Reverting only the path resolution turns the new assertions red:
The new test asserts the path via
resolveCalibrationStatePath— the same function the sweepcalls — rather than a hand-written path. A hand-written expectation would have passed happily while
reader and writer disagreed, which is precisely the state being fixed.
Live verification
SC3's disposition of the pre-existing corpus, run against the real state dir:
51 records migrated byte-identically, source removed (so it does not become a 62nd orphan for
mt#4777), and the reader now finds them. The same call before this change found nothing.
Deploy verification:
Ran the predicate over the actual changed files rather than recalling the pattern set:
ask-form-lint-calibration.ts,asks.tsand the test aretrue; bothwarn-main-workspace-mutation.tscopies arefalse. So this IS deploy surface.I will run
deployment_wait-for-latestafter merge with the merge timestamp asnotBeforeand themerge commit as
expectCommitSha, require a health body whoseservicematches, and — sinceminsky-mcp is image-source and returns
buildIdentity: indeterminateby construction — settleidentity by correlating the Deploy MCP workflow run's
head_shaagainst the merge commit.Three corrections found while implementing
calibration-review-cadence-detectoris not broken. The spec's carve-out listed it amongthree still-repo-rooted writers. mt#4748 fixed it (
:146), and the mtimes that put it on mylist predate that merge. Two writers, not three.
verify-subagent-modelwas started as a fold-in and reversed. It isfamily: "special",which
resolveStreamPath(ingest-runtime.ts:74-91) keeps FLAT rather than project-keyed bydeliberate design — so moving it is a decision about what that family means, not a path edit.
Filed as mt#4816. It is the last
location: "repo"row.writer" (mt#4752, mt#4778, this, mt#4816) — see SC5 above.
Parallel work
PR #3412 (mt#4639, 614-log-site sweep) also modifies
ask-form-lint-calibration.ts. Found onpage 3 of its 313-file list — page 1 read as clean, so a single-page check would have recorded a
false negative. It rewrites the error-formatting call inside the catch block; this changes the
path resolution above it, so the edits are line-disjoint and the rebase is trivial. My other three
in-scope files are untouched by it.