From 14b4a50f800f1654a85a46c90773660b9c53c053 Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Tue, 12 May 2026 00:31:58 +0200 Subject: [PATCH] fix(agent-flow): auto-allow staged deletions + surface gh pr create failures Two ergonomic fixes to remove recurring friction in the agent commit + finish flow. 1. pre-commit auto-claim: the existing GUARDEX_AUTOCLAIM_STAGED_LOCKS block iterated --diff-filter=ACMRDTUXB and claimed every staged path without --allow-delete, so 'locks validate --staged' then rejected the same deletion the operator just staged, forcing a manual 'gx locks allow-delete' + re-commit loop. Split non-deletion staged paths from deletions and claim deletions with --allow-delete, gated by GUARDEX_AUTOCLAIM_STAGED_DELETES (default 1). 2. agent-branch-finish run_pr_flow: 'gh pr create' was wrapped in '>/dev/null 2>&1 || true', so auth, branch-protection, or gh-version failures were invisible. The function then ran 'gh pr view' (empty URL) and 'gh pr merge' (silent no-op), leaving operators with a 'merged via pr flow' log and no PR. Capture stderr, let the idempotent 'PR already exists' path through, surface every other failure verbatim, fail fast if no URL appears, and log the URL on success. Both changes are additive and behind opt-out env vars where behavior diverges; existing operators see strictly fewer reject+retry loops and louder PR-create failures. --- .../.openspec.yaml | 2 ++ .../notes.md | 22 ++++++++++++++++ templates/githooks/pre-commit | 23 ++++++++++++++++- templates/scripts/agent-branch-finish.sh | 25 +++++++++++++++++-- 4 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 openspec/changes/agent-claude-smoother-finish-autoclaim-deletes-and-lo-2026-05-12-00-28/.openspec.yaml create mode 100644 openspec/changes/agent-claude-smoother-finish-autoclaim-deletes-and-lo-2026-05-12-00-28/notes.md diff --git a/openspec/changes/agent-claude-smoother-finish-autoclaim-deletes-and-lo-2026-05-12-00-28/.openspec.yaml b/openspec/changes/agent-claude-smoother-finish-autoclaim-deletes-and-lo-2026-05-12-00-28/.openspec.yaml new file mode 100644 index 00000000..81cd71fe --- /dev/null +++ b/openspec/changes/agent-claude-smoother-finish-autoclaim-deletes-and-lo-2026-05-12-00-28/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-05-11 diff --git a/openspec/changes/agent-claude-smoother-finish-autoclaim-deletes-and-lo-2026-05-12-00-28/notes.md b/openspec/changes/agent-claude-smoother-finish-autoclaim-deletes-and-lo-2026-05-12-00-28/notes.md new file mode 100644 index 00000000..1655cf8a --- /dev/null +++ b/openspec/changes/agent-claude-smoother-finish-autoclaim-deletes-and-lo-2026-05-12-00-28/notes.md @@ -0,0 +1,22 @@ +# agent-claude-smoother-finish-autoclaim-deletes-and-lo-2026-05-12-00-28 (minimal / T1) + +Branch: `agent/claude/smoother-finish-autoclaim-deletes-and-lo-2026-05-12-00-28` against `main`. + +Two surgical ergonomic fixes to remove recurring friction in the agent commit + finish flow: + +1. `templates/githooks/pre-commit`: the existing `GUARDEX_AUTOCLAIM_STAGED_LOCKS` block iterates `--diff-filter=ACMRDTUXB` and calls `locks claim --branch ... ` without `--allow-delete`. The next step (`locks validate --staged`) then rejects the deletion because its `allow_delete` flag is False, forcing the operator to run `gx locks allow-delete` and re-commit. The fix splits non-deletion paths from deletions, claims deletions with `--allow-delete`, and gates the new behavior behind `GUARDEX_AUTOCLAIM_STAGED_DELETES` (default `1`, opt-out only). + +2. `templates/scripts/agent-branch-finish.sh` `run_pr_flow`: `gh pr create` was wrapped in `>/dev/null 2>&1 || true`, so when it fails (auth, branch protection, gh version skew), the failure was invisible. The function then proceeded to `gh pr view` (empty URL) and `gh pr merge` (silent no-op), leaving operators with a "merged via pr flow" log line and no PR. The fix captures stderr, allows the idempotent "PR already exists" path through silently, surfaces every other failure verbatim, and fails the function fast when `pr_view` returns no URL. + +Both changes are additive and behind opt-out env vars where behavior diverges; existing operators see strictly fewer reject + retry loops and louder PR-create failures. + +## Handoff + +- Handoff: change=`agent-claude-smoother-finish-autoclaim-deletes-and-lo-2026-05-12-00-28`; branch=`agent//`; scope=`TODO`; action=`continue this sandbox or finish cleanup after a usage-limit/manual takeover`. +- Copy prompt: Continue `agent-claude-smoother-finish-autoclaim-deletes-and-lo-2026-05-12-00-28` on branch `agent//`. Work inside the existing sandbox, review `openspec/changes/agent-claude-smoother-finish-autoclaim-deletes-and-lo-2026-05-12-00-28/notes.md`, continue from the current state instead of creating a new sandbox, and when the work is done run `gx branch finish --branch agent// --base dev --via-pr --wait-for-merge --cleanup`. + +## Cleanup + +- [ ] Run: `gx branch finish --branch agent// --base dev --via-pr --wait-for-merge --cleanup` +- [ ] Record PR URL + `MERGED` state in the completion handoff. +- [ ] Confirm sandbox worktree is gone (`git worktree list`, `git branch -a`). diff --git a/templates/githooks/pre-commit b/templates/githooks/pre-commit index b34c9197..4c6e3c3f 100755 --- a/templates/githooks/pre-commit +++ b/templates/githooks/pre-commit @@ -207,11 +207,32 @@ fi if [[ "$branch" == agent/* ]]; then if [[ "${GUARDEX_AUTOCLAIM_STAGED_LOCKS:-1}" == "1" ]]; then + # Auto-claim non-deletion staged paths. Deletions need an explicit + # `--allow-delete` flag below so `locks validate --staged` doesn't + # reject the commit on the same trip the user staged the delete. while IFS= read -r staged_file; do [[ -z "$staged_file" ]] && continue [[ "$staged_file" == ".omx/state/agent-file-locks.json" ]] && continue run_guardex_cli locks claim --branch "$branch" "$staged_file" >/dev/null 2>&1 || true - done < <(git diff --cached --name-only --diff-filter=ACMRDTUXB) + done < <(git diff --cached --name-only --diff-filter=ACMRTUXB) + + # Auto-approve deletions for the same branch (gated separately so + # operators can disable this single behavior without disabling the + # broader auto-claim). Defaults to enabled — matches the auto-claim + # default and removes the "first commit fails, then `gx locks + # allow-delete`, then commit again" loop. + if [[ "${GUARDEX_AUTOCLAIM_STAGED_DELETES:-1}" == "1" ]]; then + _staged_deletes=() + while IFS= read -r staged_delete; do + [[ -z "$staged_delete" ]] && continue + [[ "$staged_delete" == ".omx/state/agent-file-locks.json" ]] && continue + _staged_deletes+=("$staged_delete") + done < <(git diff --cached --name-only --diff-filter=D) + if (( ${#_staged_deletes[@]} > 0 )); then + run_guardex_cli locks claim --branch "$branch" --allow-delete \ + "${_staged_deletes[@]}" >/dev/null 2>&1 || true + fi + fi fi if ! run_guardex_cli locks validate --branch "$branch" --staged; then diff --git a/templates/scripts/agent-branch-finish.sh b/templates/scripts/agent-branch-finish.sh index 39370700..0af6475e 100755 --- a/templates/scripts/agent-branch-finish.sh +++ b/templates/scripts/agent-branch-finish.sh @@ -1126,14 +1126,35 @@ run_pr_flow() { fi pr_body="Automated by gx branch finish (PR flow)." - "$GH_BIN" pr create \ + pr_create_output="" + if pr_create_output="$("$GH_BIN" pr create \ --base "$BASE_BRANCH" \ --head "$SOURCE_BRANCH" \ --title "$pr_title" \ - --body "$pr_body" >/dev/null 2>&1 || true + --body "$pr_body" 2>&1)"; then + : + else + # Idempotent: a PR already opened for this head is fine — fall through + # to `gh pr view` so we still capture the URL. Anything else is a real + # failure and the user needs to see it. + if ! grep -qiE 'already exists|a pull request for branch' <<<"$pr_create_output"; then + echo "[agent-branch-finish] gh pr create failed:" >&2 + echo "${pr_create_output}" >&2 + fi + fi pr_url="$("$GH_BIN" pr view "$SOURCE_BRANCH" --json url --jq '.url' 2>/dev/null || true)" + if [[ -z "$pr_url" ]]; then + echo "[agent-branch-finish] No PR found for '${SOURCE_BRANCH}' after gh pr create; cannot proceed with PR merge." >&2 + if [[ -n "$pr_create_output" ]]; then + echo "[agent-branch-finish] Last gh pr create output:" >&2 + echo "${pr_create_output}" >&2 + fi + return 1 + fi + echo "[agent-branch-finish] PR URL: ${pr_url}" >&2 + merge_output="" if merge_output="$("$GH_BIN" pr merge "$SOURCE_BRANCH" --squash --delete-branch 2>&1)"; then return 0