Skip to content

Retry a timed-out follow-up whose send then fails; fix the gate after #320 - #321

Merged
scgopi merged 2 commits into
mainfrom
fix/320-timed-out-followup-lost
Sep 7, 2026
Merged

scgopi merged 2 commits into
mainfrom
fix/320-timed-out-followup-lost

Conversation

@scgopi

@scgopi scgopi commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Makes main green after #320 and fixes the defect its exactly-once bookkeeping left behind. Two commits: the gate fixes the Release loop verified (.claude/handoff/gate-fixes-320.patch, applied as-is), then the store fix with tests.

Gate fixes (commit 1)

  • DrainWedgeVerificationTests.swift passed deliveryDeadline: last, but GraphStore.init declares it second, right after graph:. Swift matches an argument list in declaration order, so the test target did not compile; the reported messageNode / refreshUsage errors were downstream noise. Moved it to where it is declared.
  • ProjectPersistence.swift failed swift-format --strict at 61:10, 62:1, 67:14. Reformatted with the formatter on that file only.

The defect (commit 2)

#320 keeps a timed-out delivery on the queue until its abandoned send reports a verdict. A late success is removed, exactly once — correct. A late failure went through staged(_:), which only writes the memory log and returns a marked copy; the return was discarded in both finishTimedOutDelivery and the drain's fold-back, so the item silently left the live queue. A target that was idle and answering never received text it was owed, while a send that failed before the deadline was retained for retry (the !delivered branch on main). Both failures are now treated the same: staged once, kept in place on the queue, and retried by a drain the verdict itself kicks — a failure learned this late has no command to ride the drain of, so without its own it waited for the next poll, or for good if none came.

The test that was failing

DeliveryWedgeTests.aHungDeliveryStillFreezesEveryOtherLoopsFollowUps asserted delivered would contain the hung one's message, but its own onDeliverMessage closure never appended for the hung node — the hung branch returned true without recording. The last expectation was unreachable whatever the store did, and the store was in fact correct for that path: the send succeeded late, so the message must not be sent again. The closure now records its late success, and the test checks two further drains send nothing more. The new test aTimedOutSendThatThenFailsIsRetriedExactlyOnce covers the late-failure path and fails on main's GraphStore:

✘ aTimedOutSendThatThenFailsIsRetriedExactlyOnce … Expectation failed: (delivered.value → ["[graphcode] for the bystander"]) == ["[graphcode] for the bystander", "[graphcode] for the hung one"]
✘ … Expectation failed: (attempts.value → 1) == 2

Both tests now await the wedged command's return instead of a ten-second settle — the deadline is what returns it — and then refresh, because the bystander's message is queued mid-drain and moves on the pass after. The two suites run in 0.3s instead of 24s.

Not covered here: a send that never reports at all stays parked in pendingDeliveryAttempts, unstaged, until the daemon restarts. Same as #320's main; noted so it is not mistaken for fixed.

Gate

In flight from worktrees/fix-320-lost, private DerivedData; numbers posted below when it finishes.

  • make check exit 0 (swiftlint 0 errors, swift-format clean)
  • targeted DeliveryWedgeTests + DrainWedgeVerificationTests: exit 0, 6 tests; red on main's GraphStore.swift (exit 65, the new test only)

🤖 Generated with Claude Code

https://claude.ai/code/session_014niBD1D1yQ7F2gjV2K54eB

scgopi and others added 2 commits September 6, 2026 22:19
…e formatting

DrainWedgeVerificationTests.swift passed deliveryDeadline: last, but GraphStore.init
declares it right after graph:, so the test target did not compile. swift-format --strict
also rejected ProjectPersistence.swift; this is the formatter's own output for that file.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014niBD1D1yQ7F2gjV2K54eB
#320 kept a timed-out delivery on the queue until its abandoned send reported a
verdict. A late success removed it, exactly once. A late failure went through
staged(_:), which only writes the memory log and returns a marked copy — the
return was discarded, in finishTimedOutDelivery and in the drain's fold-back
alike, so the item left the live queue. A target that was idle and answering
never received text it was owed, while a send that failed before the deadline
was retained for retry. Both failures are now the same: staged once, kept in
place, retried by a drain the verdict itself kicks rather than the next poll.

DeliveryWedgeTests' hung-send test asserted a delivery its own closure could
never record — the hung branch returned without appending — so its last
expectation was unreachable whatever the store did. The closure now records
the late success and the test checks it is not sent twice. A second test
covers the late failure and fails on main. Both await the wedged command's
return instead of a ten-second settle: the deadline is what returns it, and
the bystander's mail, queued mid-drain, moves on the pass after.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014niBD1D1yQ7F2gjV2K54eB
@scgopi

scgopi commented Sep 7, 2026

Copy link
Copy Markdown
Owner Author

Gate on 2fd34769 (= remote head), from worktrees/fix-320-lost, private DerivedData:

Step Result
xcodebuild test (scheme graphcode) exit 0 — 1692 tests / 180 suites / 0 failures; 0 ** TEST FAILED **, 0 Failing tests:
graphcode-cli build exit 0
graphcoded build exit 0
make check exit 0 — swiftlint 0 errors, swift-format clean
scripts/cli-smoke.sh exit 0 on all five verbs, against a fresh swift build of this head (the worktree's .build link points at the main checkout's stale debug binary, so the script's own build was bypassed with a scratch path)
Linux build pass — https://github.com/scgopi/GraphCode/actions/runs/34087301065/job/101633685121
Red on main aTimedOutSendThatThenFailsIsRetriedExactlyOnce exit 65 with main's GraphStore.swift (attempts → 1, hung message never re-sent); exit 0 with it restored

Also, #318's PresencePollingTests ran 30 iterations in isolation on this head (-test-iterations 30 -run-tests-until-failure): 300/300 passed.

@scgopi
scgopi merged commit 3b68b69 into main Sep 7, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant