Uh oh!
There was an error while loading. Please reload this page.
Reconciler refused marker: a handler refusal is terminal, not retried every boot (LLP 0184/0186/0187) - #622
Conversation
Routes the reconciler-retries-forever fix through the pipeline per the maintainer's guidance on #601: mints LLP 0184 (the request document, transcribed from the maintainer's pasted analysis) and LLP 0185 (the design), extending the Active LLP 0041 rather than editing it. LLP 0185 settles option 1 (a terminal `refused` marker state that short-circuits like `done` and surfaces as attention-needed in `hyp status`), widens ActionOutcome/ActionMarkerStatus/ClientActionState to carry it, and specifies how the transient/permanent bit crosses the kernel's throw-only attach() seam (markActionRefused/isActionRefused) so OpenClaw's ownership-conflict refusal and Claude's JSONC refusal both migrate without special-casing either adapter. Re-arm is the explicit `hyp attach` re-run only, per the maintainer's instruction; the isCurrent-style input-hash re-arm (LLP 0086 precedent) is recorded as a named follow-up candidate, not built. attempts-bounding on transient `failed` markers is left explicitly open. LLP 0041's Extended-by: line gets a mechanical forward-ref to LLP 0185; nothing it settled is edited.
PR #608 (fix/issue-604) already carries llp/0185-unpicked-client-stays-attached.decision.md and is further along, so this change set takes the next free number instead. LLP 0041's Extended-by forward-ref updated to match.
Turns LLP 0186's design (option 1, the terminal refused marker state per #601) into an eight-task graph: the widened ActionMarkerStatus/ActionOutcome type seam, the action_refusal.js marked-Error convention, the reconciler's unconditional refused short-circuit, the action_attach.js catch translation, the OpenClaw and Claude refusal-site migrations, the hyp status attention-needed surface, and the explicit hyp attach re-arm.
Adds 'refused' to ActionMarkerStatus, ActionOutcome.status, and ReconcileActionResult.outcome, and adds the new ActionRefusalError interface (Error + required hypActionRefused: true) that lets a marked throw cross the kernel's throw-only attach() seam. Revises ActionMarker's doc comment to note a refused marker reuses `at` and `reason`, carries no `attempts`, and preserves `installed_assets` across rewrites the same way done/failed markers do. This is the type seam every other task in the reconciler-refused-marker change set (LLP 0186 / LLP 0187) builds on. Task-Id: T1
A successful manual `hyp attach <client>` now clears whatever attach marker sits at that request key, mirroring the existing detach-side clearClientActionMarker call. This is the only re-arm a `refused` marker gets in this pass: with no marker left, the next reconcile pass treats the client as a fresh target and re-`perform()`s on its own. A failed manual attach never reaches the clear, so a refused marker survives a failed re-run unchanged. Adds test/core/attach-refused-rearm.test.js, mirroring test/core/detach-rejoin-recovery.test.js's pattern. Task-Id: T8
…orever
A precondition refusal only the user can fix (a conflicting provider entry,
a JSONC settings file) came back as `failed`, so every reconcile pass
re-performed it and bumped `attempts` forever (LLP 0184). The forward-gap
loop now short-circuits a `refused` marker unconditionally, without
consulting `markerIsCurrent()`: the freshness hook answers "did the input
drift?", which says nothing about whether the refusal was resolved, and
re-arming is the explicit `hyp attach` re-run only in this pass.
A third outcome branch writes `{status, request_key, reason, at}` with no
`attempts` (nothing will ever increment one) and carries `installed_assets`
forward the way the `done` branch does, so a refusal on a re-perform never
orphans what an earlier successful attach copied. The reverse gap treats a
`refused` marker the way it treats a `failed` one: dropped when assetless,
routed to `reverse()` when it recorded an effect. `runOutcome()` accepts the
new shape, but only from a handler's return value: a bare throw stays
`failed`.
Task-Id: T3Widens ClientActionState with a refused member and renders it distinctly from failed in both hyp status --json (state/reason/at fall through existing spreads) and prose (a repair hint pointing at hyp attach), so a refused marker reads as attention-needed rather than a silent retry. Task-Id: T7
New sibling module to action_attach.js / action_backfill.js in src/core/config/. markActionRefused sets hypActionRefused: true on a thrown Error so it survives the kernel's throw-only attach(): Promise<void> seam; isActionRefused reads the marker back defensively, tolerant of any non-Error or unmarked throw. Realizes the ActionRefusalError interface LLP 0186/T1 already widened types.d.ts with. Task-Id: T2
perform()'s catch now reads isActionRefused(err) to tell a permanent precondition refusal (LLP 0186) apart from a transient environmental failure, so the reconciler can short-circuit the former unconditionally instead of retrying it forever. Task-Id: T4
settings.js's JSONC detection throw site now wraps its ClaudeSettingsError with markActionRefused so the marked Error survives index.js's attach() rethrow, letting the reconciler treat it as a terminal refused outcome instead of retrying an edit attach can never safely make. Other ClaudeSettingsError throw sites are unchanged. Adds the first test coverage for this refusal path plus a companion case proving a plain malformed-JSON throw is not marked as refused. Task-Id: T6
The `models.providers` ownership conflict (LLP 0167#attach-detach: a value
HypAware did not write sits at a key attach owns) is a property of the user's
config. No reconciler pass changes it, but the adapter reported it as
`{status:'failed'}`, so the marker was retried on every pass with `attempts`
climbing forever, which is LLP 0184's bug.
`OpenclawAttachOutcome` gains `{status:'refused', reason}` and that one call
site returns it. The other four `errorKind`s (`settings_path`, `endpoint`,
`read`, `write`) are environmental and may well clear on their own, so they
keep returning `{status:'failed'}` and keep retrying.
Mechanically, `fail()`'s side effects split out into `reportAttachFailure()`
and the refusal gets its own `refuse()` wrapper rather than a `status`
argument on the shared five-call-site helper: there is then no parameter a
future call site can pass wrong, and the four transient kinds cannot drift.
Both surfaces `reportAttachFailure()` writes are unchanged, so
`writeAttachOutput`'s `--json` payload and prose still say `status: 'failed'`
for a refusal: the split is about how the reconciler schedules a retry, not
about what the user's attach just did, and `--json` is a wire contract a
scripted caller already parses.
`index.js`'s wrapper throws `markActionRefused(new Error(reason))` for the
refused outcome, alongside the unchanged bare-`Error` throw for `failed`. The
kernel types the registered `attach()` as `Promise<void>`, so the mark is the
only way the terminal/transient bit survives that seam to `perform()`'s catch.
Tests: the three ownership-conflict assertions in openclaw-attach.test.js
become `'refused'`; the missing- and malformed-`openclaw.json` cases still
assert `'failed'`, proving the four environmental kinds did not migrate. At
the wrapper seam, the rethrown refusal is asserted marked and a hard failure
asserted unmarked. The `perform()`-level test stops asserting which non-`done`
status the catch produces: that classification is action_attach.js's, tested
there.
Task-Id: T5…ry runs Review round 1 of PR #622. The re-arm added in T8 cleared the attach marker unconditionally after any successful manual `hyp attach`, which had two effects nothing in LLP 0186 asked for: - A `done` marker carrying `installed_assets` was deleted. That marker is the only record naming the files an org-driven attach installed, and `hyp detach` reads exactly it to know what to remove, so a manual re-attach followed by a detach left those files stranded (LLP 0138#marker-undo, the same invariant the reconciler's own carry-forward branches protect). - `hyp attach --dry-run` wrote to the marker store. `hyp detach --dry-run` returns before its own clear; the attach side did not. Gate the clear on the marker's prior status being `refused`, and skip it under --dry-run. That is exactly the re-arm the maintainer asked for on issue #601 and nothing more. Also strengthen the one end-to-end assertion over the whole OpenClaw chain back to the exact status (it had been weakened to `notEqual 'done'`, which would stay green if any link regressed to `failed`), and cover the Claude half of the seam at the registration level: the JSONC refusal only reaches the reconciler if `index.js`'s wrapper and `withSpan` rethrow the marked Error unchanged, which nothing tested. LLP 0186's re-arm and test-strategy sections are updated in the same commit to match. Co-Authored-By: Claude <noreply@anthropic.com>
philcunliffe
commented
Aug 5, 2026
Neutral review, round 1 of 2Verdict: approve after the fixes below, which are pushed. The design is faithful to what the maintainer settled on #601 (option 1 only, explicit Reviewed at head FindingsF1 (medium-high, fixed) - the re-arm deleted That is a straight regression against LLP 0138#marker-undo, the invariant this same PR restates twice in the reconciler's own carry-forward comments ( Fixed by gating the clear on F2 (medium, fixed) - Fixed by skipping the clear entirely when F3 (low, fixed) - the only end-to-end assertion over the OpenClaw chain was weakened, not migrated. F4 (low, fixed) - Claude's half of the adapter seam had no registration-level coverage. LLP 0186's re-arm and test-strategy sections are updated in the same commit to match F1/F2 (the design's "the implementation does not need to branch on the marker's prior status before calling it" is what produced F1). Nothing LLP 0186 settled about the marker state itself was touched. Verified clean
Residual findingsBoth are low, neither is worth blocking on, neither is fixed.
Numbers
Pushed: yes, |
Review round 2 of PR #622. Round 1 scoped the manual-attach re-arm to `refused` markers so a `done` one stops being collateral, but a `refused` marker is an undo record too: the reconciler's own `refused` write branch carries `installed_assets` forward across the rewrite precisely because a refusal on a re-`perform()` does not un-install what an earlier successful attach copied. Clearing such a marker is the same data loss round 1 fixed for `done`, one status over: `hyp detach` reads exactly that marker to know what to remove, so the org-installed files are stranded with nothing naming them. Reproduced against the real `attach()` command path (seeded a `refused` marker with `installed_assets`, ran attach, marker came back undefined). Re-arm now goes through `rearmRefusedActionMarker`, beside the store it rewrites. An assetless `refused` marker is still dropped outright, exactly as before. One that records assets is rewritten to `failed` with the record intact: `failed` is short-circuited by nothing, so it re-arms the forward gap exactly as a cleared marker does, the reverse gap and `hyp detach` keep reading the same undo record, and the next successful `perform()` unions the carried paths onto the fresh `done` marker. Also in this commit: - `hyp status` prints the refused repair hint unconditionally instead of from inside the reason guard, so a marker whose `reason` is empty or unreadable renders the next step rather than a bare `[refused]` (round 1's residual R1). - The reverse gap's failure arm now says why a `refused` reverse outcome deliberately lands there: `ActionOutcome` is one type across both hooks, so widening it made the status expressible on `reverse()`, but nothing produces it and dropping a marker over an unreversed effect is the more expensive wrong answer. Recorded as out of scope in LLP 0186 rather than left latent (round 1's residual R2). LLP 0186's re-arm, out-of-scope, and test-strategy sections are updated in the same commit. Co-Authored-By: Claude <noreply@anthropic.com>
philcunliffe
commented
Aug 5, 2026
Neutral review, round 2 of 2Verdict: approve after the fix below, which is pushed ( Reviewed at head FindingsF5 (medium, fixed) - the re-arm still destroyed an undo record, now via
Confirmed empirically before fixing: seeded a Fixed by routing the re-arm through a new
Verified in the committed tree with Round 1's residuals, re-assessedR1 (low, cosmetic) - now fixed. R2 (low, latent) - determination: a documented non-issue, closed as a recorded constraint, not as code. Reasoning, since triage will want it:
So: no behavior change. What was latent is now recorded, in the code at the branch itself ( Verified clean (round 2)
Residual findings
Numbers
The one failure is the environmental Three tests added: the Pushed: yes, |
philcunliffe
commented
Aug 5, 2026
Neutral triage on this PR (head 4fd8b70). Round 2's fix is complete. Checked Also re-checked round 2's two determinations:
No true blockers found. Two non-blocking, pre-existing deferrals from round 2 are recorded for tracking rather than left to rot: the |
Uh oh!
There was an error while loading. Please reload this page.
Implements the terminal
refusedmarker state, so the action reconciler stops retrying a client action that can only ever fail.Routed through the pipeline at your instruction on #601: request LLP 0184 (your text, committed verbatim) -> design LLP 0186 -> executable plan LLP 0187 (8 tasks) -> this change set. LLP 0041 is untouched apart from a
Extended-by:forward-ref, so what it settled stays settled.The defect
failedwas never terminal: every reconcile pass retried it forever. Correct for a dyinghyp backfillsubprocess, wrong for a refusal whose precondition only the user can change. The field report on #601 showed"attempts": 17, one per daemon boot, withhyp statusgiving no signal that waiting would not help.Both seams are closed here: the marker could not express "permanent", and the handlers already computed the transient/permanent bit but dropped it before the marker was written.
What landed
refusedmarker state (src/core/config/action_reconciler.js,src/core/config/types.d.ts) that short-circuits likedonebut carries noattempts. The short-circuit is unconditional (not gated onisCurrent), per your "no new per-handler contract".src/core/config/action_refusal.js(new):markActionRefused/isActionRefusedcarry the bit across the adapter seam on the thrown Error, so the Accepted kernel contract inhypaware-plugin-kernel-types.d.tsis not widened and unmigrated adapters keep working unchanged.refused. Every other failure site (settings_path,endpoint,read,write, malformed JSON, backfill) staysfailed.hyp statusgains a distinctrefusedstate with the repair hint, and never flipsoverall.hyp attach <client>clears the marker, mirroring the existing detach-side call. TheisCurrent-style input-hash re-arm is recorded in LLP 0186 as a named follow-up and deliberately not built.Deliberately out of scope
attemptson genuinely transientfailedmarkers (your own Open Question in LLP 0184 did not settle it).refused.Verification
8 tasks, each implemented in isolation and merged
--no-ffafter its own verified gate. New coverage:action-refusal.test.js,attach-refused-rearm.test.js, plus extensions to the reconciler, attach, status, OpenClaw and Claude suites. Note the Claude JSONC refusal path had no prior test coverage anywhere in the repo, so that is new rather than extended.CI on this PR is the authority for the suite; the per-task local runs were the precondition to merge, not the proof.
Fixes#601
Change-Set: reconciler-refused-marker