fix(graceful-handoff): two defects found by the first real smoke test (v3.2.1)

The pipeline had never been run against an actual repository — every test in
the suite is either a prose-grep over SKILL.md or a unit test that asserts key
presence. Running it against scratch repos (private remote, and public `open/`
remote with and without a gitignored STATE.md) found two defects living under
a 42/42-green suite.

1. dirty_files truncated the first path. gitOk() trims every command's output,
   but `git status --porcelain` puts the worktree status in column 2, so a
   modified-but-unstaged file is " M path". The trim ate the leading space and
   the fixed slice(3) then ate the first character: app.js was reported as
   pp.js. Only the first line is affected, which is why it survived — no test
   asserted dirty_files VALUES, only that the key existed. Porcelain now goes
   through a non-trimming gitOkRaw().

2. The commit message claimed a STATE.md update it did not contain. The
   message was hardcoded to "oppdater STATE.md" regardless of what was staged.
   On every `open/` repo STATE.md is gitignored, so the handoff commit carries
   only the --also paths. Git history is the regime's long-term log; it was
   systematically wrong about its own contents.

Also promotes the leak condition from advisory to hard gate. A public remote
whose STATE.md is not yet gitignored is the state a FRESH open/ repo starts
in, and should_commit_state was true there — the ritual only mentioned
leak_warning, then committed. It now lands in errors[] (step 2 stops on a
non-empty errors[]), should_commit_state is false, and --commit refuses to
stage STATE.md. Explicit --also paths are still honoured: the gate protects
STATE.md, not the commit as a whole.

And corrects SKILL.md's justification for the single-line rule. It claimed a
wrapped rationale= replaces the board's next step with garbage; board.sh in
repo-mailbox 0.20.3 tracks a comment to its closer, so that no longer follows.
The rule stands, restated with the risk that is still real: a rationale
containing the closer sequence ends its own comment early.

Tests 42 -> 48, all six written failing first.

Still unverified: that /graceful-handoff loads as a user command (#26251), and
that a cross-plugin Skill invocation of repo-mailbox:route passes from a
sub-scoped skill. Both need the catalog ref bumped so the version is installed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RvLY4FwbzY157oVqwnkHD8
This commit is contained in:
Kjell Tore Guttormsen 2026-08-09 21:49:08 +02:00
commit 5334097c84
9 changed files with 201 additions and 32 deletions

View file

@ -59,10 +59,23 @@ function gitOk(cmd, opts = {}) {
}
}
// Same as gitOk, but WITHOUT trim(). `git status --porcelain` puts the worktree
// status in column 2, so a modified-but-unstaged file is " M path" — a leading
// space that carries meaning. Trimming it shifted the first line left, and the
// fixed slice(3) below then ate the first character of the first path ("app.js"
// → "pp.js"). Only ever use this where leading whitespace is significant.
function gitOkRaw(cmd, opts = {}) {
try {
return execSync(cmd, { encoding: 'utf-8', stdio: ['ignore', 'pipe', 'pipe'], ...opts });
} catch {
return null;
}
}
function gitStatus(cwd) {
const o = { cwd };
const branch = gitOk('git branch --show-current', o) || gitOk('git rev-parse --abbrev-ref HEAD', o);
const porcelain = gitOk('git status --porcelain', o) || '';
const porcelain = (gitOkRaw('git status --porcelain', o) || '').replace(/\n+$/, '');
const dirty = porcelain.length > 0;
let ahead = 0;
const upstream = gitOk('git rev-parse --abbrev-ref @{u} 2>/dev/null', o);
@ -110,6 +123,18 @@ export function classifyRemote(url) {
return 'private';
}
// The leak gate. A public remote whose STATE.md is NOT gitignored is not an
// advisory condition — it is the single state in which continuing the ritual is
// itself what causes the leak, and it is exactly the state a fresh `open/` repo
// starts in. Both --plan and --commit consult this one function so they cannot
// disagree about whether STATE.md may be staged.
export function leakBlock({ shouldBeLocalOnly, stateGitignored, remoteClass, remoteUrl }) {
if (!shouldBeLocalOnly || stateGitignored) return null;
return `BLOKKERT: STATE.md er IKKE gitignored, men remote er ${remoteClass} (${remoteUrl || 'ingen'}) — `
+ `å committe den ville lekket intern state-of-play ved push. Legg 'STATE.md' i .gitignore `
+ `(den skal være local-only på denne remoten), så kjør rituelet på nytt.`;
}
// ---------- Plan (read-only) ----------
function buildPlan(cwd, errors) {
@ -131,10 +156,9 @@ function buildPlan(cwd, errors) {
{ cwd: dirname(statePath) }
);
const leakWarning = (shouldBeLocalOnly && !stateGitignored)
? `STATE.md er IKKE gitignored, men remote er ${remoteClass} (${remoteUrl || 'ingen'}) — `
+ `legg STATE.md i .gitignore for å unngå å lekke intern state-of-play ved push.`
: null;
const leakWarning = leakBlock({ shouldBeLocalOnly, stateGitignored, remoteClass, remoteUrl });
// Blocking, not advisory: step 2 of the ritual stops on a non-empty errors[].
if (leakWarning) errors.push(leakWarning);
const git = gitStatus(cwd);
const recentCommits = (gitOk('git log --oneline -8', { cwd }) || '').split('\n').filter(Boolean);
@ -149,7 +173,7 @@ function buildPlan(cwd, errors) {
remote_url: remoteUrl,
remote_class: remoteClass,
should_be_local_only: shouldBeLocalOnly,
should_commit_state: !stateGitignored,
should_commit_state: !stateGitignored && !leakWarning,
leak_warning: leakWarning,
git_status: { branch: git.branch, dirty: git.dirty, ahead: git.ahead, detached: git.detached, upstream: git.upstream },
dirty_files: git.dirtyFiles,
@ -161,9 +185,15 @@ function buildPlan(cwd, errors) {
// ---------- Commit (write) ----------
function generateCommitMessage(statePath, root) {
// The message must describe what the commit actually contains. On every `open/`
// repo STATE.md is gitignored, so the handoff commit carries only the explicit
// --also paths — claiming a STATE.md update there made git history, the regime's
// long-term log, systematically wrong about its own contents.
function generateCommitMessage(statePath, root, includesState) {
const name = root ? basename(root) : basename(dirname(statePath));
return `docs(${name}): oppdater STATE.md (session handoff)`;
return includesState
? `docs(${name}): oppdater STATE.md (session handoff)`
: `chore(${name}): session handoff (STATE.md local-only, ikke committet)`;
}
function doCommit(cwd, args, errors) {
@ -185,8 +215,20 @@ function doCommit(cwd, args, errors) {
// Build the stage list. CRITICAL: never `git add -A` — stage ONLY STATE.md
// (when it is tracked, i.e. not gitignored) plus any explicit --also paths.
// Same gate as --plan: never stage STATE.md onto a public remote. --also paths
// stay honoured — the operator named those explicitly, and the gate protects
// STATE.md specifically, not the commit as a whole.
const remoteUrl = gitOk('git remote get-url origin 2>/dev/null', { cwd });
const remoteClass = classifyRemote(remoteUrl);
const leakWarning = leakBlock({
shouldBeLocalOnly: remoteClass !== 'private', stateGitignored, remoteClass, remoteUrl,
});
const stageList = [];
if (!stateGitignored) {
if (leakWarning) {
errors.push(leakWarning);
actions.push('state-leak-blocked (STATE.md ikke gitignored på offentlig remote — ikke staget)');
} else if (!stateGitignored) {
if (existsSync(statePath)) stageList.push(statePath);
else errors.push(`STATE.md finnes ikke på ${statePath} — skriv den før commit`);
} else {
@ -199,11 +241,11 @@ function doCommit(cwd, args, errors) {
}
if (stageList.length === 0) {
actions.push('intet-å-committe (STATE.md gitignored og ingen --also-stier)');
actions.push('intet-å-committe (STATE.md ikke committerbar og ingen --also-stier)');
return { mode: 'commit', actions_taken: actions, errors, git_status: git };
}
const message = args.message || generateCommitMessage(statePath, root);
const message = args.message || generateCommitMessage(statePath, root, stageList.includes(statePath));
try {
execFileSync('git', ['add', '--', ...stageList], { cwd, stdio: ['ignore', 'pipe', 'pipe'] });