Fix --skip-unchanged deadlocking a goal loop that is the only writer of its tree (#217 item 13)

[scgopi]

The deadlock

With --skip-unchanged, every goal poll captured a workspace fingerprint (git rev-parse HEAD + git status --porcelain hashed) and returned early if it matched the one recorded at the last failing run — no predicate re-run, no failure relayed (GraphStore.swift:2053-2062 at 0.1.56).

For a loop that is the only writer of its tree, that is a fixed point: the loop only writes once woken, and waking was gated on a tree change. After a turn that ends without moving the tree, every poll skips forever — the ~60 min silence from issue #217 item 13.

The fix

• The skip now only applies while the session is busy. That is the case its optimisation is for: the agent is mid-turn, its next write is what would change the tree, and re-running an expensive predicate buys the same failing answer. A missing fingerprint still falls through to a real run.
• Idle plus unchanged is wake-worthy — once per frozen tree. On the first poll that finds an idle session on an already-failed tree, the predicate runs again (the only path on which an external watcher's change is ever seen) and the failure is re-delivered even when the tail is identical. A marker (reawakenedFingerprints) then holds the skip until the tree moves; a new failing run at a changed fingerprint makes one more wake available. Bounded: one extra predicate run and one extra relay per frozen tree, never a relay per poll — the unbounded agent-turn spend the failure-tail dedup exists to prevent stays prevented.
• Presence-less sessions stay skipped, deliberately. The relay refuses to tell a session it cannot see idle, so falling through for a nil presence would only buy the predicate's price for a wake that cannot land; the contract is documented at the guard and pinned by a test. Such a loop's exits are its stall bound and its human.
• Help text corrected. It previously said to use the flag "only for predicates that depend on the tree" — the exact case that deadlocks. Now it states the busy/idle contract and the one-notice-per-frozen-tree bound.
• Create- and update-time warning. node create and node update --skip-unchanged true print the advice on stderr (stdout stays the rendered graph) via GraphcodeCommand.createWarnings(for:) / .updateWarnings(for:currentNode:). The update path is best-effort: a node not visible at the top level (a sub-graph child — item 15) warns nobody rather than warning wrongly.

Tests

Six tests in PredicateFeedbackTests cover the new behaviour:

• anIdleLoopOnAnUnchangedTreeIsWokenOnceNotEveryPoll — the item 13 regression, encoding the bound: wake once, then quiet (predicate runs and deliveries both stop).
• aTreeChangeBuysOneMoreWakeOnTheNextFreeze — the bound is per fingerprint, not per loop lifetime.
• aPresencelessSessionStaysSkippedRatherThanPayingForAWakeThatCannotLand — nil presence takes the skip.
• anIdleLoopOnAnUnchangedTreeStillNoticesThePredicatePassing — a CI-style predicate going green on an unchanged tree still resolves the node, on the wake poll.
• theCreateWarningFiresOnlyForSkipUnchangedPairedWithAPredicate and theUpdateWarningFiresWhenSkipUnchangedIsTurnedOnForAPredicatedGoalLoop — the warnings gate exactly on goal + predicate + flag.

Full suite: 1299 tests passed. swiftlint 0 errors; swift-format strict clean on the touched files.

Refs #217 (item 13).

[scgopi]

Review verdict: ✅ Approve

Reviewed against issue #217 item 13 and verified on the PR head (78ed83a) from a worktree. (GitHub won't accept a formal self-approval from this account, so the verdict is posted as a comment — the review itself is full-strength.)

Correctness vs the reported deadlock

The pre-fix skip (fingerprint matches → return, applied on every poll) was a fixed point for a loop that is the only writer of its tree: it only writes once woken, and waking was gated on a tree change. The fix resolves both halves of that:

Path	Behaviour	Verdict
Busy + unchanged tree	skip retained (skip predicate re-run)	✅ the case the optimisation is for
Idle + unchanged tree	predicate re-runs; lastPredicateFeedback cleared so the identical-tail dedup in relayPredicateFailure (GraphStore.swift:2155) can't suppress the wake	✅ un-strands item 13
Idle + unchanged + CI goes green	predicate re-runs → node resolves	✅ covered by test
.absent / .unknown / .awaitingInput presence	still skips	✅ same as pre-fix — the fix is a strict superset of wake paths, nothing removed

Details that check out:

• The presence == .idle gate exactly mirrors relayPredicateFailure's own deliverability gate (GraphStore.swift:2162), so the wake only opens when a relay could actually be delivered.
• createWarnings' effectivePredicate != nil guard matches the poller's hasPredicate logic — the warning fires only when the flag is paired with a live predicate (a blank predicate never arms that half of the poller).
• Re-delivery every poll while idle+unchanged+failing is the right call, not an accident: deliver-once-per-idle would re-strand (loop wakes, changes nothing, dedup suppresses the next wake). The help text and the create-time warning state this contract.
• Warning goes to stderr; stdout stays the rendered graph. Also fires on the --into composite path (same draft) — correct.
• Docs fix is right: the old help recommended exactly the case that deadlocked.

One non-blocking nit: the new idle tests exercise the node.presence fallback (no onReadPresence wired), not the production onReadPresence path. Production wiring is a thin pass-through of the same Presence value, so risk is low — fine to leave.

Verification on the PR branch

Check	Result
Full test suite (xcodebuild test, worktree at 78ed83a)	✅ 1296 tests passed (1293 before, +3)
All 3 new tests ran (not silently skipped)	✅ confirmed in the log by name
swiftlint lint errors	✅ 0
swift format lint --strict on the 5 touched files	✅ clean

Fixes the bug, no regressions found. Approving.

[scgopi]

All three findings addressed in 88cf3e7:

1. Wake-spam — the re-delivery is now spent once per fingerprint (reawakenedFingerprints): the idle+unchanged poll re-runs the predicate and re-delivers once, then the skip holds until the tree moves; a failing run at a new fingerprint makes one more wake available. anIdleLoopOnAnUnchangedTreeIsWokenOnceNotEveryPoll now asserts 4 polls → 2 predicate runs, 2 deliveries; aTreeChangeBuysOneMoreWakeOnTheNextFreeze pins the per-fingerprint (not per-lifetime) semantics.
2. Presence nil — stays skipped, with the reason documented at the guard: the relay only tells a session it can see idle, so falling through would pay the predicate's price for a wake that cannot land. Pinned by aPresencelessSessionStaysSkippedRatherThanPayingForAWakeThatCannotLand.
3. node update warning — GraphcodeCommand.updateWarnings(for:currentNode:) prints the same advice when --skip-unchanged true lands on a goal loop with a predicate (judging the predicate as the update leaves it), using the graph the update path already loads. Best-effort: an unresolvable node warns nobody rather than wrongly.

Suite at 1299 passing; lint/format clean.