jercik commented

Owner

move-codex-session could hang or exit without output while releasing its writer locks, for three reasons:

Release stopped at the first failed lock helper and left the other running.
Release waited for an exit event that had already fired for a signal-killed helper, so it hung or exited 0 with no output.
A release error replaced the move's own error.

The script now tracks close from spawn and starts every release before awaiting any (Promise.allSettled), so one stuck helper cannot hold the other home's locks. Release failures are appended to a failed move's error.

When the move completed but a release failed, the script exits 1, prints the normal JSON with a new always-present writerLocksReleased: false, and names the home and reason on stderr. On success the field is true. SKILL.md and usage() document it. Failed moves print nothing on stdout, and every failed-move test now asserts that.

Rejected shapes:

Exit 1 with no JSON: a caller would think the move failed and rerun it against a removed source.
An optional error-string field: a missing or misspelled field reads as success, and a boolean keeps the success shape uniform.
Exit code and stderr alone: a caller cannot tell "moved" from "not moved" without parsing prose.

The 7 lock tests passed in 12 sequential runs. Reverting to the serial release hangs and then fails the new blocked-release test. Success-looking JSON on a failed move fails the stdout assertions. The old late-exit listener reproduces both the hang and the silent exit 0. In a Linux container, exit fired before stderr was read in 28 of 2000 concurrent runs, hence close.

Left for a follow-up PR: a helper that prints something other than ready is never stopped, opposite-direction moves can deadlock, the release-time reacquire has no timeout, and a helper killed mid-move is noticed only at release.

🤖 Generated with Claude Code

`move-codex-session` could hang or exit without output while releasing its writer locks, for three reasons: 1. Release stopped at the first failed lock helper and left the other running. 2. Release waited for an `exit` event that had already fired for a signal-killed helper, so it hung or exited 0 with no output. 3. A release error replaced the move's own error. The script now tracks `close` from spawn and starts every release before awaiting any (`Promise.allSettled`), so one stuck helper cannot hold the other home's locks. Release failures are appended to a failed move's error. When the move completed but a release failed, the script exits 1, prints the normal JSON with a new always-present `writerLocksReleased: false`, and names the home and reason on stderr. On success the field is `true`. `SKILL.md` and `usage()` document it. Failed moves print nothing on stdout, and every failed-move test now asserts that. Rejected shapes: - Exit 1 with no JSON: a caller would think the move failed and rerun it against a removed source. - An optional error-string field: a missing or misspelled field reads as success, and a boolean keeps the success shape uniform. - Exit code and stderr alone: a caller cannot tell "moved" from "not moved" without parsing prose. The 7 lock tests passed in 12 sequential runs. Reverting to the serial release hangs and then fails the new blocked-release test. Success-looking JSON on a failed move fails the stdout assertions. The old late-`exit` listener reproduces both the hang and the silent exit 0. In a Linux container, `exit` fired before stderr was read in 28 of 2000 concurrent runs, hence `close`. Left for a follow-up PR: a helper that prints something other than `ready` is never stopped, opposite-direction moves can deadlock, the release-time reacquire has no timeout, and a helper killed mid-move is noticed only at release. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jercik added 2 commits

2026-10-10 07:14:32 +00:00

fix: move-codex-session should not hang releasing its locks 8a0206fd89

Releasing the writer locks could leave the script running or let it
exit 0 without a result:

- Release stopped at the first failing lock helper, so the other
  helper was never told to stop and kept the script alive.
- Release waited for an `exit` event registered after a killed helper
  had already emitted it, so it never settled (hang) or the event loop
  drained (silent exit 0).
- A release error replaced the error of a move that had failed.

Each helper now reports on `close`, release attempts every helper, and
the failures name the home they belong to. A failed move keeps its own
error with any release failure appended.

A move that completed but then failed to release its locks now exits 1
with a stderr diagnostic and still prints the result JSON, with
`writerLocksReleased: false` beside `sourceRemoved: true`, so a caller
does not mistake it for an unmoved session. SKILL.md documents it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

fix: address review of the move-codex-session lock-release fix

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

Details

Node tests / node:test (pull_request) Successful in 3m50s

Details

Review / Review (pull_request_target) Successful in 21m4s

Details

cf6fb309c3

- Release every writer lock helper at once instead of one after the
  other. A destination helper that cannot finish releasing no longer
  keeps the source helper's locks, or the script, waiting.
- Reject a failed lock acquisition from `close` rather than `exit`, so
  the helper's stderr has been read when the error is built. Name a
  helper killed by a signal as such there too.
- Say in `--help` that the script can exit 1 after printing the JSON.
- Rewrite the `SKILL.md` paragraph for what the code can produce: no
  helper outlives the script and its locks are no longer held, and a
  helper may have died during the move. Drop the `lsof` step.
- Assert empty stdout in every test of a failed move, so a script that
  printed success-looking JSON there fails the suite.
- Add a test where the destination release is blocked while the source
  locks must still be released.
- Skip the disturbance in `moveWithWriterLocksDisturbed` when the
  script already ended, so its own result surfaces.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

forgejo-actions commented

2026-10-10 07:35:35 +00:00

Review 01M4JAKS2E159R10QQBDREFFSF — head cf6fb309c3399376822b1b54b3d9cf1e082caedc

Review — j4k-oss/agent-skills @ `e33d8035a4`

Scope: diff against base tree 8a58ec874c49
Status: dispatched — coverage complete (5/5 slots terminal)
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-v4",
  "tally": "tally-v1",
  "triage_settle": "triage-settle-v2"
}

Findings (0)

No findings survived.

Reviewed:

general-bug (whole/default): no-claims
writing-quality (whole/default): claims-emitted
test-trimming (whole/default): no-claims
restated-sets (whole/default): claims-emitted
project-docs (whole/default): no-claims

Other claims

grounding-pending (0)
ungrounded (0)
rejected (2)
- 01M4JAXHAJGHXE871Q619VQ8HP high — LOCK_HOLDER comment puts lock paths first and omits the leading count
- 01M4JAY9H1XWZ3M1ZPRZ51WX7P medium — Lock-release failure report lists leftover .lock files without saying the locks are no longer held
duplicate-of (0)
unadjudicated (0)

Coverage

Coverage pass: 01M4JAM287DRGEBXMH2DNNHN3D
Accounting: complete
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	claims-emitted	1	no
project-docs	whole	default	no-claims	1	no

**Review** `01M4JAKS2E159R10QQBDREFFSF` — head `cf6fb309c3399376822b1b54b3d9cf1e082caedc` # Review — j4k-oss/agent-skills @ e33d8035a418 Scope: diff against base tree `8a58ec874c49` Status: dispatched — coverage complete (5/5 slots terminal) 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-v4", "tally": "tally-v1", "triage_settle": "triage-settle-v2" } ``` ## Findings (0) No findings survived. Reviewed: - general-bug (whole/default): no-claims - writing-quality (whole/default): claims-emitted - test-trimming (whole/default): no-claims - restated-sets (whole/default): claims-emitted - project-docs (whole/default): no-claims ## Other claims - grounding-pending (0) - ungrounded (0) - rejected (2) - `01M4JAXHAJGHXE871Q619VQ8HP` high — LOCK_HOLDER comment puts lock paths first and omits the leading count - `01M4JAY9H1XWZ3M1ZPRZ51WX7P` medium — Lock-release failure report lists leftover `.lock` files without saying the locks are no longer held - duplicate-of (0) - unadjudicated (0) ## Coverage Coverage pass: 01M4JAM287DRGEBXMH2DNNHN3D Accounting: complete 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 | claims-emitted | 1 | no | | project-docs | whole | default | no-claims | 1 | no |

🚀 1

jercik merged commit 6df4d7ab3e into main

2026-10-10 07:36:30 +00:00

jercik deleted branch fix/move-codex-session-lock-release-hang

2026-10-10 07:36:30 +00:00

jercik referenced this pull request from a commit

2026-10-10 07:36:32 +00:00

fix: `move-codex-session` should not hang releasing its locks (#145)

jercik referenced this pull request

2026-10-10 08:25:52 +00:00

fix: move-codex-session should wait out transient locks on its read-only connections #148