fix(session-flow): specify continue-in-background's dirty-tree gate for a non-repo consuming directory

[kyle-sexton]

Summary

continue-in-background's dirty-tree gate opened by running git status --porcelain -uall in the
consuming project. In a directory that is not a git repository — a session started in $HOME, say —
that fails with fatal: not a git repository and the skill has no specified behavior. The gate
specified clean, dirty, and linked-worktree; not "no repository at all".

The gate now establishes repository status first with git rev-parse --is-inside-work-tree, and is
exhaustive by construction rather than by enumeration: two results may leave the default, and
everything else falls through to the conservative branch.

rev-parse result	Behavior
prints true	Inspect the tree with the existing git status --porcelain -uall gate, unchanged
fails specifically with "not a git repository"	Launch. No uncommitted work to protect, and no worktree isolation to lose — outside a repository (absent a WorktreeCreate hook) background sessions write to the working directory directly. The launch report states that reading
anything else	Tree state UNKNOWN, which is not clean → do NOT launch; same fallback as the dirty case

That third row is the load-bearing part, and it is why this is a little more than the issue's "one or
two sentences" — not scope drift. A gate that fails open is worse than no gate. Reading a non-zero
exit alone as "no repository" would put dubious-ownership, damaged-repository, and git-missing cases
on the launch path — exactly where the tree is most likely dirty and least likely readable. The
unknown default is deliberately wide and it catches a zero-exit case too (below).

Also here: the context-gathering block's "treat any failure as an unknown value and carry on" is now
scoped to itself. It colors the save-point and is not the gate; carrying that shrug — or that block's
non--uall git status output — into delivery step 1 would defeat the gate from a section that runs
before it.

Evidence

Gates (run in the worktree against origin/main):

• check-skill (the checker scripts/check-changed-skills.sh invokes): PASS — 0 errors, 0 warnings; 195/500 lines, all 4 base-ref trigger phrases preserved
• scripts/validate-plugin-contracts.mjs: pass — 43 setup skills, 2123 plugin files
• scripts/check-changelog-parity.sh --check-bump origin/main: pass — 0.17.16 → 0.17.17 patch bump paired with its ## [0.17.17] entry
• markdownlint-cli2 on both changed markdown files: 0 errors
• evals.json validated against plugins/skill-quality/reference/evals.schema.json: valid

Harness claim verified against current docs. The non-repo sentence asserts harness behavior, so
it was checked against https://code.claude.com/docs/en/agent-view this session rather than from
recall. That page states isolation is skipped when the working directory is not a git repository and
no WorktreeCreate hook is configured, and that outside a repository sessions write to the working
directory directly — which is what the new branch relies on, including the hook qualifier.

Non-vacuousness — stated honestly. evals.json in this repo is declarative and model-graded;
there is no runner that asserts, so the three added cases are not executed regression tests and are
not presented as such. The discriminating argument is textual and checkable from the diff: an agent
following the pre-change SKILL.md has no branch for #5 (it runs git status --porcelain -uall blind
and hits fatal: not a git repository) and no branch at all for #6 or #7.

• feat(hook-telemetry): marketplace-wide telemetry contract + markdown-formatter producer #5 non-repo-directory-launches — non-repo consuming directory → launches, states the non-repo reading, STOPS
• perf(markdown-formatter): unwired producer builds telemetry payload before the opt-in guard (~90ms/edit on Windows) #6 unknown-tree-state-does-not-launch — dubious-ownership failure → does NOT launch. This is the case that proves the gate does not fail open
• docs(playbook): document the local plugin development loop #7 no-work-tree-despite-zero-exit-does-not-launch — bare repository, exit 0 printing false → does NOT launch

Review

Reviewed by a fresh-context reviewer with the rationale withheld. Two IMPORTANT findings, both fixed
in 748ab3cb before this PR:

1. Unrouted fourth outcome. git rev-parse --is-inside-work-tree exits 0 and prints false
   inside a bare repository or a .git directory — verified empirically by the reviewer. The
   original three-way enumeration left that state unspecified, which is the same defect class the
   issue reports. Fixed by inverting the structure to two specified results plus a default, and
   guarded by eval docs(playbook): document the local plugin development loop #7.
2. Contradiction with the context block. "Treat any failure as an unknown value and carry on"
   sat 50 lines above a gate that now reads a git failure the opposite way. Fixed by scoping that
   sentence to its own block and pointing at the gate as owning failure semantics.

One finding was accepted and not fixed, as out of scope: the gate's pre-existing rationale that
uncommitted changes "would NOT carry into the launched agent's edits" does not hold when a consumer
sets worktree.bgIsolation: "none", under which background sessions edit the working copy directly.
The gate's decision stays conservative either way, so this is an inaccurate rationale rather than a
hole — untouched by this PR and worth its own issue.

Closes #929

Related

• feat(session-flow)!: extract background delegation into continue-in-background #924 — introduced the skill and this dirty-tree gate
• session-flow: /handoff --bg is misleading — background delegation deserves an explicit action #233 — origin design for the save-point/handoff engine the skill shares
• fix(skills): pre-compute Class 2 remainder — genuine shell expansion mixed into pre-compute in 5 plugins (canonical tracker; scope in comments, body list superseded) #1687 — the $-expansion isolation constraint the context block's shape exists to satisfy; the new
  git rev-parse --is-inside-work-tree carries no $-expansion and sits in the body, not pre-compute

[claude]

Claude encountered an error after 1s —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error after 1s —— View job

---

I'll analyze this and get back to you.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 748ab3cbc8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 2m 2s —— View job

---

PR Review — fix(session-flow): continue-in-background non-repo gate

• Gather context and read changed files
• Review SKILL.md changes (gate logic)
• Review evals.json additions
• Review CHANGELOG.md and plugin.json
• Check Codex finding about WorktreeCreate hook
• Post final review

---

Overview

The gate redesign is correct and the approach is principled. Replacing enumeration with an exhaustive-by-construction structure (two specified results + deliberate wide default) closes the original bug and the bare-repo edge case that the independent review found. The "never route by exit status alone" rule is explicit and correct. The context-block scoping, checklist update, and gotchas index are all consistent with the new gate semantics.

One finding worth addressing before merge, and one minor observation:

---

Finding 1 — WorktreeCreate hook not checked before the non-repo launch (Codex P2 finding)

File: SKILL.md line 79–83

The non-repo branch launches on the basis that "outside a repository (and absent a WorktreeCreate hook) background sessions write to the working directory directly." The parenthetical condition is correct per the docs, but the skill does not check for the hook's presence before launching — it only states the qualifier.

If a consumer has a WorktreeCreate hook configured for non-git VCS isolation (Mercurial, Perforce, SVN — the playbook's documented use case), git rev-parse will still fail with "not a git repository" and the non-repo branch will fire. The launched session then moves into the hook-created workspace, leaving any untracked files in the original working directory inaccessible. The dirty-tree gate's protections exist precisely for this scenario.

The PR description lists a finding about worktree.bgIsolation: "none" as "accepted and not fixed, as out of scope." This WorktreeCreate-in-non-repo case is distinct and not mentioned in that section.

Options in order of preference:

1. Before taking the non-repo branch, check whether a WorktreeCreate hook is defined in the effective settings (project + user). If one is present, treat as UNKNOWN → no launch. This is the fully correct fix but requires reading harness settings reliably.
2. If that check is impractical, downgrade the non-repo branch to a gated launch: surface the non-repo reading, note that a WorktreeCreate hook would defeat this assumption, and ask the user to confirm before launching. This preserves the existing behavior for the common case while making the hook caveat actionable.
3. Accept and document as a known gap (same pattern as the worktree.bgIsolation: "none" finding), open a follow-up issue, and leave this PR as-is. The behavior is still strictly better than the pre-fix fatal: not a git repository hard stop.

Fix this →

---

Observation — Eval #5 expected_output omits the hook qualifier

File: evals.json line 59

The expected_output for non-repo-directory-launches says "because outside a repository background sessions write to the working directory directly" — the same statement the SKILL.md makes, but without the "(absent a WorktreeCreate hook)" qualifier. A model graded against this eval learns that the direct-write path is always true for non-repo sessions, which is the imprecision Finding 1 targets. Updating the expected output to match the qualified form in SKILL.md would keep the eval faithful regardless of whether Finding 1 gets a code fix.

---

Everything else looks good

• Gate structure — exhaustive by construction, correct. Two identified results + deliberate wide unknown default. The false/exit-0/bare-repo case being caught by the default (not by the non-repo branch) is the right call, and eval docs(playbook): document the local plugin development loop #7 guards it.
• "Never route by exit status alone" — explicit, correctly placed, and the rationale (zero exit is not clean, non-zero exit is not "no repo") is stated in both the gate body and the checklist item.
• Context-block scoping — the added sentence ("this block only colors the save-point … never carry this block's shrug … into it") is the right defensive annotation. Without it the "treat any failure and carry on" shrug sat 50 lines above a gate that reads the same failure the opposite way.
• Enforcement checklist — updated consistently with all three branches.
• Evals perf(markdown-formatter): unwired producer builds telemetry payload before the opt-in guard (~90ms/edit on Windows) #6 and docs(playbook): document the local plugin development loop #7 — correct and non-vacuous. perf(markdown-formatter): unwired producer builds telemetry payload before the opt-in guard (~90ms/edit on Windows) #6 proves the gate does not fail open on dubious-ownership failures; docs(playbook): document the local plugin development loop #7 proves it does not misread a zero exit as clean.
• Version bump — 0.17.16 → 0.17.17 patch, paired with its CHANGELOG entry. Correct.

[claude]

Claude finished @kyle-sexton's task in 1m 57s —— View job

---

Security Review — PR #1728

• Gather context and load PR diff
• Read changed files
• Analyze for security issues (logic flaws, injection surfaces, authorization gaps)
• Post findings

---

No security issues to report. The four changed files are a version bump (plugin.json), a changelog entry (CHANGELOG.md), evaluation test cases (evals.json), and a Markdown instruction file (SKILL.md). There is no executable code in this diff. My analysis focused on the instruction logic, the commands the skill directs an agent to run, and the trust boundaries involved in the new gate.

Analysis

Gate logic — no failure-open path introduced.
The new two-step gate (git rev-parse --is-inside-work-tree, then git status --porcelain -uall) is structured with a deliberately wide default: any result that is not a positive true or a positive "not a git repository" string stays on the no-launch path. This correctly handles the safe.directory/dubious-ownership case (non-zero exit with a different message → stays on the no-launch default, not the non-repo launch branch), the bare-repository / .git-directory case (zero exit printing false → also stays on the no-launch default), and git-missing-from-PATH (non-zero exit → no-launch). The earlier version would have stalled or failed open; this version fails closed on everything unrecognized.

The Codex P2 finding — not a security issue in this context.
The review comment at SKILL.md line 83 argues that a consumer with a WorktreeCreate hook configured in a non-git directory could have the non-repo branch launch a session that moves into a hook-created workspace, leaving non-git VCS changes behind. This is accurate as a description of hook behavior, but it is not a security issue introduced by this PR:

1. The dirty-tree gate has always been git-scoped. Its purpose is to protect uncommitted git changes. Non-git VCS changes (Mercurial, Perforce, SVN) were not protected before this PR either.
2. In a non-git directory there is no git tree and no git-tracked uncommitted work by definition. The gate's core security property — don't launch when uncommitted git work would be stranded — is preserved unconditionally on the non-repo branch.
3. The WorktreeCreate hook concern is a behavioral accuracy issue (the launch report's statement that "sessions write to the working directory directly" may be wrong when the hook is active), not a dirty-tree or unauthorized-launch issue. This is out of scope for a fix whose mandate is the non-repo gate (session-flow: continue-in-background dirty-tree gate unspecified when consuming directory is not a git repository #929).

Command injection surface — unchanged.
The launch command's slug sanitization ([a-z0-9-] only) and the write-prompt-to-temp-file-then-$(cat) pattern pre-exist this PR and are unchanged. No new injection surfaces are introduced.

Privilege / authorization model — unchanged.
CLAUDE_CODE_FORCE_SESSION_PERSISTENCE=1 is a pre-existing override documented in the env-vars page; this PR neither adds it nor widens its use.

Summary: Nothing in this diff constitutes a new security finding. The Codex P2 item is a correctness observation about hook behavior documentation — worth its own issue if the team wants to tighten the wording — but it does not represent a vulnerability introduced here.

[claude]

Claude finished @kyle-sexton's task in 2m 3s —— View job

---

PR Review — fix(session-flow): continue-in-background non-repo gate

• Gather context
• Read changed files (SKILL.md, evals.json, CHANGELOG.md, plugin.json)
• Review gate logic and exhaustiveness
• Check eval correctness and non-vacuousness
• Verify prior review findings addressed
• Post final review

---

What's new in this update

The latest commit (1eabb8d) addresses both findings raised in the prior Claude review:

Finding 1 (WorktreeCreate hook not checked) — Fixed. The non-repo branch in SKILL.md (lines 79–86) now makes hook absence a required condition, not a parenthetical note. The language is explicit: a configured hook, or an absence that cannot be established, falls to the unknown default and does not launch. The logic at SKILL.md#L84–86 is correct:

"a configured one — or an absence you cannot establish — puts the launched session in a workspace this checkout's local changes never reach: that is the default branch below, not this one."

Observation (eval #5 missing hook qualifier) — Fixed. Eval #5's expected_output, expectations, and prompt now all include the hook check. A new eval #8 (non-repo-with-worktreecreate-hook-does-not-launch) was added to exercise the hook-configured case directly.

---

Gate review — exhaustive by construction

The final gate structure at SKILL.md#L75–97:

rev-parse result	Hook check	Behavior
prints true	—	Inspect tree
fails with "not a git repository"	absence established	Launch
fails with "not a git repository"	hook present or absence not established	UNKNOWN → no launch
anything else (including false at exit 0)	—	UNKNOWN → no launch

All four cases are correct. The "cannot establish absence" path falling to the unknown default is the load-bearing piece — it makes the gate conservative on ambiguity rather than permissive.

The "never route by exit status alone" principle is stated at SKILL.md#L94–97 and correctly covers both directions: non-zero exit ≠ "no repository", zero exit ≠ clean tree.

Evals review

All four new evals are non-vacuous and cover the discriminating cases:

Eval	Tests	Non-vacuousness
#5 non-repo-directory-launches	Non-repo + no hook → launches, hook check required	Gate must positively identify both conditions; pre-fix code had no branch at all
#6 unknown-tree-state-does-not-launch	Dubious-ownership failure → no launch	Proves gate does not fail open on non-zero exit that isn't "not a git repository"
#7 no-work-tree-despite-zero-exit-does-not-launch	Bare repo (false at exit 0) → no launch	Proves zero exit alone is not a passing gate
#8 non-repo-with-worktreecreate-hook-does-not-launch	Non-repo + hook present → no launch; inability to establish absence treated same	Guards the hook condition in both directions

Eval #8's expectation — "An inability to establish the hook's absence is treated the same way — unknown, not clean, no launch" — correctly aligns with the skill's language.

The expected_output for eval #5 now includes both conditions ("establishes that no WorktreeCreate hook is configured"), resolving the prior observation.

Everything else

• Checklist item (SKILL.md#L169–175) correctly captures the hook condition: "any hook whose absence is not established → state unknown, no launch."
• Context-block scoping unchanged and correct — the "treat any failure and carry on" shrug is explicitly scoped to the context-gathering block and not to the gate.
• CHANGELOG entry at 0.17.17 accurately describes four new evals and the hook condition.
• Version bump 0.17.16 → 0.17.17 paired correctly with the changelog entry.

No issues remaining. Both prior review findings are fixed, the gate is exhaustive and fails closed on everything unrecognized, and the four evals are non-vacuous. Ready to merge.

[claude]

Claude finished @kyle-sexton's task in 2m 1s —— View job

---

Security Review — PR #1728

• Read changed files (SKILL.md, evals.json, plugin.json, CHANGELOG.md)
• Analyze gate logic for logic flaws and injection surfaces
• Review command execution surfaces
• Check trust-boundary and authorization model
• Post findings

---

No security issues to report.

This PR changes a Markdown instruction file (SKILL.md), evaluation test cases (evals.json), a version bump (plugin.json), and a changelog entry (CHANGELOG.md). There is no executable code in the diff. My analysis focused on the instruction logic the skill directs an agent to follow, the commands the gate runs, and whether the latest commit (1eabb8d) closed the P2 finding cleanly.

Analysis

Gate structure — conservative by construction.
The new two-step gate (git rev-parse --is-inside-work-tree, then git status --porcelain -uall on true) has a deliberately wide unknown default: anything that is not a positive true or a confirmed "not a git repository with no WorktreeCreate hook" stays on the no-launch path. Specifically:

• Bare repository / .git directory → exit 0, prints false → unknown default, no launch. SKILL.md line 89–92 names this case and confirms it lands in the default, not the non-repo branch.
• Dubious ownership (non-zero exit, different message) → unknown default, no launch.
• Git absent from PATH → unknown default, no launch.
• Non-repo + WorktreeCreate hook present → unknown default, no launch (the gate now treats this identically to unknown).
• Non-repo + hook absence cannot be established → unknown default, no launch (the eval #8 expectation makes this explicit).

The only way off the no-launch default is two positively identified states. This is fail-closed throughout.

WorktreeCreate hook gap — addressed in the latest commit.
The Codex P2 finding and the prior review's Finding 1 both flagged that a non-Git consumer with a WorktreeCreate hook could fire the non-repo launch branch despite having isolation configured. Commit 1eabb8d elevates hook-absence from a parenthetical premise to a mandatory condition: the non-repo branch now requires "not a git repository AND no WorktreeCreate hook configured" explicitly, with the addition "or an absence you cannot establish — puts the launched session in a workspace this checkout's local changes never reach: that is the default branch below, not this one." The checklist item (line 173–175) and eval #8 are consistent with this. The gap is closed.

Injection surfaces — unchanged.
The skill runs git rev-parse --is-inside-work-tree against no user-supplied input. Its output is pattern-matched against two well-known strings (true, "not a git repository"); the comparison does not feed back into any shell command. The pre-existing prompt-to-temp-file pattern ($(cat "<prompt-file>")) and slug sanitization ([a-z0-9-] only) are unchanged. No new injection surface.

Output matching and locale.
The non-repo branch depends on matching git's "not a git repository" message. If git is localized, this string may differ — in which case the case falls to the unknown/no-launch default. That is safe behavior (conservative, not permissive), and not a security issue.

Hard gate and authorization model — unchanged.
The explicit-user-intent gate for background delegation (## Hard gate) is untouched by this PR. The dirty-tree gate changes are purely additive.