Skip to content

Fix: lifecycle commits stage only what the change owns, never git add -A - #795

Merged
0xLeif merged 4 commits into
mainfrom
fix/no-git-add-all
Sep 26, 2026
Merged

0xLeif merged 4 commits into
mainfrom
fix/no-git-add-all

Conversation

@0xLeif

@0xLeif 0xLeif commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Bug: specsync change check --commit and specsync change ship --push committed through git_commit_all, which ran git add -A. That staged every untracked, non-ignored file in the project, and --push published it. This week that put a private debug zip into a pushed arcsite commit, and .agents/ and exp2.sh into corvid-bot commits.
  • Fix: the staging path (git_commit_lifecycle in src/commands/change.rs) stages two things, using explicit git --literal-pathspecs add -- <paths>:
    • every edit to a tracked file, because verification digests the working tree and a verified tracked edit has to be committed;
    • the untracked paths the change domain says the change owns, from the new change::lifecycle_commit_scope: its workspace .specsync/changes/<id>/, its archive package, each affected spec's canonical *.spec.md and companions, and the lifecycle ledgers (change-sequence.json, workflow-v2-baseline.json, archive/legacy-baseline.json, bootstrap.json, hashes.json).
  • Never staged: any other untracked file. Those files are listed once per run on stderr with what to do about them. After check --commit the warning also says that removing one stales the recorded verification, and names the command that re-records it. --format json stdout is unchanged. The lock and transaction journal are neither committed nor listed, and unmerged entries are not staged, so an unresolved conflict still stops the commit.
  • Not owned on purpose: the change's affected_paths prefixes (src/, ., …) and whole spec directories, because owning them would bring the sweep back.
  • Behavior change for authors: a brand-new untracked source file is no longer swept in, so git add it before check --commit. This is documented in docs/ADOPTING.md and the AGENTS.md quick reference.
  • Doc comment: run_checked_commit said "nothing is committed unless verification passes", which is true for the first pass only. I took the "fix the comment" option rather than auto-rewinding a commit. The comment now describes what happens, and when the second-pass verification fails the error names the materialize commit it left in place and the resume command.
  • Found while testing: change new writes .specsync/workflow-v2-baseline.json in a freshly adopted project. It is a lifecycle ledger, so it stays in the commit (asserted in the check test).
  • SpecSync change: lifecycle-commits-stage-only-what-the-change-owns-never-every-untracked-file (bug-fix; specs cmd_change and change). Deltas: MODIFIED REQ-cmd-change-012 (drops the git add -A wording), ADDED REQ-cmd-change-017, ADDED REQ-change-102, and the Public API rows for LifecycleCommitScope / lifecycle_commit_scope. The change is a draft awaiting your definition approval; see below.

Test Plan

  • check_commit_never_commits_an_unrelated_untracked_file: run_checked_commit over a tree with debug-dump.zip, .agents/scratch.md and exp2.sh. No stray appears anywhere in history and all stay untracked. Controls: the untracked workspace, the workflow-v2 baseline and the tracked delivery edit are committed.
  • ship_push_never_commits_an_unrelated_untracked_file: check --commit → review → run_ship --push into a bare remote. No stray in the remote's history, the archive package is pushed, and the vacated workspace is gone from the pushed tree.
  • Discrimination: with staging put back to git add -A, both tests fail (debug-dump.zip was committed). With only the archive commit put back, the ship test still fails.
  • Unit tests: lifecycle_commit_scope_names_exactly_what_the_change_owns (exact set), staging_reads_each_porcelain_entry_once_and_stages_only_tracked_edits (rename parsing; fails if the source field is misread), a_lifecycle_scope_owns_its_subtree_and_not_a_prefix_sibling, the_left_out_warning_names_the_files_and_what_to_do, and the renamed lifecycle_commit_raises_a_stale_ledger_before_staging_it.
  • fledge lanes run verify: fmt, clippy -D warnings, check, full cargo test (2504 unit and 437 integration tests, 0 failures), release build, strict spec check 62/62 at 100% coverage, and the RC tests.
  • fledge lanes run pre-push and fledge trust verify pass (Augur risk 48, "review" tier; provenance is soft and gets attested on main by CI).
  • Rehearsed the post-approval lifecycle in a throwaway clone with a fake actor, never pushed. approve → check --commit (stray left out and warned about) → change audit --strict → review → ship → archive all pass, and audit --strict passes after the archive commit.
  • Definition approval, check --commit, review, and ship (steps below)
  • CI green. The lifecycle gate (change audit --strict) fails until the change is approved and checked, because a draft covers no paths.

For @0xLeif: SpecSync steps (not recorded by the agent)

I did not record a definition approval or a review under your identity. From a checkout of this branch:

specsync change approve lifecycle-commits-stage-only-what-the-change-owns-never-every-untracked-file --actor user:0xLeif
cargo run --quiet -- change check lifecycle-commits-stage-only-what-the-change-owns-never-every-untracked-file --commit
git push
# wait for CI, review the PR
specsync change review lifecycle-commits-stage-only-what-the-change-owns-never-every-untracked-file --reviewer user:0xLeif
cargo run --quiet -- change ship lifecycle-commits-stage-only-what-the-change-owns-never-every-untracked-file --push
# wait for CI green, then merge

cargo run -- runs this branch's binary, so the lifecycle commits themselves use the fixed staging.

Human intent

This relates to SHIP-1.d ("I can commit the verified result in the same step…"). A candidate criterion, not captured because it needs your words: "Committing the verified result never sweeps in files I did not put there."

🤖 Generated with Claude Code

https://claude.ai/code/session_01V3ZZAEiUP7xRJPozhZb6rL

`change check --commit` and `change ship --push` committed through
`git_commit_all`, which ran `git add -A`. That staged every untracked,
non-ignored file in the project, and `--push` published it: a private
debug zip reached a pushed arcsite commit, and `.agents/` and `exp2.sh`
reached corvid-bot commits.

The staging path (`git_commit_lifecycle`) now stages every tracked edit
plus only the untracked paths `change::lifecycle_commit_scope` says the
change owns: its workspace, its archive package, each affected spec's
canonical file and companions, and the lifecycle ledgers. It uses
explicit `--literal-pathspecs`. Every other untracked file is left
unstaged and listed once per run on stderr, with what to do about it.
The project lock and transaction journal are neither committed nor
listed, and unmerged entries are not staged.

`run_checked_commit`'s doc comment claimed nothing is committed unless
verification passes. That holds for the first pass only. The comment
now says so, and a second-pass failure names the materialize commit it
leaves in place.

Regression tests drive check --commit and ship --push (into a bare
remote) over a tree with unrelated untracked files. Both fail with
`git add -A` restored.

SpecSync change: lifecycle-commits-stage-only-what-the-change-owns-never-every-untracked-file
(draft; definition approval is left to the owner).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V3ZZAEiUP7xRJPozhZb6rL
@0xLeif
0xLeif requested a review from a team as a code owner September 26, 2026 05:41
@0xLeif
0xLeif requested review from 0xGaspar, Kyntrin and tofu-ux and removed request for a team September 26, 2026 05:41
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Corvin says...

      _
    <(^\  .oO(Caw! ^v^)
     |/(\
      \(\\
      " "\\

"Caw! Your code sparkles like a dropped french fry."

CI Summary

Check Status
Validate action.yml ✅ Passed
Packaged Action Consumer ✅ Passed
Dependency Audit ✅ Passed
Code Coverage ✅ Passed
Format Check ✅ Passed
Human intent check ✅ Passed
Docs Site ✅ Passed
Spec Validation ✅ Passed
Tests (build, test, clippy) ✅ Passed
VS Code Extension ✅ Passed
📋 Spec Validation Details

✅ SpecSync: Passed

Metric Value
Specs checked 62
Passed 62
Errors 0
Warnings 0
File coverage 100% (107/107)
LOC coverage 100% (149230/149230)

Generated by specsync · Run specsync check --format github to reproduce


Powered by corvid-pet

@0xLeif
0xLeif merged commit cddc39e into main Sep 26, 2026
22 checks passed
@0xLeif
0xLeif deleted the fix/no-git-add-all branch September 26, 2026 06:22
0xLeif added a commit that referenced this pull request Sep 26, 2026
* Fix: Required CI gate fails when the lifecycle gate fails (#796)

`Required CI gate` passes when `implementation-gate` ("SpecSync
implementation ready") passes. That job needed neither `preflight` nor
`lifecycle-gate`, and it accepted `success` or `skipped` from every job
it did need. When the lifecycle gate failed, `test`, `audit`, `coverage`
and `spec-check` were skipped rather than failed, and both gates went
green. #795 showed it at bb1d80f.

`implementation-gate` now needs every job that can finish before it,
`preflight` and `lifecycle-gate` included. Its step reads one row per
job: the job's own `if:` (less a leading `always() &&`), evaluated again
over the same classify outputs, and the job's result. `skipped` passes
only where that says classify deselected the job. A selected job that
was skipped had a dependency that did not succeed, and it fails the
gate. The old check that nothing in `needs` failed or was cancelled
stays, and the gate fails if its rows do not cover `needs`.

`.github/scripts/test-required-ci-gate.py` guards it. It fails if
`preflight` or `lifecycle-gate` leaves `implementation-gate.needs`, if a
job that gates on `lifecycle-gate` or otherwise runs before the gate is
missing, or if a row drifts from its job's `if:`. It simulates the job
graph for every classify lane and flag combination under
pull_request, push and workflow_dispatch, runs the gate's own bash, and
requires the required gate to be green when every selected job succeeds
and red when any one fails or is cancelled. The pre-#796 gate,
kept as a fixture, reproduces the bug in the same simulation. It runs
in `validate-action` and as the Fledge task `ci-gate-test` in the
verify, ci and repo lanes.

SpecSync change: required-ci-gate-fails-when-the-lifecycle-gate-fails
(draft; definition approval is left to the owner).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V3ZZAEiUP7xRJPozhZb6rL

* Fix: simulate gate steps under bash -e, as the runner runs them (#796)

The CI log for the first push shows the gate's step running as
`/usr/bin/bash -e {0}`: a step that names no shell gets `bash -e`, not
the `bash --noprofile --norc -eo pipefail` that `shell: bash` gets. The
simulation now uses the same invocation for each case. Results are
unchanged.

Record the live check in the change's testing notes: on the unapproved
draft `Lifecycle gate` failed, `test`, `audit`, `coverage` and
`spec-check` were skipped, and `SpecSync implementation ready` and
`Required CI gate` both failed, with one annotation per cause.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V3ZZAEiUP7xRJPozhZb6rL

* chore(lifecycle): materialize required-ci-gate-fails-when-the-lifecycle-gate-fails

* chore(lifecycle): record required-ci-gate-fails-when-the-lifecycle-gate-fails verification

* chore(lifecycle): archive required-ci-gate-fails-when-the-lifecycle-gate-fails

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant