diff --git a/.claude/hooks/gate-walk-provenance.ts b/.claude/hooks/gate-walk-provenance.ts index 53b43571b..472e034ed 100755 --- a/.claude/hooks/gate-walk-provenance.ts +++ b/.claude/hooks/gate-walk-provenance.ts @@ -496,15 +496,27 @@ export function appendCalibrationRecord( repoRootDir: string ): void { // mt#4752: the shared helper derives the filename from the stream NAME, so it - // cannot drift from the convention the .gitignore globs encode. Unlike the - // detectors migrated alongside this one, `repoRootDir` here IS authoritative — - // it is resolved by the caller, not a raw shell cwd — so it goes in the - // `projectDir` tier, which outranks `CLAUDE_PROJECT_DIR`. + // cannot drift from the convention the .gitignore globs encode. + // + // mt#4885: `repoRootDir` goes in the `fallbackCwd` tier, NOT `projectDir`. + // This comment previously claimed it "IS authoritative — it is resolved by the + // caller, not a raw shell cwd". That was false: the only caller resolves it as + // `findRepoRoot(input.cwd)`, so it IS a raw shell cwd, merely one that has been + // walked up to a repo root. `calibrationLogPath`'s own docblock names this exact + // mistake — "Passing that cwd as `projectDir` would preserve the bug through the + // migration — the whole point is that it lands in the lower tier" — because in a + // session workspace `findRepoRoot` returns the CLONE, which is a legitimate repo + // root and the wrong project. Demoting it lets `CLAUDE_PROJECT_DIR` outrank it. + // + // This fixes the TIER only. When `CLAUDE_PROJECT_DIR` is unset the ladder still + // falls through to this value and still keys to the clone — that is a separate + // defect (which root a session workspace SHOULD resolve to), tracked as mt#4954 + // and deliberately not changed here. // // The former third parameter (`logRelPath`, defaulted to the path literal) is // gone: both call sites used the default, and a path-taking parameter is the // degree of freedom that let a sibling's filename be misspelled (mt#2492). - logCalibrationRecord(CALIBRATION_LOG_NAME, record, { projectDir: repoRootDir }); + logCalibrationRecord(CALIBRATION_LOG_NAME, record, { fallbackCwd: repoRootDir }); } // --------------------------------------------------------------------------- diff --git a/.claude/hooks/require-execution-evidence-before-merge.ts b/.claude/hooks/require-execution-evidence-before-merge.ts index 19497c475..4a5dfb7c9 100755 --- a/.claude/hooks/require-execution-evidence-before-merge.ts +++ b/.claude/hooks/require-execution-evidence-before-merge.ts @@ -986,14 +986,22 @@ export function isAtCoverageSkipped(): boolean { * parameter itself is now gone, so neither failure has anywhere left to occur: every call site * names a stream constant, and `logCalibrationRecord` derives the filename from it. * - * `repoRootDir` is authoritative here — the caller resolved it, it is not a raw shell cwd — so it - * goes in the `projectDir` tier, which outranks `CLAUDE_PROJECT_DIR`. + * mt#4885: `repoRootDir` goes in the `fallbackCwd` tier, NOT `projectDir`. This docblock + * previously claimed it was "authoritative here — the caller resolved it, it is not a raw shell + * cwd". That was false: the caller resolves it as `findRepoRoot(input.cwd)`, so it IS a raw shell + * cwd walked up to a repo root. In a session workspace `findRepoRoot` returns the CLONE — a real + * repo root, and the wrong project — which is why `calibrationLogPath`'s docblock reserves the top + * tier for an ALREADY-RESOLVED authoritative directory and says passing a cwd there "would preserve + * the bug through the migration". Demoting it lets `CLAUDE_PROJECT_DIR` outrank it. + * + * TIER only. With `CLAUDE_PROJECT_DIR` unset the ladder still falls through to this value and still + * keys to the clone — a separate defect, tracked as mt#4954. */ export function appendAtCoverageCalibration( record: Record, repoRootDir: string ): void { - logCalibrationRecord(AT_COVERAGE_STREAM, record, { projectDir: repoRootDir }); + logCalibrationRecord(AT_COVERAGE_STREAM, record, { fallbackCwd: repoRootDir }); } /** Result of fetching a task's spec for the AT-coverage check. */ @@ -1284,7 +1292,7 @@ if (import.meta.main) { ); if (scCoverage.calibrationRecord) { logCalibrationRecord(SC_COVERAGE_STREAM, scCoverage.calibrationRecord, { - projectDir: repoRootDir, + fallbackCwd: repoRootDir, }); } if (scCoverage.warning) { @@ -1309,7 +1317,7 @@ if (import.meta.main) { ); if (testFirst.calibrationRecord) { logCalibrationRecord(TEST_FIRST_STREAM, testFirst.calibrationRecord, { - projectDir: repoRootDir, + fallbackCwd: repoRootDir, }); } if (testFirst.warning) { @@ -1324,7 +1332,7 @@ if (import.meta.main) { const renderPath = runRenderPathCalibration(task, context.prNumber, prFiles, prBody); if (renderPath.calibrationRecord) { logCalibrationRecord(RENDER_PATH_STREAM, renderPath.calibrationRecord, { - projectDir: repoRootDir, + fallbackCwd: repoRootDir, }); } if (renderPath.warning) { @@ -1353,7 +1361,7 @@ if (import.meta.main) { ); if (consumerAccount.calibrationRecord) { logCalibrationRecord(CONSUMER_ACCOUNT_STREAM, consumerAccount.calibrationRecord, { - projectDir: repoRootDir, + fallbackCwd: repoRootDir, }); } if (consumerAccount.warning) { diff --git a/.minsky/hooks/calibration-root-tier.test.ts b/.minsky/hooks/calibration-root-tier.test.ts new file mode 100644 index 000000000..edea96bcd --- /dev/null +++ b/.minsky/hooks/calibration-root-tier.test.ts @@ -0,0 +1,114 @@ +/** + * mt#4885 — the two calibration writers put their cwd-derived root in the `fallbackCwd` + * tier, not `projectDir`. + * + * **Why a dedicated file rather than an assertion in each writer's own suite.** The existing + * suites (`gate-walk-provenance.test.ts`, `require-execution-evidence-before-merge.test.ts`, + * 264 tests between them) pass IDENTICALLY before and after the fix, because they run with + * `CLAUDE_PROJECT_DIR` unset — and with an empty top tier, `projectDir: root` and + * `fallbackCwd: root` resolve to the same path. A test that cannot fail against the defect is + * not evidence about it (mem#704), so the discriminating condition has to be constructed: + * `CLAUDE_PROJECT_DIR` SET, to a directory that is not the one the caller passes. + * + * Against the pre-fix code every assertion below inverts — the record lands under the passed + * root because `projectDir` outranks `CLAUDE_PROJECT_DIR`. That is the negative control, and + * it is recorded in the PR body. + */ +/* eslint-disable custom/no-real-fs-in-tests -- The assertion under test IS the on-disk location a writer resolves to. Injecting a mock fs would move the writes into a fake filesystem and leave the real path-resolution ladder — the only thing this file exists to pin — unexercised, so the rule's remedy would defeat the test. Blast radius is bounded to per-test `mkdtemp` directories under the OS temp dir, with `MINSKY_STATE_DIR` redirected so nothing touches the operator's real calibration streams. */ +import { describe, it, expect, beforeEach, afterEach } from "bun:test"; +import { mkdtempSync, rmSync, readFileSync, existsSync } from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; + +/** Env var names, extracted so the ladder's two tiers are named once each. */ +const STATE_DIR_ENV = "MINSKY_STATE_DIR"; +const PROJECT_DIR_ENV = "CLAUDE_PROJECT_DIR"; + +import { calibrationLogPath } from "./dispatcher"; +import { appendCalibrationRecord } from "./gate-walk-provenance"; +import { + appendAtCoverageCalibration, + AT_COVERAGE_STREAM, +} from "./require-execution-evidence-before-merge"; + +/** + * `gate-walk-provenance.ts` keeps its stream name module-private, so it is spelled here rather + * than exported purely for this test. A rename there makes these cases fail (the record is not + * found under either root), which is the correct signal — not a silent pass. + */ +const GATE_WALK_STREAM = "gate-walk-provenance"; + +/** + * Two roots that cannot collapse to one key. Both live under the OS temp dir, which has no + * `.git` above it, so `findRepoRoot` degrades to its input unchanged (the documented behavior + * `calibrationLogPath`'s docblock relies on for synthetic paths) and the sha256 keys differ. + * + * `authoritativeRoot` stands in for the real project — what `CLAUDE_PROJECT_DIR` names. + * `callerRoot` stands in for the session-workspace clone a guard's `findRepoRoot(input.cwd)` + * actually returns. + */ +let stateDir: string; +let authoritativeRoot: string; +let callerRoot: string; +let priorStateDir: string | undefined; +let priorProjectDir: string | undefined; + +beforeEach(() => { + stateDir = mkdtempSync(join(tmpdir(), "mt4885-state-")); + authoritativeRoot = mkdtempSync(join(tmpdir(), "mt4885-project-")); + callerRoot = mkdtempSync(join(tmpdir(), "mt4885-clone-")); + + priorStateDir = process.env[STATE_DIR_ENV]; + priorProjectDir = process.env[PROJECT_DIR_ENV]; + process.env[STATE_DIR_ENV] = stateDir; + process.env[PROJECT_DIR_ENV] = authoritativeRoot; +}); + +afterEach(() => { + if (priorStateDir === undefined) delete process.env[STATE_DIR_ENV]; + else process.env[STATE_DIR_ENV] = priorStateDir; + if (priorProjectDir === undefined) delete process.env[PROJECT_DIR_ENV]; + else process.env[PROJECT_DIR_ENV] = priorProjectDir; + + for (const d of [stateDir, authoritativeRoot, callerRoot]) { + rmSync(d, { recursive: true, force: true }); + } +}); + +/** Read the one record a writer just appended, resolving the path the READER's own way. */ +function readRecordAt(stream: string, root: string): Record | null { + const path = calibrationLogPath(stream, { projectDir: root }); + if (!existsSync(path)) return null; + const body = readFileSync(path, "utf-8").trim(); + if (body === "") return null; + return JSON.parse(body.split("\n")[0] ?? "{}") as Record; +} + +describe("mt#4885 — cwd-derived roots land in the fallbackCwd tier", () => { + it("gate-walk-provenance: CLAUDE_PROJECT_DIR outranks the caller's findRepoRoot(input.cwd)", () => { + appendCalibrationRecord({ marker: "gate-walk" }, callerRoot); + + // The record belongs to the PROJECT, not the clone the guard happened to run in. + expect(readRecordAt(GATE_WALK_STREAM, authoritativeRoot)?.["marker"]).toBe("gate-walk"); + // The absence half — this is what fails pre-fix, where `projectDir` won. + expect(readRecordAt(GATE_WALK_STREAM, callerRoot)).toBeNull(); + }); + + it("require-execution-evidence: same tier, same outcome", () => { + appendAtCoverageCalibration({ marker: "at-coverage" }, callerRoot); + + expect(readRecordAt(AT_COVERAGE_STREAM, authoritativeRoot)?.["marker"]).toBe("at-coverage"); + expect(readRecordAt(AT_COVERAGE_STREAM, callerRoot)).toBeNull(); + }); + + it("with CLAUDE_PROJECT_DIR unset the caller's root is still used — the tier is a ladder, not a redirect", () => { + // Guards the OTHER direction: demoting the tier must not break the ordinary case where + // nothing outranks it. This is also why the fix does NOT by itself stop the stranding + // mt#4885 documents — that is SC3's separate decision about which root a clone resolves to. + delete process.env[PROJECT_DIR_ENV]; + + appendCalibrationRecord({ marker: "no-project-dir" }, callerRoot); + + expect(readRecordAt(GATE_WALK_STREAM, callerRoot)?.["marker"]).toBe("no-project-dir"); + }); +}); diff --git a/.minsky/hooks/gate-walk-provenance.ts b/.minsky/hooks/gate-walk-provenance.ts index 9c2ebb670..040dfaac1 100644 --- a/.minsky/hooks/gate-walk-provenance.ts +++ b/.minsky/hooks/gate-walk-provenance.ts @@ -493,15 +493,27 @@ export function appendCalibrationRecord( repoRootDir: string ): void { // mt#4752: the shared helper derives the filename from the stream NAME, so it - // cannot drift from the convention the .gitignore globs encode. Unlike the - // detectors migrated alongside this one, `repoRootDir` here IS authoritative — - // it is resolved by the caller, not a raw shell cwd — so it goes in the - // `projectDir` tier, which outranks `CLAUDE_PROJECT_DIR`. + // cannot drift from the convention the .gitignore globs encode. + // + // mt#4885: `repoRootDir` goes in the `fallbackCwd` tier, NOT `projectDir`. + // This comment previously claimed it "IS authoritative — it is resolved by the + // caller, not a raw shell cwd". That was false: the only caller resolves it as + // `findRepoRoot(input.cwd)`, so it IS a raw shell cwd, merely one that has been + // walked up to a repo root. `calibrationLogPath`'s own docblock names this exact + // mistake — "Passing that cwd as `projectDir` would preserve the bug through the + // migration — the whole point is that it lands in the lower tier" — because in a + // session workspace `findRepoRoot` returns the CLONE, which is a legitimate repo + // root and the wrong project. Demoting it lets `CLAUDE_PROJECT_DIR` outrank it. + // + // This fixes the TIER only. When `CLAUDE_PROJECT_DIR` is unset the ladder still + // falls through to this value and still keys to the clone — that is a separate + // defect (which root a session workspace SHOULD resolve to), tracked as mt#4954 + // and deliberately not changed here. // // The former third parameter (`logRelPath`, defaulted to the path literal) is // gone: both call sites used the default, and a path-taking parameter is the // degree of freedom that let a sibling's filename be misspelled (mt#2492). - logCalibrationRecord(CALIBRATION_LOG_NAME, record, { projectDir: repoRootDir }); + logCalibrationRecord(CALIBRATION_LOG_NAME, record, { fallbackCwd: repoRootDir }); } // --------------------------------------------------------------------------- diff --git a/.minsky/hooks/require-execution-evidence-before-merge.ts b/.minsky/hooks/require-execution-evidence-before-merge.ts index 5b75aa839..a73f5c208 100755 --- a/.minsky/hooks/require-execution-evidence-before-merge.ts +++ b/.minsky/hooks/require-execution-evidence-before-merge.ts @@ -983,14 +983,22 @@ export function isAtCoverageSkipped(): boolean { * parameter itself is now gone, so neither failure has anywhere left to occur: every call site * names a stream constant, and `logCalibrationRecord` derives the filename from it. * - * `repoRootDir` is authoritative here — the caller resolved it, it is not a raw shell cwd — so it - * goes in the `projectDir` tier, which outranks `CLAUDE_PROJECT_DIR`. + * mt#4885: `repoRootDir` goes in the `fallbackCwd` tier, NOT `projectDir`. This docblock + * previously claimed it was "authoritative here — the caller resolved it, it is not a raw shell + * cwd". That was false: the caller resolves it as `findRepoRoot(input.cwd)`, so it IS a raw shell + * cwd walked up to a repo root. In a session workspace `findRepoRoot` returns the CLONE — a real + * repo root, and the wrong project — which is why `calibrationLogPath`'s docblock reserves the top + * tier for an ALREADY-RESOLVED authoritative directory and says passing a cwd there "would preserve + * the bug through the migration". Demoting it lets `CLAUDE_PROJECT_DIR` outrank it. + * + * TIER only. With `CLAUDE_PROJECT_DIR` unset the ladder still falls through to this value and still + * keys to the clone — a separate defect, tracked as mt#4954. */ export function appendAtCoverageCalibration( record: Record, repoRootDir: string ): void { - logCalibrationRecord(AT_COVERAGE_STREAM, record, { projectDir: repoRootDir }); + logCalibrationRecord(AT_COVERAGE_STREAM, record, { fallbackCwd: repoRootDir }); } /** Result of fetching a task's spec for the AT-coverage check. */ @@ -1281,7 +1289,7 @@ if (import.meta.main) { ); if (scCoverage.calibrationRecord) { logCalibrationRecord(SC_COVERAGE_STREAM, scCoverage.calibrationRecord, { - projectDir: repoRootDir, + fallbackCwd: repoRootDir, }); } if (scCoverage.warning) { @@ -1306,7 +1314,7 @@ if (import.meta.main) { ); if (testFirst.calibrationRecord) { logCalibrationRecord(TEST_FIRST_STREAM, testFirst.calibrationRecord, { - projectDir: repoRootDir, + fallbackCwd: repoRootDir, }); } if (testFirst.warning) { @@ -1321,7 +1329,7 @@ if (import.meta.main) { const renderPath = runRenderPathCalibration(task, context.prNumber, prFiles, prBody); if (renderPath.calibrationRecord) { logCalibrationRecord(RENDER_PATH_STREAM, renderPath.calibrationRecord, { - projectDir: repoRootDir, + fallbackCwd: repoRootDir, }); } if (renderPath.warning) { @@ -1350,7 +1358,7 @@ if (import.meta.main) { ); if (consumerAccount.calibrationRecord) { logCalibrationRecord(CONSUMER_ACCOUNT_STREAM, consumerAccount.calibrationRecord, { - projectDir: repoRootDir, + fallbackCwd: repoRootDir, }); } if (consumerAccount.warning) {