Skip to content

Add per-repo autocommit opt-in for /close archiving#49

Open
gering wants to merge 3 commits into
mainfrom
task/add-close-autocommit-optin
Open

Add per-repo autocommit opt-in for /close archiving#49
gering wants to merge 3 commits into
mainfrom
task/add-close-autocommit-optin

Conversation

@gering

@gering gering commented Jul 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Summary

  • /close step 10's "commit the archived task file to main and push? [y/n]" prompt is pure friction in repos where archiving-to-main is the established norm (dotfiles, this repo). This adds a per-repo opt-in that skips it.
  • A committed .claude/work-system-close-autocommit flag routes /close straight to archive-task.sh commit-push — no AskUserQuestion — while still printing the same scoped git status preview first. Standing authorization is not authorization to act unseen.
  • Off by default; unset repos keep today's ask-once behavior exactly. Per-repo only, no global default.
  • The flag is the durable authorization for exactly one narrow action — the archive-metadata commit+push — not a general loosening of the "ask before pushing to the default branch" rule.

Trust model (the part worth reviewing)

Presence does not authorize anything. get reads the value from the committed object on refs/heads/<main-branch>:

  • Fully qualified, because git resolves refs/tags/ before refs/heads/ — a tag named like the default branch (tags auto-follow on fetch) otherwise supplied a value the branch never carried.
  • From the commit, not the working tree, so git update-index --assume-unchanged/--skip-worktree cannot hide an edit that a diff-based guard would call clean.
  • Fail-closed everywhere: unresolvable branch, checkout not on that branch, symlinked flag or .claude, unsupported layout, unreadable git state → enabled=no, which merely restores the prompt.

Stated honestly: this raises the bar from "anything can write a file" to "something had to commit" — not to "a human reviewed it". Whoever can commit in your repo can commit this too. What bounds the damage is commit-push itself: archive-scoped pathspec, fast-forward only, never a force-push, refused when main carries other unpushed commits. Worst case is inert archived markdown reaching main unreviewed.

Changes

  • archive-task.sh — new autocommit get|set|unset subcommand as the single source of truth for the flag. Resolution delegates to main-repo-path.sh (the same resolver /close uses, so set cannot write where get won't look) and is cross-checked as a real work tree. set/unset refuse a symlinked or wrong-typed flag or .claude parent, re-validate after mkdir, and write via an exclusively-created mktemp + rename. commit-push passes :(literal) pathspecs so a task named x* cannot sweep in other archives.
  • close/SKILL.md — passes <main-branch>, shows the preview on the auto path, and authorizes only on a whole-line enabled=yes match (never a substring — note= can carry repo-controlled text). Any other output, including none, falls through to asking.
  • test_archive_task.py — new suite against real git repos, with regression tests for each reproduced bypass: tag shadowing, the index bits, the symlinked .claude parent, relative-path helper resolution, glob-y task names, gitignored .claude/.
  • Docs — knowledge entry (decisions, gotchas, named residuals), README, CHANGELOG, CLAUDE.md version marker.
  • Version — work-system 1.9.4 → 1.10.0 (minor: new config surface), plugin.json + marketplace.json in sync.

Review provenance

Five /swarm:review --loop rounds (claude + codex + grok), 52 findings, 51 applied. Four criticals were self-inflicted — each round's hardening created the next round's bug: the assume-unchanged bypass, $(dirname $0) resolving inside a cd'd subshell (ran scripts from the target repo), :(literal) passed to check-ignore (broke gitignore detection), and the tag-shadowing above. All reproduced, fixed, and pinned by tests.

Deliberately not done, with reasons in the knowledge entry: requiring the flag's commit to be an ancestor of origin/<default> (breaks origin-less repos, disproportionate to the blast radius), and migrating agent-registry.sh to the shared resolver (sits on the /kickoff hot path — follow-up).

Known residuals: separate-git-dir layouts are refused rather than supported (pre-existing, belongs in main-repo-path.sh); the write-path TOCTOU window is narrowed, not closed (needs openat/O_NOFOLLOW, unavailable in shell).

Readiness

  • ✅ check-structure.py + plugin tests: 0 errors
  • ✅ README, CHANGELOG, knowledge, version sync
  • ➖ Build — N/A (build-less markdown/shell repo)

Test plan

  • With the flag committed on main and the main checkout on main, /close shows the preview, commits+pushes without prompting, and reports the commit-push result.
  • Without the flag, /close asks exactly once (unchanged).
  • set → get reports untracked until committed; a tag named like the default branch does not enable it; a local edit to no reports locally-disabled.

🤖 Generated with Claude Code

gering added 3 commits July 24, 2026 14:07
@gering
Add per-repo autocommit opt-in for /close archiving
6f3db1e 
Route step 10's commit+push prompt straight to archive-task.sh
commit-push when a repo commits .claude/work-system-close-autocommit,
skipping the y/n ask in repos where archiving-to-main is the norm.
Off by default; the flag is an authorization scoped to exactly that
one narrow, already-guarded action (pathspec commit, ff-only, never
force-pushes).

- archive-task.sh: new `autocommit get|set|unset` subcommand
- close/SKILL.md: check the flag before asking, same result reporting
  either way
- test_archive_task.py, docs, CHANGELOG, work-system 1.9.4 -> 1.10.0
@gering
Harden the /close autocommit opt-in after swarm review
8908405 
Three review rounds against the initial cut found the flag's trust model
too weak and the write path unsafe. Presence no longer authorizes
anything: the value is read from the committed object on <main-branch>,
so neither a file a tool merely wrote nor a working-tree edit hidden by
--assume-unchanged/--skip-worktree can waive /close's push prompt, and
the verdict no longer depends on which branch the checkout sits on.

- get: read from <main-branch>:<rel>; bare enabled=no when nothing is
  configured, else reason= + a ready-to-print note= (callers relay it
  instead of restating the vocabulary); locally-disabled is reported
  neutrally rather than told to revert itself
- set/unset: delegate resolution to main-repo-path.sh and cross-check it
  is a real work tree; refuse a symlinked or wrong-typed flag/.claude and
  re-validate after mkdir; mktemp+rename instead of a PID-derived temp
  path; warn when .claude/ is gitignored; remind to commit the deletion
- commit-push: :(literal) pathspecs so a task named `x*` cannot sweep in
  other archives (not for check-ignore, which rejects pathspec magic);
  archive() uses mktemp too
- close/SKILL.md: pass <main-branch>, show the scoped status preview on
  the auto path, fail closed on any non-enabled=yes output
- docs: state what the guard is and is NOT worth (it raises the bar to
  "a commit", not "human-reviewed"); CLAUDE.md marker -> v1.10.x

Scope stated honestly in the knowledge entry: agent-registry.sh still
open-codes the old resolver, and separate-git-dir layouts are refused
rather than supported.
@gering
Fix tag-shadowing and fail-open paths in the autocommit opt-in
f108e7e 
Round 4 of the swarm review found the authorization ref was read by bare
name, so a tag named like the default branch shadowed refs/heads (git
resolves refs/tags first, and tags auto-follow on fetch) — letting a
value the branch never carried authorize an unattended commit+push.
Verified, and now covered by a regression test.

- get: read refs/heads/<main-branch>, fully qualified; fail CLOSED when
  the branch is unresolvable instead of falling back to HEAD; decline
  when the checkout is not on that branch (commit-push would refuse
  anyway, so enabled=yes only announced a commit that then bailed)
- unset: take the same optional <main-branch> so the "commit this
  deletion" reminder uses the ref that actually grants authorization
- one autocommit_normalize() for both the committed and working-tree
  values; they had drifted (only one rejected multi-line content)
- archive: verify a real regular file is in place before removing the
  source; derive the mode from umask rather than forcing 644; use the
  shared lit() helper for its committable probe
- lit() hoisted to top level, with the actual rule stated: prefix every
  command that interprets pathspec magic, omit for check-ignore, which
  rejects it
- close/SKILL.md: authorize on a whole-line `enabled=yes` match, never a
  substring — note= can carry repo-controlled text
- header/usage/CHANGELOG/knowledge corrected to match the code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@gering