chore: sync align local command templates #7

jercik · 2026-10-24T06:57:02Z

jercik commented

2026-10-24 06:57:02 +00:00

Regenerates the repository's managed files with Align 13.0.0 so declaration-check recovery instructions use the current local repair command. Includes the staged-snapshot cleanup update and review wrapper update with the existing zero-failure requirement retained.

jercik added 1 commit

2026-10-24 06:57:03 +00:00

chore: sync align local command templates

Checks / pr-flow-canary (pull_request) Has been skipped

Details

Checks / reuse (pull_request) Successful in 5s

Details

commit-msg / commitlint (pull_request) Successful in 10s

Details

Checks / quality-checks (26.5.0) (pull_request) Successful in 46s

Details

Checks / quality-checks (24.15.0) (pull_request) Successful in 46s

Details

Review / Review (pull_request_target) Successful in 6m12s

Details

4cf8475514

forgejo-actions commented

2026-10-24 07:43:55 +00:00

Review 01M5PDGEDE5CD91721E8DVPCX3 — head 4cf847551453ad737a9b0761c1a4e57a2d14259c

Review — j4k-oss/perplexity-agent-mcp @ `eaf7864b77`

Scope: diff against base tree 5368bdadc54d
Review finished: 10 succeeded, 0 failed
Triage: settled
Facts: current review-wide projection

Computed under:

{
  "abandonment": "abandonment-v1",
  "anchor_recipe": 1,
  "batch_policy": "batch-v1",
  "coverage": "coverage-v3",
  "dispatch_policy": "dispatch-v2",
  "grounder_version": 1,
  "grounding_read_rule": "grounding-read-v1",
  "promotion_policy": "promotion-v1",
  "report": "report-v5",
  "tally": "tally-v1",
  "triage_settle": "triage-settle-v2"
}

Findings (1)

medium — The watcher treats fd-6 EOF as "the hook is gone", so it deletes the snapshot while an orphaned `git worktree add` (started with `6>&-`) is still writing into it

claim: 01M5PDN1QT42FP6V491HHZWKT8
anchor: .githooks/commit-msg (snippet)

git --git-dir="$git_common_dir" -c worktree.useRelativePaths=false -c core.hooksPath=/dev/null worktree add --quiet --detach "$snapshot_dir" "$snapshot_commit" 6>&-

lens: aposd-plain · arm: default
verdicts: 1 valid / 0 invalid / 0 uncertain
- pass 01M5PDPZARC71HRAS7T36V2XEW · valid: The grounded anchor shows git worktree add launched with 6>&- and no process-group tracking. The reviewer quotes the comment above exec 6> that pushes onto every later command the duty to close fd 6 on any command that can leave a process behind. They also show that only run_step pairs that closure with tell_watcher. So the watcher's liveness rule (the hook counts as dead once every fd-6 holder exits) is hidden knowledge each call site must re-derive. Getting it wrong fails silently either way: the watcher hangs if a daemon keeps fd 6, or cleans up underneath an untracked orphan. This is the Information Leakage / Nonobvious Code red flag, plus the 'pull complexity downward' principle, with a concrete cost at the anchored line. The unix-cli claim's reproduction independently confirms the orphan survives the cleanup. The proposed remedies (a single spawn helper, or tracking the hook PID) fit the defect. One overstatement: the inference that Git recreates the directory and leaves a partly populated snapshot is unverified. The unix-cli reproduction instead shows the resumed checkout failing with 'unable to create file ... No such file or directory', so the leftover state is the surviving orphan process and possibly a stale worktree registration, not necessarily recreated files. Low to medium severity fits.
duplicates: 01M5PDP5Q5SCB8TV0FWMH5B15E (unix-cli)
disposition: none

The watcher exists for one case: the hook process is SIGKILLed by itself and never runs its EXIT trap. In that case, if the hook dies during the snapshot checkout, git worktree add keeps running as an orphan. It was started with 6>&-, so it does not hold the pipe open. It is not in its own process group, and the watcher was never sent its PID. The watcher therefore reads EOF immediately and runs remove_snapshot_files. That runs worktree remove --force --force and rm -rf "$snapshot_dir" while git is still checking files out into that directory. Git recreates leading directories as it writes, so a partly populated pre-commit-snapshot.* directory and possibly a stale worktree registration can outlive the cleanup that was supposed to remove them.

The root cause is the design: a decision about how processes are handled has leaked into every call site. watch_snapshot decides the hook is dead once every holder of fd 6 has exited. The comment above exec 6> passes that rule to every later command: "close it (6>&-) on any command that can leave a process behind". Each command then has to choose between two silent failure modes. If it keeps fd 6 and leaves a daemon behind (a smudge filter, fsmonitor), the watcher never sees EOF, so it never cleans up and never exits. If it closes fd 6 and is not reported through tell_watcher, as with worktree add, the watcher cleans up underneath it. Only run_step closes fd 6 and also reports a process group for the watcher to kill. The worktree add line does the first half and skips the second. Every command added after exec 6> in the future needs the same reasoning, and nothing fails loudly when it is missed.

A deeper module would put this in one place. Start every child that might outlive the hook through a single helper that closes fd 6, puts the child in its own process group, and calls tell_watcher with that group. In other words, generalise run_step's spawn half and use it for worktree add as well, so the watcher kills the orphan before it removes anything. A second option is for the watcher to track the hook's own PID instead of pipe EOF, so a child's file descriptors no longer decide whether the hook counts as alive. Either way, the knowledge stays in the watcher instead of in each command's redirections.

I read run_step, tell_watcher, watch_snapshot, remove_snapshot_files, and the exec 6> >(set -m; watch_snapshot <&0 > /dev/null &) setup in .githooks/commit-msg. I also ran a reduced probe: the same exec 6> >(set -m; watch <&0 > /dev/null &) pattern, then sleep 3 6>&-, then kill -KILL on the hook PID alone 0.5 s in. The watcher's cleanup line was logged at the same instant as the kill (1792827596.13457 vs 1792827596.13456), while sleep still had about 2.5 s to run. I did not run a real git worktree add under SIGKILL, so the exact leftover state (files only, or files plus a .git/worktrees/<name> entry) is inferred from git creating directories as it checks out. Running the hook against a large repository and SIGKILLing only the hook PID during worktree add would settle this. A smaller window has the same cause: between local step_pid=$! and tell_watcher "$step_pid", a SIGKILL leaves a check running that the watcher does not know about.

Other claims

grounding-pending (0)
ungrounded (0)
rejected (0)
duplicate-of (1)
- 01M5PDP5Q5SCB8TV0FWMH5B15E medium — The snapshot watcher leaves checkout processes running after the hook is killed → 01M5PDN1QT42FP6V491HHZWKT8
unadjudicated (1)
- 01M5PDK6M07DZ8ZNK499SBDGEW medium — The recursion guard duplicates the wrapper's write-operation inventory

Coverage

Coverage pass: 01M5PDGEPW8RY10AN9J5HDYVYJ
Accounting: complete (all slots terminal; see slot results for failures)
Slot health: healthy

lens	part	arm	unit status	runs	loss
general-bug	whole	default	no-claims	1	no
writing-quality	whole	default	claims-emitted	1	no
test-trimming	whole	default	no-claims	1	no
restated-sets	whole	default	no-claims	1	no
project-docs	whole	default	no-claims	1	no
unix-cli	whole	default	claims-emitted	1	no
use-effect	whole	default	no-claims	1	no
module-design	whole	default	no-claims	1	no
aposd-plain	whole	default	claims-emitted	1	no
react	whole	default	no-claims	1	no

**Review** `01M5PDGEDE5CD91721E8DVPCX3` — head `4cf847551453ad737a9b0761c1a4e57a2d14259c` # Review — j4k-oss/perplexity-agent-mcp @ eaf7864b7777 Scope: diff against base tree `5368bdadc54d` Review finished: 10 succeeded, 0 failed Triage: settled Facts: current review-wide projection Computed under: ```json { "abandonment": "abandonment-v1", "anchor_recipe": 1, "batch_policy": "batch-v1", "coverage": "coverage-v3", "dispatch_policy": "dispatch-v2", "grounder_version": 1, "grounding_read_rule": "grounding-read-v1", "promotion_policy": "promotion-v1", "report": "report-v5", "tally": "tally-v1", "triage_settle": "triage-settle-v2" } ``` ## Findings (1) ### medium — The watcher treats fd-6 EOF as "the hook is gone", so it deletes the snapshot while an orphaned `git worktree add` (started with `6>&-`) is still writing into it - claim: `01M5PDN1QT42FP6V491HHZWKT8` - anchor: `.githooks/commit-msg` (snippet) ``` git --git-dir="$git_common_dir" -c worktree.useRelativePaths=false -c core.hooksPath=/dev/null worktree add --quiet --detach "$snapshot_dir" "$snapshot_commit" 6>&- ``` - lens: aposd-plain · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - pass `01M5PDPZARC71HRAS7T36V2XEW` · valid: The grounded anchor shows git worktree add launched with 6>&- and no process-group tracking. The reviewer quotes the comment above exec 6> that pushes onto every later command the duty to close fd 6 on any command that can leave a process behind. They also show that only run_step pairs that closure with tell_watcher. So the watcher's liveness rule (the hook counts as dead once every fd-6 holder exits) is hidden knowledge each call site must re-derive. Getting it wrong fails silently either way: the watcher hangs if a daemon keeps fd 6, or cleans up underneath an untracked orphan. This is the Information Leakage / Nonobvious Code red flag, plus the 'pull complexity downward' principle, with a concrete cost at the anchored line. The unix-cli claim's reproduction independently confirms the orphan survives the cleanup. The proposed remedies (a single spawn helper, or tracking the hook PID) fit the defect. One overstatement: the inference that Git recreates the directory and leaves a partly populated snapshot is unverified. The unix-cli reproduction instead shows the resumed checkout failing with 'unable to create file ... No such file or directory', so the leftover state is the surviving orphan process and possibly a stale worktree registration, not necessarily recreated files. Low to medium severity fits. - duplicates: `01M5PDP5Q5SCB8TV0FWMH5B15E` (unix-cli) - disposition: none > The watcher exists for one case: the hook process is SIGKILLed by itself and never runs its EXIT trap. In that case, if the hook dies during the snapshot checkout, `git worktree add` keeps running as an orphan. It was started with `6>&-`, so it does not hold the pipe open. It is not in its own process group, and the watcher was never sent its PID. The watcher therefore reads EOF immediately and runs `remove_snapshot_files`. That runs `worktree remove --force --force` and `rm -rf "$snapshot_dir"` while git is still checking files out into that directory. Git recreates leading directories as it writes, so a partly populated `pre-commit-snapshot.*` directory and possibly a stale worktree registration can outlive the cleanup that was supposed to remove them. > > The root cause is the design: a decision about how processes are handled has leaked into every call site. `watch_snapshot` decides the hook is dead once every holder of fd 6 has exited. The comment above `exec 6>` passes that rule to every later command: "close it (`6>&-`) on any command that can leave a process behind". Each command then has to choose between two silent failure modes. If it keeps fd 6 and leaves a daemon behind (a smudge filter, fsmonitor), the watcher never sees EOF, so it never cleans up and never exits. If it closes fd 6 and is not reported through `tell_watcher`, as with `worktree add`, the watcher cleans up underneath it. Only `run_step` closes fd 6 and also reports a process group for the watcher to kill. The `worktree add` line does the first half and skips the second. Every command added after `exec 6>` in the future needs the same reasoning, and nothing fails loudly when it is missed. > > A deeper module would put this in one place. Start every child that might outlive the hook through a single helper that closes fd 6, puts the child in its own process group, and calls `tell_watcher` with that group. In other words, generalise `run_step`'s spawn half and use it for `worktree add` as well, so the watcher kills the orphan before it removes anything. A second option is for the watcher to track the hook's own PID instead of pipe EOF, so a child's file descriptors no longer decide whether the hook counts as alive. Either way, the knowledge stays in the watcher instead of in each command's redirections. > > I read `run_step`, `tell_watcher`, `watch_snapshot`, `remove_snapshot_files`, and the `exec 6> >(set -m; watch_snapshot <&0 > /dev/null &)` setup in `.githooks/commit-msg`. I also ran a reduced probe: the same `exec 6> >(set -m; watch <&0 > /dev/null &)` pattern, then `sleep 3 6>&-`, then `kill -KILL` on the hook PID alone 0.5 s in. The watcher's cleanup line was logged at the same instant as the kill (1792827596.13457 vs 1792827596.13456), while `sleep` still had about 2.5 s to run. I did not run a real `git worktree add` under SIGKILL, so the exact leftover state (files only, or files plus a <code>.git/worktrees/<name></code> entry) is inferred from git creating directories as it checks out. Running the hook against a large repository and SIGKILLing only the hook PID during `worktree add` would settle this. A smaller window has the same cause: between `local step_pid=$!` and `tell_watcher "$step_pid"`, a SIGKILL leaves a check running that the watcher does not know about. ## Other claims - grounding-pending (0) - ungrounded (0) - rejected (0) - duplicate-of (1) - `01M5PDP5Q5SCB8TV0FWMH5B15E` medium — The snapshot watcher leaves checkout processes running after the hook is killed → `01M5PDN1QT42FP6V491HHZWKT8` - unadjudicated (1) - `01M5PDK6M07DZ8ZNK499SBDGEW` medium — The recursion guard duplicates the wrapper's write-operation inventory ## Coverage Coverage pass: 01M5PDGEPW8RY10AN9J5HDYVYJ Accounting: complete (all slots terminal; see slot results for failures) Slot health: healthy | lens | part | arm | unit status | runs | loss | | --- | --- | --- | --- | --- | --- | | general-bug | whole | default | no-claims | 1 | no | | writing-quality | whole | default | claims-emitted | 1 | no | | test-trimming | whole | default | no-claims | 1 | no | | restated-sets | whole | default | no-claims | 1 | no | | project-docs | whole | default | no-claims | 1 | no | | unix-cli | whole | default | claims-emitted | 1 | no | | use-effect | whole | default | no-claims | 1 | no | | module-design | whole | default | no-claims | 1 | no | | aposd-plain | whole | default | claims-emitted | 1 | no | | react | whole | default | no-claims | 1 | no |

🚀 1

forgejo-actions reviewed

2026-10-24 07:43:56 +00:00

.githooks/commit-msg

					
				@ -146,2 +205,3 @@

				exec 6> >(set -m; watch_snapshot <&0 > /dev/null &)

				# Relative worktree paths would upgrade the repository format; post-checkout hooks must not run here.

				git --git-dir="$git_common_dir" -c worktree.useRelativePaths=false -c core.hooksPath=/dev/null worktree add --quiet --detach "$snapshot_dir" "$snapshot_commit"

				git --git-dir="$git_common_dir" -c worktree.useRelativePaths=false -c core.hooksPath=/dev/null worktree add --quiet --detach "$snapshot_dir" "$snapshot_commit" 6>&-

medium — The watcher treats fd-6 EOF as "the hook is gone", so it deletes the snapshot while an orphaned git worktree add (started with 6>&-) is still writing into it

The watcher exists for one case: the hook process is SIGKILLed by itself and never runs its EXIT trap. In that case, if the hook dies during the snapshot checkout, git worktree add keeps running as an orphan. It was started with 6>&-, so it does not hold the pipe open. It is not in its own process group, and the watcher was never sent its PID. The watcher therefore reads EOF immediately and runs remove_snapshot_files. That runs worktree remove --force --force and rm -rf "$snapshot_dir" while git is still checking files out into that directory. Git recreates leading directories as it writes, so a partly populated pre-commit-snapshot.* directory and possibly a stale worktree registration can outlive the cleanup that was supposed to remove them.

The root cause is the design: a decision about how processes are handled has leaked into every call site. watch_snapshot decides the hook is dead once every holder of fd 6 has exited. The comment above exec 6> passes that rule to every later command: "close it (6>&-) on any command that can leave a process behind". Each command then has to choose between two silent failure modes. If it keeps fd 6 and leaves a daemon behind (a smudge filter, fsmonitor), the watcher never sees EOF, so it never cleans up and never exits. If it closes fd 6 and is not reported through tell_watcher, as with worktree add, the watcher cleans up underneath it. Only run_step closes fd 6 and also reports a process group for the watcher to kill. The worktree add line does the first half and skips the second. Every command added after exec 6> in the future needs the same reasoning, and nothing fails loudly when it is missed.

A deeper module would put this in one place. Start every child that might outlive the hook through a single helper that closes fd 6, puts the child in its own process group, and calls tell_watcher with that group. In other words, generalise run_step's spawn half and use it for worktree add as well, so the watcher kills the orphan before it removes anything. A second option is for the watcher to track the hook's own PID instead of pipe EOF, so a child's file descriptors no longer decide whether the hook counts as alive. Either way, the knowledge stays in the watcher instead of in each command's redirections.

I read run_step, tell_watcher, watch_snapshot, remove_snapshot_files, and the exec 6> >(set -m; watch_snapshot <&0 > /dev/null &) setup in .githooks/commit-msg. I also ran a reduced probe: the same exec 6> >(set -m; watch <&0 > /dev/null &) pattern, then sleep 3 6>&-, then kill -KILL on the hook PID alone 0.5 s in. The watcher's cleanup line was logged at the same instant as the kill (1792827596.13457 vs 1792827596.13456), while sleep still had about 2.5 s to run. I did not run a real git worktree add under SIGKILL, so the exact leftover state (files only, or files plus a .git/worktrees/<name> entry) is inferred from git creating directories as it checks out. Running the hook against a large repository and SIGKILLing only the hook PID during worktree add would settle this. A smaller window has the same cause: between local step_pid=$! and tell_watcher "$step_pid", a SIGKILL leaves a check running that the watcher does not know about.

lens aposd-plain · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M5PDN1QT42FP6V491HHZWKT8 of review 01M5PDGEDE5CD91721E8DVPCX3

**medium** — The watcher treats fd-6 EOF as "the hook is gone", so it deletes the snapshot while an orphaned `git worktree add` (started with `6>&-`) is still writing into it > The watcher exists for one case: the hook process is SIGKILLed by itself and never runs its EXIT trap. In that case, if the hook dies during the snapshot checkout, `git worktree add` keeps running as an orphan. It was started with `6>&-`, so it does not hold the pipe open. It is not in its own process group, and the watcher was never sent its PID. The watcher therefore reads EOF immediately and runs `remove_snapshot_files`. That runs `worktree remove --force --force` and `rm -rf "$snapshot_dir"` while git is still checking files out into that directory. Git recreates leading directories as it writes, so a partly populated `pre-commit-snapshot.*` directory and possibly a stale worktree registration can outlive the cleanup that was supposed to remove them. > > The root cause is the design: a decision about how processes are handled has leaked into every call site. `watch_snapshot` decides the hook is dead once every holder of fd 6 has exited. The comment above `exec 6>` passes that rule to every later command: "close it (`6>&-`) on any command that can leave a process behind". Each command then has to choose between two silent failure modes. If it keeps fd 6 and leaves a daemon behind (a smudge filter, fsmonitor), the watcher never sees EOF, so it never cleans up and never exits. If it closes fd 6 and is not reported through `tell_watcher`, as with `worktree add`, the watcher cleans up underneath it. Only `run_step` closes fd 6 and also reports a process group for the watcher to kill. The `worktree add` line does the first half and skips the second. Every command added after `exec 6>` in the future needs the same reasoning, and nothing fails loudly when it is missed. > > A deeper module would put this in one place. Start every child that might outlive the hook through a single helper that closes fd 6, puts the child in its own process group, and calls `tell_watcher` with that group. In other words, generalise `run_step`'s spawn half and use it for `worktree add` as well, so the watcher kills the orphan before it removes anything. A second option is for the watcher to track the hook's own PID instead of pipe EOF, so a child's file descriptors no longer decide whether the hook counts as alive. Either way, the knowledge stays in the watcher instead of in each command's redirections. > > I read `run_step`, `tell_watcher`, `watch_snapshot`, `remove_snapshot_files`, and the `exec 6> >(set -m; watch_snapshot <&0 > /dev/null &)` setup in `.githooks/commit-msg`. I also ran a reduced probe: the same `exec 6> >(set -m; watch <&0 > /dev/null &)` pattern, then `sleep 3 6>&-`, then `kill -KILL` on the hook PID alone 0.5 s in. The watcher's cleanup line was logged at the same instant as the kill (1792827596.13457 vs 1792827596.13456), while `sleep` still had about 2.5 s to run. I did not run a real `git worktree add` under SIGKILL, so the exact leftover state (files only, or files plus a <code>.git/worktrees/<name></code> entry) is inferred from git creating directories as it checks out. Running the hook against a large repository and SIGKILLing only the hook PID during `worktree add` would settle this. A smaller window has the same cause: between `local step_pid=$!` and `tell_watcher "$step_pid"`, a SIGKILL leaves a check running that the watcher does not know about. lens `aposd-plain` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M5PDN1QT42FP6V491HHZWKT8` of review `01M5PDGEDE5CD91721E8DVPCX3`

🚀 1

jercik marked this conversation as resolved

jercik commented

2026-10-24 08:31:58 +00:00

Replying to review comment #183802

Confirmed with real Git on macOS, without filters: killing only the hook during delayed snapshot checkout can leave a temporary snapshot and worktree registration. The current hook retains this documented limitation; this PR does not fix it. The producer correction and focused regression are owned by j4k/align#826, which routes checkout through the existing run_step helper. Consumers will receive the generated correction after that release.

> Replying to review comment #183802 Confirmed with real Git on macOS, without filters: killing only the hook during delayed snapshot checkout can leave a temporary snapshot and worktree registration. The current hook retains this documented limitation; this PR does not fix it. The producer correction and focused regression are owned by https://code.j4k.dev/j4k/align/pulls/826, which routes checkout through the existing run_step helper. Consumers will receive the generated correction after that release.

jercik commented

2026-10-24 08:32:27 +00:00

Replying to review comment #183801

For unadjudicated claim 01M5PDK6M07DZ8ZNK499SBDGEW, the recursion comment is a manually maintained copy, so its maintenance cost is real. Inspection of review-wrapper at the pinned 7725fcb7d0300c467c6b0224f7d442df98756481 found its mutation methods create/edit issue comments, create COMMENT reviews/add review comments, and resolve/unresolve conversations. Those operations fit the comment's categories; no current mismatch was found. We retain the three explanatory lines and decline the optional prose rewrite within this migration scope. This does not claim that duplication was disproved or that Forgejo event semantics were freshly tested.

> Replying to review comment #183801 For unadjudicated claim 01M5PDK6M07DZ8ZNK499SBDGEW, the recursion comment is a manually maintained copy, so its maintenance cost is real. Inspection of review-wrapper at the pinned 7725fcb7d0300c467c6b0224f7d442df98756481 found its mutation methods create/edit issue comments, create COMMENT reviews/add review comments, and resolve/unresolve conversations. Those operations fit the comment's categories; no current mismatch was found. We retain the three explanatory lines and decline the optional prose rewrite within this migration scope. This does not claim that duplication was disproved or that Forgejo event semantics were freshly tested.

jercik merged commit f920d91c08 into main

2026-10-24 08:33:33 +00:00

jercik deleted branch chore/align-local-forge

2026-10-24 08:33:33 +00:00

jercik referenced this pull request from a commit

2026-10-24 08:33:34 +00:00

chore: sync align local command templates (#7)

Sign in to join this conversation.

No reviewers

No labels

No milestone

No assignees

2 participants

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

j4k-oss/perplexity-agent-mcp!7

Rows
Columns

chore: sync align local command templates #7

Review — j4k-oss/perplexity-agent-mcp @ eaf7864b77

Findings (1)

medium — The watcher treats fd-6 EOF as "the hook is gone", so it deletes the snapshot while an orphaned git worktree add (started with 6>&-) is still writing into it

Other claims

Coverage

Review — j4k-oss/perplexity-agent-mcp @ `eaf7864b77`

medium — The watcher treats fd-6 EOF as "the hook is gone", so it deletes the snapshot while an orphaned `git worktree add` (started with `6>&-`) is still writing into it