ssf execution review: report path is double-prefixed for relative <change-dir> (receipts cannot be recorded) #137

Version: spec-superflow 2.0.1 (ssf --version → 2.0.1), Node v22.23.3, Linux
Severity: blocks a required workflow step (recording a review receipt); no workaround via CLI flags alone

Summary

Recording a review receipt fails with a path that contains the change directory twice:

$ ssf execution review changes/dock-ui-fixes \
    --wave implementation \
    --base 989add3d1231a75a7f8b377bbe44fb9e98de1d77 \
    --head 1f1deb1162206683fb5529a8dfa1d7fd453e0421 \
    --report .superpowers/sdd/reviews/final.md \
    --verdict pass
Error: Review report evidence cannot be read: ENOENT: no such file or directory, lstat
'.../.superpowers/sdd/reviews/snapshots/223aabf8-e253-4d3c-855d-1bec839f1fab.md'

The snapshot file itself is created, at the correct location
(…/.superpowers/sdd/reviews/snapshots/<uuid>.md); only the path handed to the
validator is wrong. The same failure occurs with an absolute --report value, so the report
argument is not the deciding factor — the shape of <change-dir> is.

Root cause

changeDir is used verbatim, never resolved to an absolute path:

• scripts/lib/cmd-execution.mjs:32 — const changeDir = positionals[1]; (passed through unchanged)
• scripts/lib/cmd-execution.mjs:181-188 — recordAndPrintReview() forwards it to recordReview()

Overlay paths therefore inherit the relativity of the argument:

// scripts/lib/sdd-overlay.mjs:17-28
export function getOverlayPaths(changeDir) {
  const root = join(changeDir, '.superpowers', 'sdd');   // relative in → relative out
  return { root, …, reviews: join(root, 'reviews') };
}

Verified directly:

$ node --input-type=module -e "import {getOverlayPaths} from './scripts/lib/sdd-overlay.mjs';
  console.log(getOverlayPaths('changes/dock-ui-fixes').reviews)"
changes/dock-ui-fixes/.superpowers/sdd/reviews          # relative

$ node --input-type=module -e "…getOverlayPaths('/abs/changes/dock-ui-fixes').reviews"
/abs/changes/dock-ui-fixes/.superpowers/sdd/reviews     # absolute

The snapshot step then mixes that relative path with an absolute change root:

// scripts/lib/execution-plan.mjs:206-210
const snapshot = join(paths.reviews, 'snapshots', `${randomUUID()}.md`);  // relative when changeDir is
mkdirSync(dirname(snapshot), { recursive: true });
atomicWrite(snapshot, body);                       // written relative to process cwd → correct file lands
const immutableReport = validateReviewReportEvidence(changeDir, snapshot);

// scripts/lib/execution-plan.mjs:1105-1106
const { changeRoot, reviewsDir } = getPhysicalReviewsDirectory(changeDir); // realpath(changeDir) → absolute
const reportPath = isAbsolute(report) ? resolve(report) : resolve(changeRoot, report);
//               → resolve(absolute changeRoot, relative snapshot) → doubled prefix

So the two halves of the code disagree about the base for overlay paths: sdd-overlay.mjs treats
changeDir as the base, validateReviewReportEvidence treats the repo-relative change root as the base
after an absolute realpath.

Suggested fix

One of:

1. Normalise the base once — e.g. const base = resolve(changeDir) inside getOverlayPaths()
   (and/or in recordReview), so every overlay path is absolute; or
2. Make validateReviewReportEvidence relative to the same base used to build the snapshot
   (do not mix realpath(changeDir) with a possibly-relative join(changeDir, …)).

Either fix also protects the other callers of getOverlayPaths() (debug-attempts.mjs:183,
execution-plan.mjs:48/84) from the same class of bug.

Impact

• Any invocation using a repo-relative change dir — i.e. the documented usage
  (ssf execution review changes/<name> …) — cannot record a receipt, so ssf workflow complete
  can never succeed for a planned workflow.
• It is environment-dependent: passing an absolute change dir works, and calling
  recordReview() from the lib directly with an absolute changeDir works too (that is how this was
  worked around in practice). This ambiguity is probably why it went unnoticed: receipts recorded from
  a context where changeDir happened to be absolute pass.

Workarounds

Resolver behaviour verified directly (snapshot name stubbed with a fixed UUID, same call sequence as
recordReview):

changeDir = "changes/dock-ui-fixes"
  atomicWrite          → changes/dock-ui-fixes/.superpowers/sdd/reviews/snapshots/<uuid>.md
  validator lstat      → /repo/changes/dock-ui-fixes/changes/dock-ui-fixes/.superpowers/sdd/reviews/snapshots/<uuid>.md   ← ENOENT

changeDir = "/repo/changes/dock-ui-fixes"
  atomicWrite          → /repo/changes/dock-ui-fixes/.superpowers/sdd/reviews/snapshots/<uuid>.md
  validator lstat      → /repo/changes/dock-ui-fixes/.superpowers/sdd/reviews/snapshots/<uuid>.md                        ← matches

# 1) absolute change dir
ssf execution review /abs/path/changes/<name> --wave <id> --base <sha> --head <sha> \
  --report .superpowers/sdd/reviews/final.md --verdict pass

# 2) call the lib directly (report is change-dir relative)
node --input-type=module -e "import {recordReview} from '<pkg>/scripts/lib/execution-plan.mjs';
  console.log(recordReview('/abs/path/changes/<name>', '<id>',
    {status:'pass', base:'<sha>', head:'<sha>', report:'.superpowers/sdd/reviews/final.md'}))"

Secondary finding (same command family)

ssf finish <change-dir> runs the trunk verification with a hard-coded default:

// scripts/lib/cmd-finish.mjs:163
const verifyCmd = values['test-cmd'] || 'npm test';

In a non-Node repository the merge succeeds but the finish step then fails on npm test
(ENOENT … package.json) and leaves the isolation context pending. Suggest defaulting to the
verification command already recorded for the change (or requiring --test-cmd explicitly) instead of
assuming Node.

Reproduction environment

• Change directory: changes/dock-ui-fixes inside a git repo, ssf invoked from the repository root.
• The change had a valid execution plan (ssf execution show → current) and a resolvable
  base/head range; only the report-path resolution failed.
• Two failed attempts left two unreferenced snapshot files behind (they are written before the
  failing validation).