fix(backlog): self-heal already-tracked scaffolding files, add CI backstop#195
Merged
Conversation
…kstop .backlog-context.md and .claude/commands/backlog/*.md are written into every backlog work-session worktree, then git-excluded via $GIT_DIR/info/exclude. That only stops NEW files from being staged — it does nothing once a file is already tracked, which is why this repo's own history has ~20 manual "chore(backlog): untrack backlog context/command files" commits, and why two currently-open PRs (#169, #170) both carry an incidental deletion of a stale, committed .backlog-context.md from an unrelated item's context. WriteSlashCommands and WriteBacklogContextFile now call selfHealWorktreeScaffolding before writing: it walks the git index (via go-git, not a subshell) for any entry matching backlogExcludePatterns and untracks it (git-rm-cached semantics — working tree file is left alone), so a branch that ever got one of these files committed self-heals on the very next spawn/reattach/reopen instead of requiring a manual untrack commit. Also adds a CI backstop (.github/workflows/backlog-scaffolding-guard.yml) that fails any PR whose diff adds/modifies a backlogExcludePatterns file, as a second, independent layer for this repo. This only protects stapler-squad's own repo — it cannot reach into the arbitrary target repos backlog items point at, which is why the self-heal above (not CI) is the primary fix. On the "scoped by session/backlog item so it's not reused poorly" half of the request: chose to harden the existing worktree-relative storage rather than relocate it to a session/item-scoped path outside the worktree (~/.stapler-squad/backlog-context/<item>/...). Both WriteBacklogContextFile and WriteSlashCommands already do a full atomic overwrite on every spawn/reattach/reopen, so no session ever reads content from a different item/session — the actual "reused poorly" risk was exclusively the already-tracked git pollution this fix closes. Relocating storage is a larger, real architecture change (touches taskProtocolBlock's literal `.backlog-context.md` references, needs a session UUID that doesn't exist yet at the pre-spawn write callsite, and increases collision surface with other agents' concurrent work in these same files) and is left as a follow-up rather than folded into this bug fix. CleanupSlashCommands/CleanupBacklogContextFile remain uncalled from any production teardown path, and that's intentional, not an oversight: shipViaAgentOrFallback relies on ship.md still existing after a work session exits review to re-invoke /backlog/ship as a one-shot call. Wiring cleanup into a review-exit teardown would delete ship.md out from under that path. Documented this directly in both functions' doc comments so it isn't "fixed" again by mistake. Regression tests: a worktree with .backlog-context.md (or .claude/commands/backlog/status.md) already committed gets it untracked on the next Write call with fresh on-disk content; respawning the same worktree path for a different item never leaks the prior item's content; the self-heal helper no-ops cleanly on a non-git directory and leaves unrelated tracked files alone. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BxNAMeGteuzNyN46Q4zAn1
Contributor
✅ Registry ValidationTest Coverage: 17/178 features have
|
Contributor
Go Benchmarks (Tier 1) |
Contributor
E2E RPC Latency |
Contributor
📊 Feature E2E CoverageFeature E2E coverage: 5/178 tested (3%)
|
Contributor
Frontend Terminal Throughput |
tstapler
added a commit
that referenced
this pull request
Jul 22, 2026
…olding files (#206) * fix(backlog): guard commit/push staging against already-tracked scaffolding files .backlog-context.md and .claude/commands/backlog/* were only kept out of the repo at write time (selfHealWorktreeScaffolding, added in #195) and via gitignore/info-exclude — neither stops a later `git add .` in GitWorktree.CommitChanges/PushChanges or QuickCommitPush from restaging a copy that's already tracked in the branch's history, which is how ~20 manual "chore(backlog): untrack ..." commits accumulated over time. GitWorktree.StageAllExceptScaffolding (session/git/worktree_git.go) now runs `git add .` and then untracks any path matching the shared git.ScaffoldingExcludePatterns list, used by both CommitChanges and PushChanges; HasStagedChanges lets both skip the commit gracefully instead of erroring when the only staged change was scaffolding that just got untracked. QuickCommitPush applies the identical guard via the same GitWorktree methods. Consolidated the previously-duplicated exclude-path list (backlogExcludePatterns in session/backlog_commands.go, plus hardcoded strings elsewhere) into a single exported git.ScaffoldingExcludePatterns (session/git/scaffolding.go), consumed by the worktree-write side, the new staging guard, and QuickCommitPush. Added a test asserting it stays in sync with .gitignore. CleanupBacklogContextFile/CleanupSlashCommands were dead code with zero production call sites; ReconcilePRPending now invokes them once an item's PR is confirmed merged and it reaches done — after ship.md's one-shot-reinvoke constraint no longer applies. Skipped a scripted mass remediation across the ~5 backlog/* branches still carrying tracked copies: origin/main is already clean, and the remaining branches are actively-cycling autonomous backlog sessions (one on revision r7) rather than dead branches — the self-heal plus this commit-time guard already untrack them the next time each is touched, so an out-of-band push was unnecessary risk for no real benefit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZLdyDZJ1urzJ8FcMFpMMu * address code review: serialize UntrackScaffolding per worktree, dedupe stage/commit, fix test gap - UntrackScaffolding now takes a per-worktree-path mutex before its index read-modify-write: go-git's Storer.Index()/SetIndex() don't participate in git's index.lock protocol, and adding more call sites (CommitChanges, PushChanges, QuickCommitPush) on top of the existing write-time self-heal meaningfully increases the odds of two concurrent callers racing on the same worktree's index. - Extracted stageAndCommit as the one shared stage/commit/skip-if-empty sequence used by both CommitChanges and PushChanges, removing the duplicated block. - Softened StageAllExceptScaffolding's doc comment, which overclaimed scaffolding files "can never be committed" when the untrack step deliberately fails open on error (mitigated by the separate CI backstop workflow, not a behavior change). - Fixed TestStageAllExceptScaffolding_LeavesUnrelatedTrackedFilesAlone, which never actually exercised the "except scaffolding" half of its own name — it only asserted a normal file gets staged. Now also asserts a scaffolding file added alongside it is excluded. Found via /code:review (testing, code-quality, architecture, security agents + skeptic pass) on PR #206. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a chronic, confirmed bug:
.backlog-context.mdand.claude/commands/backlog/*.md— scaffolding files stapler-squad writes into every backlog work-session worktree — keep getting committed to target repos despite being "gitignored." This repo's own history has ~20 manualchore(backlog): untrack backlog context/command filescommits, and two currently-open PRs (#169, #170) each carry an incidental deletion of a stale, committed.backlog-context.mdleft over from an unrelated item's context.Root cause
WriteBacklogContextFile/WriteSlashCommands(session/backlog_commands.go) write scaffolding files, then calladdWorktreeExcludesto add patterns to$GIT_DIR/info/exclude. That — and a tracked.gitignoreentry — only stop new files from being staged by a broadgit add/git commit -a. Neither does anything once a file is already tracked in the branch's history, which is exactly what a long chain of manual "untrack" commits has been working around.What changed
1. Self-healing untrack (
session/backlog_commands.go)WriteSlashCommandsandWriteBacklogContextFilenow callselfHealWorktreeScaffoldingbefore writing. It opens the worktree's git index directly via go-git (repo.Storer.Index()/idx.Remove()/repo.Storer.SetIndex()— no subshell, per.claude/rules/prefer-go-git-over-subshells.md), finds any entry matchingbacklogExcludePatterns, and untracks it (git-rm-cached semantics: index entry removed, working-tree file left alone). A branch that ever got one of these files committed — however it happened — self-heals the very next time a session is spawned, reattached, or reopened on it, closing the loop that's needed ~20 manual commits so far.2. CI backstop (
.github/workflows/backlog-scaffolding-guard.yml)New workflow fails any PR whose diff adds/modifies a file matching
backlogExcludePatterns. This is a second, independent layer for stapler-squad's own repo only — it structurally cannot reach into the arbitrary target repos backlog items point at (stapler-squad has no mechanism to impose new CI on repos it doesn't own), which is why item 1 above, not this workflow, is the primary defense for those.3. "Scoped by session/backlog item" — decision: harden in place, don't relocate
The request asked me to evaluate moving
.backlog-context.mdout of the worktree entirely (e.g.~/.stapler-squad/backlog-context/<item-id>/...) so it can never be tracked in the first place. I considered this (option b) against hardening the existing worktree-relative storage (option a) and chose (a):WriteBacklogContextFile's tmp-file-then-rename) on every spawn/reattach/reopen — no session ever reads content left behind by a different item or session. The actual "reused poorly" risk was exclusively the already-tracked git pollution, not content staleness — that risk is closed by fix ci: fix forbidigo pattern field for golangci-lint 1.60.1 #1.SpawnSessionFromItem'swriteSessionFilescall site, which today runs before the session (and its UUID) is created; it requires rewritingtaskProtocolBlock's literal.backlog-context.mdreferences (session/backlog_context.gorules 5–6) to an absolute path; and.claude/commands/backlog/(unlike the context file) must stay inside the worktree for Claude Code to discover it as project-scoped slash commands, so only half the scaffolding could move anyway.session/backlog_commands.goandsession/backlog_lifecycle.gothis week — a storage relocation meaningfully increases collision surface with that work for a single bug-fix session.I'm flagging this explicitly as a scoped-out follow-up rather than hand-waving it: relocating storage is worth doing eventually (a file that was never inside the git working directory can never be accidentally committed, full stop), but it's an ADR-sized decision, not a bug-fix-sized one.
4.
CleanupSlashCommands/CleanupBacklogContextFile— audited, left uncalled on purposeNeither function has a production call site (confirmed via
grep— only tests call them). I initially assumed this was a bug ("cleanup isn't wired up") but tracingshipViaAgentOrFallback(session/backlog_lifecycle.go) surfaced why: it relies onship.mdstill existing in the worktree after a work session exits review, so it can re-invoke/backlog/shipas a one-shot headless call. WiringCleanupSlashCommandsinto any review-exit teardown would deleteship.mdout from under that path. I documented this directly in both functions' doc comments so a future pass doesn't "fix" this dead code in a way that breaks shipping. Worktree teardown (Instance.Kill/Instance.Pause) already removes the whole directory anyway, and untracked files never appear in a git diff/PR regardless of how long they linger on disk — so there was no actual gap to close here beyond fix #1.Test plan
session/backlog_commands_test.go:TestWriteBacklogContextFile_UntracksPreviouslyCommittedContextFile— commits.backlog-context.mdto a real git repo, then verifies the nextWriteBacklogContextFilecall untracks it (still on disk, no longer in the index, fresh content).TestWriteSlashCommands_UntracksPreviouslyCommittedSlashCommandFiles— same for.claude/commands/backlog/status.md.TestWriteBacklogContextFile_NoStaleContentLeaksAcrossRespawn— writes for item A, then item B, at the same worktree path; asserts no cross-contamination.TestUntrackTrackedScaffolding_NoErrorOnNonGitDirectory/TestUntrackTrackedScaffolding_LeavesUnrelatedTrackedFilesAlone— safety/scoping checks on the new helper.go test ./session/...— 3069 passedgo test ./server/services/...— passedmake build— succeeds (Go + web UI)make lintcustom-lint failures present are pre-existing and unrelated (session/unfinished/gogitstore/*_test.gonorawexecfindings, untouched by this diff);golangci-lint run ./session/...reports zero findings for the changed fileactionlintclean on the new workflow file; regex manually verified against sample pathsOne unrelated, confirmed-flaky test (
TestEnsureServerRunning_NoOpinsession/tmux, real tmux server contention) failed once undermake testand passed cleanly in isolation and in a full-count=1re-run of the whole package (228/228) — not related to this change.Reviewer notes
session/backlog_commands.go, but both are alreadyCONFLICTINGagainstmainindependent of this PR (stale since 2026-07-18) and their diffs to that file only re-add a block already merged intomain— no actual line overlap with this change..backlog-context.mdto a session/item-scoped path outside the worktree (option b above) — a real architecture change, not folded in here.