j4k-oss/agent-skills
Watch
1
0
Fork
You've already forked agent-skills
 0
Code Issues 6 Pull requests Releases Packages Activity Actions

fix: move-codex-session rollback should keep destination history rows it never copied #133

Merged
jercik merged 2 commits from fix/move-codex-session-rollback-history into main 2026-10-09 21:03:33 +00:00
Conversation 2 Commits 2 Files changed 2 +55 -1
jercik commented 2026-10-09 19:50:02 +00:00
Owner
Copy link

Follow-up to #132 from its review (finding): when the state copy commits and the history copy then fails, the rollback deleted destination history rows for every moving rollout, including rows a destination writer committed between the two copies. It now deletes history only when the history copy committed. The deletion predates #132.

Follow-up to #132 from its review ([finding](https://code.j4k.dev/j4k-oss/agent-skills/pulls/132#issuecomment-150303)): when the state copy commits and the history copy then fails, the rollback deleted destination history rows for every moving rollout, including rows a destination writer committed between the two copies. It now deletes history only when the history copy committed. The deletion predates #132.
fix: move-codex-session rollback should keep destination history rows it never copied
All checks were successful
commit-msg / commitlint (pull_request) Successful in 23s
Details
Review / Review (pull_request_target) Successful in 3m20s
Details
Node tests / node:test (pull_request) Successful in 3m55s
Details
c0cd86281d
 
When the state copy committed and the history copy then failed, the
destination rollback still deleted history rows for every moving rollout,
including rows a destination writer committed between the two copies.
Delete destination history only when the history copy committed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
forgejo-actions commented 2026-10-09 19:53:22 +00:00
Copy link

Review 01M4H763PQSV82W9GT592YVAF8 — head 7d4d9b4c10808e6a110b48942c23016d564ad088

Review — j4k-oss/agent-skills @ b95311f880

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
  • writing-quality (whole/default): no-claims
  • restated-sets: skipped-by-dispatch — no restated sets or duplicated enumerations in the change
  • project-docs: skipped-by-dispatch — no documentation changed

Other claims

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

Coverage

Coverage pass: 01M4H76H88VKZ50VD2S21FANP1
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
writing-quality whole default no-claims 1 no
  • restated-sets — skipped-by-dispatch: no restated sets or duplicated enumerations in the change
  • project-docs — skipped-by-dispatch: no documentation changed
<!-- review:summary --> **Review** `01M4H763PQSV82W9GT592YVAF8` — head `7d4d9b4c10808e6a110b48942c23016d564ad088` # Review — j4k-oss/agent-skills @ b95311f8800f 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 - writing-quality (whole/default): no-claims - restated-sets: skipped-by-dispatch — no restated sets or duplicated enumerations in the change - project-docs: skipped-by-dispatch — no documentation changed ## Other claims - grounding-pending (0) - ungrounded (0) - rejected (0) - duplicate-of (0) - unadjudicated (0) ## Coverage Coverage pass: 01M4H76H88VKZ50VD2S21FANP1 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 | | writing-quality | whole | default | no-claims | 1 | no | - restated-sets — skipped-by-dispatch: no restated sets or duplicated enumerations in the change - project-docs — skipped-by-dispatch: no documentation changed
🚀 1
jercik commented 2026-10-09 20:01:33 +00:00
Author
Owner
Copy link

Replying to review comment #150348

The summary's one finding (claim 01M4H3J3X9RYG7TT2S4ZXF5HTH, the precision test's trigger firing for both threads) is valid but sits in a test this PR does not change. It is the same defect #132's review reported in #132 (comment). Tracked in #136.

> Replying to review comment #150348 The summary's one finding (claim `01M4H3J3X9RYG7TT2S4ZXF5HTH`, the precision test's trigger firing for both threads) is valid but sits in a test this PR does not change. It is the same defect #132's review reported in https://code.j4k.dev/j4k-oss/agent-skills/pulls/132#issuecomment-150308. Tracked in https://code.j4k.dev/j4k-oss/agent-skills/pulls/136.
test: move-codex-session history rollback test should wait for the script before cleanup
Some checks failed
commit-msg / commitlint (pull_request) Successful in 29s
Details
Node tests / node:test (pull_request) Has been cancelled
Details
Review / Review (pull_request_target) Has been cancelled
Details
7fef9c3b8f
 
The test polled the destination state database without a busy timeout. Under
load a poll hit "database is locked", the helper threw without waiting for the
script, and the test deleted the fixture while the script still ran: the
cleanup failed with ENOTEMPTY or the run hung. The poll now waits out brief
locks, and the helper always waits for the script to exit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jercik changed target branch from fix/move-codex-session-codex-0160 to main 2026-10-09 20:58:05 +00:00
jercik force-pushed fix/move-codex-session-rollback-history from 7fef9c3b8f
Some checks failed
commit-msg / commitlint (pull_request) Successful in 29s
Details
Node tests / node:test (pull_request) Has been cancelled
Details
Review / Review (pull_request_target) Has been cancelled
Details
to 7d4d9b4c10
All checks were successful
commit-msg / commitlint (pull_request) Successful in 21s
Details
Review / Review (pull_request_target) Successful in 4s
Details
Node tests / node:test (pull_request) Successful in 3m39s
Details
2026-10-09 20:58:43 +00:00 Compare
jercik merged commit 4365944ca1 into main 2026-10-09 21:03:33 +00:00
jercik deleted branch fix/move-codex-session-rollback-history 2026-10-09 21:03:33 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
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/agent-skills!133
No description provided.