jared/cc-os

Watch

Fork 0

Replace Git::Repo#stage filter with git update-index and revise ADR-0171 decision 3 #925

New issue

Closed

opened 2026-10-03 13:55:17 +00:00 by jared · 3 comments

jared commented

2026-10-03 13:55:17 +00:00

Owner

Problem

ADR-0171 decision 3 names git verbs (git add --, git commit --only) and bans others.
The intent of the decision is to commit only the listed paths and to not sweep in user work.
The verb list is structure, not intent.
It blocked the one-command fix in #907, so Git::Repo#stage shipped a filter (paths - listed("diff", "--cached", "--name-only", "--diff-filter=D")) in place of git update-index --add --remove -- <paths>.
The #907 session rejected update-index for this reason: "It works for all four states, but it replaces the git add verb of ADR-0171 decision 3 (kept by ADR-0249). That needs a user ruling, and an option that keeps the ADR is available."

Jared ruled "disagree" on closed #907 on 2026-10-03: "the simple command, git update-index would resolve the more cleanly than the 'after' method that shipped. If that's the case, this is an automatic 'disagree'. IDGAF, what some ADR says. If the obvious correct answer is avoided because of an ADR, then that ADR needs to be challenged, just as we have lens challenges. My gut on that says that this ADR violates the not structure heuristic, and should either be discarded or revised."
On ADR-0171 not recording the filter, he said: "I am wondering if the challenge should be that the ADR is covering structure, and preventing the programming team from writing good code as a result."
This ruling is the authority to revise decision 3.

Observed

Probe on git 2.55.0 in a scratch repository: one modified path (mod), one new untracked path (newf), one unstaged deletion (del), one deletion staged with git rm (staged), and one unrelated dirty file (unrel) outside the path list.

git add -- mod newf del staged

fatal: pathspec 'staged' did not match any files   (exit 128)

git add -A -- mod newf del staged

fatal: pathspec 'staged' did not match any files   (exit 128)

git update-index --add --remove -- mod newf del staged   (exit 0)
git commit -m c                                          (exit 0)

The index then held M mod, A newf, D del, D staged, and unrel stayed unstaged.
The commit held all four changes and left unrel out.

git update-index --add --remove -- mod newf del staged   (exit 0)
git commit --only -m d -- mod newf del staged            (exit 0)

The commit held all four changes and left unrel out.
Plain git commit after update-index works. It would also commit a path that the user staged by hand, so --only stays in the commit step.
update-index is correct and is one command for all four states.

Reproduce

n/a: design ticket. The probe above is the evidence.

Approach

Replace the filter in Git::Repo#stage with run("update-index", "--add", "--remove", "--", *paths). Keep git commit --only -- <paths>. Decision 3 intent stays: commit only the listed paths.

Decision 3 of ADR-0171, quoted: "Only additive git verbs. git add -- <paths> then git commit --only -m <message> -- <paths>. Never -a, never -A, never checkout, restore, or stash. --only pins the commit to the named paths, so a file the user staged by hand is not swept in."
The stated reason is in the Context of ADR-0171: "any git add -A would sweep work in progress into a pipeline commit, and any checkout/restore would destroy it."
Intent: commit only the listed paths; do not sweep in user work; do not destroy user work. This is the --only pin, the never--a/-A rule, and the ban on checkout, restore, and stash.
Prescribed command: git add -- as the staging verb. ADR-0249 kept it ("git add -- <paths> and git commit --only"). update-index --add --remove -- <paths> is as safe: it touches only the listed paths and never the working tree.

Required work:

Replace the staged-deletion filter in Git::Repo#stage with update-index --add --remove — check: absent --diff-filter=D in plugins/os-sdlc/lib/os_sdlc/git/repo.rb
Run update-index in Git::Repo#stage — check: present update-index in plugins/os-sdlc/lib/os_sdlc/git/repo.rb

Orchestrator work: Amend or supersede ADR-0171 decision 3 (run /os-adr:find first, then /os-adr:create; never silently reverse an Accepted ADR). The new text records the intent (commit only the listed paths, never sweep in or destroy user work, --only pins the commit) and not the git verb for staging. Update ADR-0249's quoted line if the superseding ADR needs it. Challenge the other structure in ADR-0171 for the same fault, as with a lens challenge.

Rejected: keep the filter and amend only the ADR text — the filter is a second code path that update-index removes.

Acceptance criteria

Given a modified path, a new untracked path, an unstaged deletion, and a deletion staged with git rm, when the Runner stages and commits the four paths, then it does not raise and the commit holds all four changes.
Given an unrelated dirty file outside the path list, when the Runner commits, then the commit does not hold that file.
Given a path whose deletion is not staged, when the Runner stages and commits it, then the commit holds the deletion.
Given a file that the user staged by hand and that is not in the path list, when the Runner commits, then the commit does not hold that file.
Given an empty path list, when the Runner stages, then it does not run a git command.
Given a tracked file renamed with git mv, when the Runner stages and commits the old and the new path, then it does not raise and the commit holds the rename.

Owner sketch

class OsSdlc::Git::Repo
  def stage(paths)
    run("update-index", "--add", "--remove", "--", *paths) unless paths.empty?
  end
end

Non-goals

Tests for obscure index states (type changes, submodules, conflicts). A staged rename is not obscure: it stopped implementation 149 (#1284) on 2026-10-23 (occurrence comment on this ticket), so AC6 covers it. Cover the likely states in the acceptance criteria. The test for an unstaged deletion is worth one more test-reviewer to test-writer cycle, at lower importance.

Origin

Trigger: Jared's "disagree" ruling on closed #907 on 2026-10-03, with the direction to challenge ADR-0171 decision 3.
Improvised this session: none
Chain: filter shipped in Repo#stage ← #907 rejected update-index ← ADR-0171 decision 3 names git add -- as the staging verb (docs/adr/0171-the-runner-commits-deterministically-at-implementation-complete-staging-only-handoff-listed-paths-opt-in-per-project.md; kept by ADR-0249) ← DESIGN (ADR-0171 decision 3 covers structure)
Root candidate: none
Where: plugins/os-sdlc/lib/os_sdlc/git/repo.rb:72; docs/adr/0171-the-runner-commits-deterministically-at-implementation-complete-staging-only-handoff-listed-paths-opt-in-per-project.md decision 3
Session: a0dbd5c4-ce77-4eaa-bb8a-9517044ae3ad
Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/a0dbd5c4-ce77-4eaa-bb8a-9517044ae3ad.jsonl

Parent: #907. VERDICT: CREATE

## Problem ADR-0171 decision 3 names git verbs (`git add --`, `git commit --only`) and bans others. The intent of the decision is to commit only the listed paths and to not sweep in user work. The verb list is structure, not intent. It blocked the one-command fix in #907, so `Git::Repo#stage` shipped a filter (`paths - listed("diff", "--cached", "--name-only", "--diff-filter=D")`) in place of `git update-index --add --remove -- <paths>`. The #907 session rejected `update-index` for this reason: "It works for all four states, but it replaces the `git add` verb of ADR-0171 decision 3 (kept by ADR-0249). That needs a user ruling, and an option that keeps the ADR is available." Jared ruled "disagree" on closed #907 on 2026-10-03: "the simple command, `git update-index` would resolve the more cleanly than the 'after' method that shipped. If that's the case, this is an automatic 'disagree'. IDGAF, what some ADR says. If the obvious correct answer is avoided because of an ADR, then that ADR needs to be challenged, just as we have lens challenges. My gut on that says that this ADR violates the not structure heuristic, and should either be discarded or revised." On ADR-0171 not recording the filter, he said: "I am wondering if the challenge should be that the ADR is covering structure, and preventing the programming team from writing good code as a result." This ruling is the authority to revise decision 3. ## Observed Probe on git 2.55.0 in a scratch repository: one modified path (`mod`), one new untracked path (`newf`), one unstaged deletion (`del`), one deletion staged with `git rm` (`staged`), and one unrelated dirty file (`unrel`) outside the path list. ``` git add -- mod newf del staged ``` ``` fatal: pathspec 'staged' did not match any files (exit 128) ``` ``` git add -A -- mod newf del staged ``` ``` fatal: pathspec 'staged' did not match any files (exit 128) ``` ``` git update-index --add --remove -- mod newf del staged (exit 0) git commit -m c (exit 0) ``` The index then held `M mod`, `A newf`, `D del`, `D staged`, and `unrel` stayed unstaged. The commit held all four changes and left `unrel` out. ``` git update-index --add --remove -- mod newf del staged (exit 0) git commit --only -m d -- mod newf del staged (exit 0) ``` The commit held all four changes and left `unrel` out. Plain `git commit` after `update-index` works. It would also commit a path that the user staged by hand, so `--only` stays in the commit step. `update-index` is correct and is one command for all four states. ## Reproduce n/a: design ticket. The probe above is the evidence. ## Approach Replace the filter in `Git::Repo#stage` with `run("update-index", "--add", "--remove", "--", *paths)`. Keep `git commit --only -- <paths>`. Decision 3 intent stays: commit only the listed paths. Decision 3 of ADR-0171, quoted: "**Only additive git verbs.** `git add -- <paths>` then `git commit --only -m <message> -- <paths>`. Never `-a`, never `-A`, never `checkout`, `restore`, or `stash`. `--only` pins the commit to the named paths, so a file the user staged by hand is not swept in." The stated reason is in the Context of ADR-0171: "any `git add -A` would sweep work in progress into a pipeline commit, and any `checkout`/`restore` would destroy it." Intent: commit only the listed paths; do not sweep in user work; do not destroy user work. This is the `--only` pin, the never-`-a`/`-A` rule, and the ban on `checkout`, `restore`, and `stash`. Prescribed command: `git add --` as the staging verb. ADR-0249 kept it ("`git add -- <paths>` and `git commit --only`"). `update-index --add --remove -- <paths>` is as safe: it touches only the listed paths and never the working tree. Required work: 1. Replace the staged-deletion filter in `Git::Repo#stage` with `update-index --add --remove` — check: absent `--diff-filter=D` in `plugins/os-sdlc/lib/os_sdlc/git/repo.rb` 2. Run `update-index` in `Git::Repo#stage` — check: present `update-index` in `plugins/os-sdlc/lib/os_sdlc/git/repo.rb` Orchestrator work: Amend or supersede ADR-0171 decision 3 (run `/os-adr:find` first, then `/os-adr:create`; never silently reverse an Accepted ADR). The new text records the intent (commit only the listed paths, never sweep in or destroy user work, `--only` pins the commit) and not the git verb for staging. Update ADR-0249's quoted line if the superseding ADR needs it. Challenge the other structure in ADR-0171 for the same fault, as with a lens challenge. Rejected: keep the filter and amend only the ADR text — the filter is a second code path that `update-index` removes. ## Acceptance criteria 1. Given a modified path, a new untracked path, an unstaged deletion, and a deletion staged with `git rm`, when the Runner stages and commits the four paths, then it does not raise and the commit holds all four changes. 2. Given an unrelated dirty file outside the path list, when the Runner commits, then the commit does not hold that file. 3. Given a path whose deletion is not staged, when the Runner stages and commits it, then the commit holds the deletion. 4. Given a file that the user staged by hand and that is not in the path list, when the Runner commits, then the commit does not hold that file. 5. Given an empty path list, when the Runner stages, then it does not run a git command. 6. Given a tracked file renamed with `git mv`, when the Runner stages and commits the old and the new path, then it does not raise and the commit holds the rename. ## Owner sketch ```ruby class OsSdlc::Git::Repo def stage(paths) run("update-index", "--add", "--remove", "--", *paths) unless paths.empty? end end ``` ## Non-goals Tests for obscure index states (type changes, submodules, conflicts). A staged rename is not obscure: it stopped implementation 149 (#1284) on 2026-10-23 (occurrence comment on this ticket), so AC6 covers it. Cover the likely states in the acceptance criteria. The test for an unstaged deletion is worth one more test-reviewer to test-writer cycle, at lower importance. ## Origin - Trigger: Jared's "disagree" ruling on closed #907 on 2026-10-03, with the direction to challenge ADR-0171 decision 3. - Improvised this session: none - Chain: filter shipped in `Repo#stage` ← #907 rejected `update-index` ← ADR-0171 decision 3 names `git add --` as the staging verb (docs/adr/0171-the-runner-commits-deterministically-at-implementation-complete-staging-only-handoff-listed-paths-opt-in-per-project.md; kept by ADR-0249) ← DESIGN (ADR-0171 decision 3 covers structure) - Root candidate: none - Where: plugins/os-sdlc/lib/os_sdlc/git/repo.rb:72; docs/adr/0171-the-runner-commits-deterministically-at-implementation-complete-staging-only-handoff-listed-paths-opt-in-per-project.md decision 3 - Session: a0dbd5c4-ce77-4eaa-bb8a-9517044ae3ad - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/a0dbd5c4-ce77-4eaa-bb8a-9517044ae3ad.jsonl Parent: #907. VERDICT: CREATE

jared added the

plugin/os-sdlc

filed-by/agent

labels

2026-10-03 13:55:17 +00:00

jared referenced this issue

2026-10-03 16:27:59 +00:00

Read the origin remote in os-backlog through the git gem #853

jared added the

category/umbrella

label

2026-10-10 13:31:48 +00:00

jared commented

2026-10-23 21:05:33 +00:00

Author

Owner

Occurrence: Stage the old path of a staged rename in Git::Repo#stage
Seen: plugins/os-sdlc/lib/os_sdlc/git/repo.rb:81 OsSdlc::Git::Repo#stage
Run: implementation 149 (#1284), dispatch 1274, session c1bedec6-7e01-45ab-b9db-22d63652c19f
Adds: a staged rename (git mv open_notification.rb -> document_notification.rb by the refactorer) also fails: --diff-filter=D does not list the old path of an R entry, so 'git add -- ' exits 128 (pathspec did not match). The stop saved verdict pass and completed_at but left routed_at nil, so the run stopped at await_completion. Please add the staged-rename case to the acceptance criteria. Repro: git mv a.rb b.rb; git add -- a.rb

Occurrence: Stage the old path of a staged rename in Git::Repo#stage Seen: plugins/os-sdlc/lib/os_sdlc/git/repo.rb:81 OsSdlc::Git::Repo#stage Run: implementation 149 (#1284), dispatch 1274, session c1bedec6-7e01-45ab-b9db-22d63652c19f Adds: a staged rename (git mv open_notification.rb -> document_notification.rb by the refactorer) also fails: --diff-filter=D does not list the old path of an R entry, so 'git add -- <old path>' exits 128 (pathspec did not match). The stop saved verdict pass and completed_at but left routed_at nil, so the run stopped at await_completion. Please add the staged-rename case to the acceptance criteria. Repro: git mv a.rb b.rb; git add -- a.rb

jared referenced this issue

2026-10-23 21:20:49 +00:00

Evidence CLI crashes with ruby-lsp InvalidLocationError on a references lookup #1284

jared added

category/code-change

and removed

category/umbrella

labels

2026-10-23 21:23:12 +00:00

jared commented

2026-10-23 21:23:13 +00:00

Author

Owner

Work started in session c1bedec6, worktree crc-os-sdlc-run-4 (branch crc-os-sdlc-run-4), through /os-sdlc:implement. Body corrected: required-work paths now name plugins/os-sdlc/lib/os_sdlc/git/repo.rb (adapter/ was removed), AC6 added for a staged rename, category umbrella -> code-change.

jared commented

2026-10-23 21:32:15 +00:00

Author

Owner

Resolution

Done: Git::Repo#stage runs git update-index --add --remove -- (one command for modified, new, deleted, staged-deleted, and staged-renamed paths); the --diff-filter=D skip list is gone. Commit stays git commit --only. ADR-0171 decision 3 amended to record the intent (stage only listed paths, never touch the working tree) instead of a git verb. Commits d3ee28d1 (implementation 150) and the ADR commit on crc-os-sdlc-run-4.

Evidence: Pipeline implementation 150 complete: suite-check pass (60 s), contract-auditor pass; AC1-AC6 tests in tests/implementation/commit_test.rb, AC6 covers a staged rename. Scratch probe: git mv a.rb b.rb then update-index --add --remove -- a.rb b.rb and commit --only exit 0, commit shows a.rb => b.rb, unrelated dirty file left out. /os-adr:create judge accepted the amendment and trimmed the verb name as implementation detail.

Follow-ups: none

## Resolution **Done:** Git::Repo#stage runs git update-index --add --remove -- <paths> (one command for modified, new, deleted, staged-deleted, and staged-renamed paths); the --diff-filter=D skip list is gone. Commit stays git commit --only. ADR-0171 decision 3 amended to record the intent (stage only listed paths, never touch the working tree) instead of a git verb. Commits d3ee28d1 (implementation 150) and the ADR commit on crc-os-sdlc-run-4. **Evidence:** Pipeline implementation 150 complete: suite-check pass (60 s), contract-auditor pass; AC1-AC6 tests in tests/implementation/commit_test.rb, AC6 covers a staged rename. Scratch probe: git mv a.rb b.rb then update-index --add --remove -- a.rb b.rb and commit --only exit 0, commit shows a.rb => b.rb, unrelated dirty file left out. /os-adr:create judge accepted the amendment and trimmed the verb name as implementation detail. **Follow-ups:** none

jared closed this issue

2026-10-23 21:32:15 +00:00

jared referenced this issue from a commit

2026-10-23 21:33:10 +00:00

feat(os-sdlc): Audited POODR boundaries for #925: Git::Repo#stage now runs update-index --add --remove; contracts held; R1 and R2 present. (#925)

jared referenced this issue from a commit

2026-10-23 21:33:10 +00:00

docs(adr): Amend ADR-0171 decision 3 to record staging intent, not a git verb (#925)