Follow-up: deferred review findings from PR #622

[philcunliffe]

Neutral triage on PR #622 (head 4fd8b70b799e463e7ff649172168685a82b481c9, the reconciler-refused-marker change set, LLP 0184/0186/0187, Fixes #601) confirmed round 2's fix (rearmRefusedActionMarker) is complete and found no production-risk blockers. Two items round 2 recorded as deliberate deferrals are worth tracking so they do not silently rot:

1. reverse() returning refused falls into the generic failure branch

src/core/config/action_reconciler.js:340-356. ActionOutcome is one type shared by perform() and reverse(), so widening it for the terminal refused state also made refused expressible as a reverse() return value, even though no in-tree handler's reverse() produces it today (verified: action_attach.js's is the only reverse() in the tree, and it only ever returns done or failed). If a future reverse() ever does return refused, the reverse gap's else branch treats it exactly like failed: keep the marker, retry forever. That is documented as the deliberately safe half of a genuine open design question (LLP 0186 Explicitly out of scope, referencing LLP 0138#refusal-is-not-failure vs LLP 0212) rather than an oversight, and today it is dead code. Follow-up: when a real reverse-refusal need shows up, settle terminal-undo semantics explicitly (a new LLP) rather than let the fallback keep standing in by accident.

2. hyp leave's reverse gate was not migrated to the refused status

src/core/commands/central.js:435. The reconciler's own reverse gap (action_reconciler.js:315-319) and rearmRefusedActionMarker both special-case refused explicitly. hyp leave's parallel assetless-marker drop only checks marker.status === 'failed':

if (!marker || (marker.status === 'failed' && installedAssets.length === 0)) {

An assetless refused marker does not match this condition, so it falls through to the full detachClientViaCore path instead of being dropped directly the way the reconciler's reverse gap drops it. Verified this is safe today: detachClientFromDisk (src/core/config/client_detach_disk.js) no-ops ({ changed: false }) when there is nothing on disk to reverse (a refusal never wrote the client's settings), and the marker still gets cleared afterward inside detachClientViaCore. So this is not data loss, just an inconsistency between the three marker-reading gates (reconciler reverse gap, hyp detach, hyp leave) that costs an unnecessary disk probe and diverges from the pattern the other two gates now follow. Follow-up: add the || marker.status === 'refused' branch to central.js:435 for consistency, and to avoid future drift if a fourth marker status is ever added and only two of the three gates get updated for it.

Neither item blocks PR #622; both are pre-existing, deliberate deferrals from round 2's review, recorded here so they don't get lost.

Backlink: #622

[philcunliffe]

Parked: this targets code that is not on master yet

What neutral was doing. This issue carries neutral:fix, so the maintenance reconciler picked it up to produce a fix PR.

Why it cannot proceed. Both deferred findings are about the refused marker state, and that state does not exist on master:

git show origin/master:src/core/config/action_reconciler.js | grep -c refused
0

The whole feature lives only on integration/reconciler-refused-marker, which is PR #622 - ready, green, reviewed, and held for you to merge. A fix branched from master would have nothing to change.

Why neutral is not stacking this on #622. It could branch from the integration branch, but that has a known cost, demonstrated in this repo today. PR #625 was stacked on PR #608's branch; when #608 was squash-merged, the squash rewrote the history #625 was built on and #625 immediately went CONFLICTING. Stacking #627 on #622 would set up exactly the same collision the moment you merge #622. Waiting is cheaper and safer than resolving a conflict neutral created on purpose.

What it needs from you. Nothing but the merge of #622. Once that lands on master:

• this issue unsticks on a later tick (reply here, or just drop neutral:stuck), and
• the two deferred findings become ordinary fix-family work against master.

For reference, the two findings, both classified preference by triage and neither a blocker:

1. hyp leave's reverse gate was never migrated. src/core/commands/central.js:435 short-circuits the assetless-drop on marker.status === 'failed' only, not refused. Triage traced the fallthrough: it reaches detachClientViaCore to detachClientFromDisk, which no-ops safely ({changed: false}) because a refusal never wrote the client's settings, and the marker is cleared afterwards. So this is an inconsistency across the three marker-reading gates, not data loss.
2. reverse() returning refused would retry forever (recorded at action_reconciler.js:340 and in LLP 0186's out-of-scope section). Confirmed dead code: action_attach.js holds the only reverse() in the tree and it returns only done or failed. Note this one is a design question, not a bug - choosing terminal-undo semantics has two settled LLPs pulling opposite ways (LLP 0138 #refusal-is-not-failure vs client attach: probe-less contributes.client can attach but reverse() silently no-ops, orphaning settings #212). If you want it closed, it wants a design decision first, the way Action reconciler retries permanently-failed client actions on every boot (LLP 0184) #601 was handled.

How to unstick. Merge #622, then reply here or remove neutral:stuck. Neutral monitors this thread and re-engages on its next tick.

[philcunliffe]

Unparking: the condition this was waiting on is met. #622 merged (b56ab21), so the refused marker state is now on master, and src/core/commands/central.js:435 still carries the un-migrated gate.

Neutral is picking up finding 1 (the hyp leave reverse gate checking only failed, not refused) on this tick.

Finding 2 is deliberately not being implemented. Choosing terminal-undo semantics for a reverse() that returns refused is a design decision with two settled LLPs pointing opposite ways (LLP 0138 #refusal-is-not-failure vs #212), and it is confirmed dead code today. It stays recorded as an out-of-scope constraint in LLP 0186. If you want it settled, it wants a design decision first, the way #601 was handled - say so here and neutral will route it through the pipeline instead.