fix: make agentic-git publishable + close 3 run bugs + smoke tests#9
Merged
Conversation
Adversarial code review (fugu) of main found 3 real defects; all fixed with
regression coverage, plus a CI publish gate and end-to-end smoke tests.
1. **agentic-git crate was not publishable** (the real cause of the failed
release run). `cli.rs`/`shim_phase1.rs` did `include_str!("../../../assets/
hooks/…")` — workspace-ROOT assets that `cargo package` does not put in the
crate tarball, so the verify build failed. Move `assets/hooks/` INTO the
crate (`crates/agentic-git/assets/`), fix the 6 include paths. `cargo
package -p agentic-git` now verifies clean.
2. **`run` could launch UNGUARDED.** `spawn_agent`'s child PATH used
`join_paths(...).unwrap_or(path_env)`; a home path containing ':' made
join fail → silent fallback to the real PATH → the agent's git bypassed the
shim entirely. Now a hard EX_CONFIG refusal.
3. **`.agend-managed` defeated skip-when-clean.** The provisioned marker is an
untracked file, so every fresh session's tree looked dirty and clean-tree
destructive ops still snapshotted. `is_clean` now treats a tree whose only
untracked entry is the marker as clean.
Also: **smoke tests** (`tests/smoke.rs`) drive the shipped binary through the
real `agentic-git run` flow (routing→trailer→snapshot→recover→deny) + pin #2/#3;
**CI `cargo package` gate** (both crates, verify build) so #1 can't regress;
**release.yml `set +e`** so `bash -e` no longer aborts the idempotent publish
on an already-uploaded crate.
cargo test --workspace: 150 green (+3 smoke); clippy clean; both crates package.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…#4) fugu verified the 3 packaging/run fixes, then found a real usability blocker: the recovery layer's OWN documented restore — `git checkout <snapshot-ref> -- .` — was DENIED by the shim. The cross-branch guard reads args[1] as a branch target, so a snapshot ref tripped 'cross-branch — cannot switch to refs/agentic-git/snapshots/...', leaving snapshots un-restorable unless the user knew to set AGENTIC_GIT_BYPASS=1. The recovery feature was effectively broken for a docs-following user. Fix: a `checkout <tree-ish> -- <pathspec>` is a working-tree path restore, not a branch switch (the bound branch is unchanged), so the cross-branch guard must not deny it. Recognize the `--` pathspec form and skip the cross-branch check for checkout. +smoke test run_session_documented_restore_works_without_bypass_smoke: full real-session round-trip (dirty+untracked → reset --hard snapshot → delete untracked → `git checkout <snap> -- .` through the shim, NO bypass → both recovered). Also harden legacy_env_adoption's bypass assertion: check the deny SIGNATURE ('cross-branch'), not the loose word 'denied' (the #2158 bypass audit dumps process ancestry to stderr, which a parent command's text could false-trip). cargo test --workspace: green; clippy clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The new smoke tests + Linux CI caught a real cross-platform bug invisible on
macOS: on a runner/container/fresh machine with NO git user.name/user.email
(ubuntu-latest has none; macOS derives a default), the snapshot's
`git commit-tree` — which REQUIRES an author+committer — silently failed,
so a real session's destructive op created NO snapshot (SNAPS=0, fail-open),
i.e. the recovery layer didn't work exactly where it's supposed to. And the
agent's own `git commit` failed (TRAILER=no).
Fix: the snapshot forces a fixed GIT_AUTHOR/COMMITTER_NAME+EMAIL
('agentic-git <agentic-git@localhost>') alongside the dates it already
forces, making snapshots environment-independent. The smoke session also
sets an identity so the agent's own commits succeed on bare runners.
Verified by reproducing the failure on macOS with GIT_CONFIG_GLOBAL=/dev/null
+ empty HOME: smoke now passes (previously SNAP=NONE, as on ubuntu CI).
cargo test --workspace: green; smoke green under a no-git-identity env.
Co-Authored-By: Claude Fable 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.
Adversarial code review (fugu) of main found 3 real defects; all fixed with regression coverage + a CI publish gate + end-to-end smoke tests.
include_str!("../../../assets/hooks/…")reached workspace-root assets thatcargo packageexcludes from the tarball → verify build failed. Movedassets/hooks/into the crate;cargo package -p agentic-gitnow verifies clean.runcould launch UNGUARDED: child PATHjoin_paths(...).unwrap_or(path_env)silently fell back to the real PATH ifAGENTIC_GIT_HOMEhad a:→ shim bypassed. Now a hard EX_CONFIG refusal..agend-manageddefeated skip-when-clean: the marker made every fresh session's tree look dirty.is_cleannow treats a marker-only tree as clean.Plus: smoke tests (real
agentic-git runflow: routing→trailer→snapshot→recover→deny + #2/#3), CIcargo packagegate (both crates), release.ymlset +efix (bash -e no longer aborts idempotent publish).150 tests green, clippy clean, both crates package+verify. Adversarial re-review: fugu.
🤖 Generated with Claude Code