chore(recovery): guard the recover workflow's git add --all -- changelog.d (AGENTS.md section 5a) instead of relying on the verb to create the directory #1537

Open
opened 2026-10-03 14:53:20 +02:00 by claude · 0 comments
Member

The recovery workflow stages changelog.d by name under set -euo pipefail, without the existence-or-tracked guard of AGENTS.md section 5a. Today it cannot fail, but only because the Go verb creates the directory as a side effect, and nothing in the workflow says it depends on that. This is defence in depth, not a live failure.

Measured

At origin/main 8086700:

.forgejo/workflows/reusable-recover-pending-cut.yml:266   set -euo pipefail          (step "recover the interrupted cut and open a PR")
.forgejo/workflows/reusable-recover-pending-cut.yml:378   git add -u
.forgejo/workflows/reusable-recover-pending-cut.yml:379   git add --all -- changelog.d   <- no guard
.gitea/workflows/reusable-recover-pending-cut.yml:392     git add --all -- changelog.d   <- generated twin, same line

The line came in with dfe9a85 (2026-09-05, PR #1182 for #1166).

What git does with that pathspec (git 2.55.0.windows.3, scratch repo):

State of changelog.d git add --all -- changelog.d
absent and untracked fatal: pathspec 'changelog.d' did not match any files, rc=128
exists, empty rc=0, stages nothing
holds only .keep rc=0, stages .keep
tracked, then deleted from the tree rc=0, stages the deletion

Why it is green today. The guarantee comes from the verb, not the workflow:

  • On main, rt recover-pending-cut refuses a prepare that deleted no fragment (cmd/rt/recover_pending_cut.go:399-400). A successful run then writes at least one file into changelog.d, after os.MkdirAll in applyRecovery (:474 onward). So the directory always exists when :379 runs.
  • PR #1531 (open, head 6979efc) adds a successful path that restores zero fragments (release_notes.source: commits). Its applyRecovery keeps the MkdirAll and adds a comment saying the directory is created "even when nothing is restored", because the workflow's pathspec "is fatal when the directory does not exist". Its test recoverCommitOnlyPrepare replays the two git add lines from a copy in Go.

So the workflow's staging line depends on a side effect of another layer. That dependency is stated in a Go comment and pinned by a Go test that copies the YAML line. If the side effect ever goes, :379 kills the step after the fold has been written in the runner's checkout and before the commit and push. The run goes red, nothing is pushed, and the error names a pathspec rather than the recovery.

A trap for the fix. AGENTS.md section 5a spells its guard git add -A -- "$FILE". The bats arm workflows: recovery never commits the toolkit checkout (#1135) (tests/workflows.bats, around :4953) refuses any code line matching ^\s*git add -A\b. Copying section 5a verbatim would redden that arm. The long form --all at :379 passes it.

Not covered by #1530. That consolidation lists recover-pending-cut among the workflows that stay unchanged.

Why it matters

Recovery (B) is the documented remedy for a stranded cut (#1135, #1166). When it fails, it should fail about the recovery, not about a pathspec. Section 5a exists because this exact git add shape has bitten before (#77). A guard in the workflow removes the cross-layer coupling instead of documenting it.

Acceptance criteria

  • :379 is guarded for a directory, in section 5a's shape: stage only when [[ -d changelog.d ]] or git ls-files --error-unmatch -- changelog.d succeeds. The long form --all is kept, or the #1135 arm's regex is narrowed in the same PR.
  • A bats arm runs the shipped staging lines, extracted from the run block as _recover_run and #1160's arm do, under set -euo pipefail in a repo with no changelog.d. It asserts rc 0 and the tracked CHANGELOG.md change staged. Mutation check: removing the guard reddens it (AGENTS.md section 8, Axis B).
  • The comment above the add (:369-377) names the guard. If #1531 has merged by then, the applyRecovery comment stops calling the workflow's pathspec fatal, or points at the guard.
  • The .gitea twin is regenerated, and rt gitea-twin --check passes.
  • The new lines are ASCII-only (AGENTS.md section 11).

Out of scope

  • Removing the verb's MkdirAll. It is harmless and keeps the manual recipe working.
  • The manual recipe at docs/integration.md:985 (git add -u && git add -A -- changelog.d). An operator runs it by hand and sees the error.
  • Making changelog.d configurable. Both the verb and the workflow hard-code it.

Anchor

Found 2026-10-03 while reviewing PR #1531 / issue #1529. It was listed in the PR's "Not in this PR" follow-ups as the optional section 5a guard. The git behaviour above was probed the same day.

The recovery workflow stages `changelog.d` by name under `set -euo pipefail`, without the existence-or-tracked guard of AGENTS.md section 5a. Today it cannot fail, but only because the Go verb creates the directory as a side effect, and nothing in the workflow says it depends on that. This is defence in depth, not a live failure. ## Measured At origin/main `8086700`: ``` .forgejo/workflows/reusable-recover-pending-cut.yml:266 set -euo pipefail (step "recover the interrupted cut and open a PR") .forgejo/workflows/reusable-recover-pending-cut.yml:378 git add -u .forgejo/workflows/reusable-recover-pending-cut.yml:379 git add --all -- changelog.d <- no guard .gitea/workflows/reusable-recover-pending-cut.yml:392 git add --all -- changelog.d <- generated twin, same line ``` The line came in with dfe9a85 (2026-09-05, PR #1182 for #1166). **What git does with that pathspec** (git 2.55.0.windows.3, scratch repo): | State of `changelog.d` | `git add --all -- changelog.d` | |---|---| | absent and untracked | `fatal: pathspec 'changelog.d' did not match any files`, rc=128 | | exists, empty | rc=0, stages nothing | | holds only `.keep` | rc=0, stages `.keep` | | tracked, then deleted from the tree | rc=0, stages the deletion | **Why it is green today.** The guarantee comes from the verb, not the workflow: - On main, `rt recover-pending-cut` refuses a prepare that deleted no fragment (`cmd/rt/recover_pending_cut.go:399-400`). A successful run then writes at least one file into `changelog.d`, after `os.MkdirAll` in `applyRecovery` (:474 onward). So the directory always exists when :379 runs. - PR #1531 (open, head `6979efc`) adds a successful path that restores **zero** fragments (`release_notes.source: commits`). Its `applyRecovery` keeps the `MkdirAll` and adds a comment saying the directory is created "even when nothing is restored", because the workflow's pathspec "is fatal when the directory does not exist". Its test `recoverCommitOnlyPrepare` replays the two `git add` lines from a **copy** in Go. So the workflow's staging line depends on a side effect of another layer. That dependency is stated in a Go comment and pinned by a Go test that copies the YAML line. If the side effect ever goes, :379 kills the step after the fold has been written in the runner's checkout and before the commit and push. The run goes red, nothing is pushed, and the error names a pathspec rather than the recovery. **A trap for the fix.** AGENTS.md section 5a spells its guard `git add -A -- "$FILE"`. The bats arm `workflows: recovery never commits the toolkit checkout (#1135)` (`tests/workflows.bats`, around :4953) refuses any code line matching `^\s*git add -A\b`. Copying section 5a verbatim would redden that arm. The long form `--all` at :379 passes it. **Not covered by #1530.** That consolidation lists `recover-pending-cut` among the workflows that stay unchanged. ## Why it matters Recovery (B) is the documented remedy for a stranded cut (#1135, #1166). When it fails, it should fail about the recovery, not about a pathspec. Section 5a exists because this exact `git add` shape has bitten before (#77). A guard in the workflow removes the cross-layer coupling instead of documenting it. ## Acceptance criteria - [ ] :379 is guarded for a directory, in section 5a's shape: stage only when `[[ -d changelog.d ]]` or `git ls-files --error-unmatch -- changelog.d` succeeds. The long form `--all` is kept, or the #1135 arm's regex is narrowed in the same PR. - [ ] A bats arm runs the shipped staging lines, extracted from the run block as `_recover_run` and #1160's arm do, under `set -euo pipefail` in a repo with no `changelog.d`. It asserts rc 0 and the tracked `CHANGELOG.md` change staged. Mutation check: removing the guard reddens it (AGENTS.md section 8, Axis B). - [ ] The comment above the add (:369-377) names the guard. If #1531 has merged by then, the `applyRecovery` comment stops calling the workflow's pathspec fatal, or points at the guard. - [ ] The `.gitea` twin is regenerated, and `rt gitea-twin --check` passes. - [ ] The new lines are ASCII-only (AGENTS.md section 11). ## Out of scope - Removing the verb's `MkdirAll`. It is harmless and keeps the manual recipe working. - The manual recipe at `docs/integration.md:985` (`git add -u && git add -A -- changelog.d`). An operator runs it by hand and sees the error. - Making `changelog.d` configurable. Both the verb and the workflow hard-code it. ## Anchor Found 2026-10-03 while reviewing PR #1531 / issue #1529. It was listed in the PR's "Not in this PR" follow-ups as the optional section 5a guard. The git behaviour above was probed the same day.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
frankenbit/release-toolkit#1537
No description provided.