j4k-oss/agent-skills

Watch

Fork 0

fix: `move-codex-session` should re-check goals, memories, queue and logs before deleting the source #138

Merged

jercik merged 1 commit from fix/move-codex-session-auxiliary-recheck into main

2026-10-09 21:03:55 +00:00

jercik commented

2026-10-09 20:56:10 +00:00

Owner

Follow-up to #132 from its review (finding).

Before deleting the moved threads from the source, the script confirmed that the destination still held their state and history rows. It never checked the goals, memories, queue and logs copies. A destination writer could delete or change those rows after the copy, and the source deletion would then discard the only remaining copy. This gap predates #132.

Source deletion now compares every destination database with the source. It then write-locks all destination databases, confirms none changed since that comparison, and commits the source deletion while holding the locks. Log row IDs and queue revisions are generated in each home, so the comparison skips them.

The new test deletes a destination goal after the comparison. The script exits 1 with destination goals table thread_goals changed during source deletion and keeps the source rows; on #132's head the same move exits 0 and deletes them. Edits to logs, memory jobs, stage-one outputs, queued items, queue revisions and goal deferrals stop the deletion the same way. A queue revision bump and an unrelated log row do not.

The destination locks now cover six databases instead of two. In a fixture with logs at the partition limit, the hold took 39 ms. If another process keeps a destination database locked past the 30-second busy timeout, the move fails at this step and keeps the source.

Follow-up to #132 from its review ([finding](https://code.j4k.dev/j4k-oss/agent-skills/pulls/132#issuecomment-150305)). Before deleting the moved threads from the source, the script confirmed that the destination still held their state and history rows. It never checked the goals, memories, queue and logs copies. A destination writer could delete or change those rows after the copy, and the source deletion would then discard the only remaining copy. This gap predates #132. Source deletion now compares every destination database with the source. It then write-locks all destination databases, confirms none changed since that comparison, and commits the source deletion while holding the locks. Log row IDs and queue revisions are generated in each home, so the comparison skips them. The new test deletes a destination goal after the comparison. The script exits 1 with `destination goals table thread_goals changed during source deletion` and keeps the source rows; on #132's head the same move exits 0 and deletes them. Edits to logs, memory jobs, stage-one outputs, queued items, queue revisions and goal deferrals stop the deletion the same way. A queue revision bump and an unrelated log row do not. The destination locks now cover six databases instead of two. In a fixture with logs at the partition limit, the hold took 39 ms. If another process keeps a destination database locked past the 30-second busy timeout, the move fails at this step and keeps the source.

jercik added 1 commit

2026-10-09 20:56:10 +00:00

fix: move-codex-session should re-check goals, memories, queue and logs before deleting the source

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

Details

Review / Review (pull_request_target) Successful in 2m20s

Details

Node tests / node:test (pull_request) Has been cancelled

Details

63d639f142

The source commit re-checked only the destination state and history rows. A
destination writer that deleted or changed a moved goal, memory, queued item or
log after the comparison let the source deletion commit, losing the row from
both homes. Every destination database is now compared under the source lock
and write-locked through the source commit.

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

jercik referenced this pull request

2026-10-09 20:56:46 +00:00

fix: move-codex-session should handle Codex 0.160.1 and large native homes #132

jercik changed target branch from fix/move-codex-session-codex-0160 to main

2026-10-09 20:58:05 +00:00

forgejo-actions commented

2026-10-09 20:58:29 +00:00

Review 01M4H77GQHV2ND5246CEBAY52C — head eeed5507e7c7de2534abb487f0afa981c4a815d0

Review — j4k-oss/agent-skills @ `7f7243ecfa`

Scope: diff against base tree 40154724b696
Status: dispatched — coverage complete (3/3 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
test-trimming (whole/default): no-claims
restated-sets (whole/default): no-claims
writing-quality: skipped-by-dispatch — no prose or user-facing documentation changed; only code and a test
project-docs: skipped-by-dispatch — no documentation files changed

Other claims

grounding-pending (0)
ungrounded (0)
rejected (0)
duplicate-of (0)
unadjudicated (0)

Coverage

Coverage pass: 01M4H77Y8JC183X1R1Q4ENHTWM
Accounting: complete
Slot health: healthy

lens	part	arm	unit status	runs	loss
general-bug	whole	default	no-claims	1	no
test-trimming	whole	default	no-claims	1	no
restated-sets	whole	default	no-claims	1	no

writing-quality — skipped-by-dispatch: no prose or user-facing documentation changed; only code and a test
project-docs — skipped-by-dispatch: no documentation files changed

**Review** `01M4H77GQHV2ND5246CEBAY52C` — head `eeed5507e7c7de2534abb487f0afa981c4a815d0` # Review — j4k-oss/agent-skills @ 7f7243ecfa45 Scope: diff against base tree `40154724b696` Status: dispatched — coverage complete (3/3 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 - test-trimming (whole/default): no-claims - restated-sets (whole/default): no-claims - writing-quality: skipped-by-dispatch — no prose or user-facing documentation changed; only code and a test - project-docs: skipped-by-dispatch — no documentation files changed ## Other claims - grounding-pending (0) - ungrounded (0) - rejected (0) - duplicate-of (0) - unadjudicated (0) ## Coverage Coverage pass: 01M4H77Y8JC183X1R1Q4ENHTWM Accounting: complete Slot health: healthy | lens | part | arm | unit status | runs | loss | | --- | --- | --- | --- | --- | --- | | general-bug | whole | default | no-claims | 1 | no | | test-trimming | whole | default | no-claims | 1 | no | | restated-sets | whole | default | no-claims | 1 | no | - writing-quality — skipped-by-dispatch: no prose or user-facing documentation changed; only code and a test - project-docs — skipped-by-dispatch: no documentation files changed

🚀 1

jercik force-pushed fix/move-codex-session-auxiliary-recheck from 63d639f142

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

Details

Review / Review (pull_request_target) Successful in 2m20s

Details

Node tests / node:test (pull_request) Has been cancelled

Details

to eeed5507e7

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

Details

Review / Review (pull_request_target) Successful in 4s