hyp status: a probe-less client is unattachable, not unattached - #553
hyp status: a probe-less client is unattachable, not unattached#553philcunliffe wants to merge 2 commits into
Conversation
`action_attach.desired()` skips a client descriptor with no `attach_probe`
(attach must be reversible, and only the probe can reverse it), so
`perform()` never runs and no attach marker is ever written for openclaw or
claude-desktop. Three `hyp status` surfaces derived against the attach
contract without that gate, and each turned that permanent silence into a
permanent negative on a joined host:
clients:
- openclaw [configured, not attached] [local]
client actions:
- attach openclaw [pending]
diagnostics:
[WARN ] client_attach_missing: '@hypaware/openclaw' is enabled but
openclaw settings show no HypAware marker - run 'hyp attach --client
openclaw'
None of the three can ever resolve, and the printed repair resolves the
adapter's deliberate LLP 0143 no-op, so running it clears nothing.
Gate all three on `descriptor.attachProbe`, the same rule `desired()` uses:
- the client-actions row derives `n/a` (via a new `inert` flag on the
declared-attach entry) rather than a permanent `pending`;
- `ClientAttachReport` gains a required `attachable`, and the clients row
prints `attach n/a` instead of `not attached`; `--json` carries
`attachable` beside the unchanged `attached` boolean;
- `client_attach_missing` no longer fires for a probe-less client.
Clients that do declare a probe are untouched: claude with no marker still
reports `pending`, `not attached`, and the warning.
LLP 0143 grows #status-derives-by-the-same-gate for the rule; LLP 0044's
status-surface vocabulary and LLP 0139 #repair-must-be-runnable are amended
to match (Desktop's attach-missing warning fired identically before consent,
after a decline, and after a successful install, so withdrawing it loses no
signal; `hyp claude-desktop verify` is the check that can answer it).
Co-Authored-By: Claude <noreply@anthropic.com>
The probe-less gate removed `client_attach_missing` for openclaw, but docs/ACCEPTANCE.md step 1 of the OpenClaw flow still told the release tester to expect that warning, so its (correct) absence would read as a regression. Restate the expectation as the pass condition. The `@ref LLP 0139#repair-must-be-runnable` gloss claimed the repair names an adapterless client's `configure_command`; after the gate the only adapterless client (claude-desktop) never reaches that code, and no shipped picker row takes the branch. Re-glossed to what the code does, with LLP 0139's amendment box recording the same. README's diagnostics table said `client_attach_missing` fires for any enabled client plugin with no marker; it is now probe-gated. Co-Authored-By: Claude <noreply@anthropic.com>
Neutral review round - PR #553 @
|
| File | Before | After |
|---|---|---|
docs/ACCEPTANCE.md:302 |
Expect `hyp status` to also carry a `client_attach_missing` warning for |
Expect `hyp status` to carry **no** `client_attach_missing` warning for |
src/core/daemon/status.js |
:626 gloss an adapterless client's attach-missing repair names its configure_command |
:629 gloss the repair is the client's own picker configure_command when it declares one |
README.md:381 |
a client plugin is enabled but its settings file shows no HypAware marker |
a client plugin with an attach probe is enabled but shows no HypAware marker |
llp/0139-...decision.md:220 |
(absent) | no plugin shipping today takes its first branch |
Each row was read back with git show <sha>:<path>, not inferred from a green suite (LLP 0002).
Neutral review round 2 - PR #553 @
|
| home | claude / codex (probed) |
openclaw / claude-desktop (probe-less) |
|---|---|---|
| A no settings files | attachable:true, [configured, not attached], attach [pending], client_attach_missing fired |
attachable:false, [configured, attach n/a], attach [n/a], no warning |
B settings file present, no marker ({"theme":"dark"}, [model_providers.other]) |
identical to A - all three negatives survive | same |
C probe errors (corrupt JSON; EISDIR on the TOML) |
attachable:true with error carried to text and --json; not attached, pending, client_attach_missing all still fired |
same |
| D marker present | attachable:true, attached:true, [configured, attached], no warning |
same |
So a probed client that is genuinely unattached still reports the full trio, including the two cases the brief singled out (probe reads back no marker; probe throws). An unresolvable/erroring probe is not collapsed into the probe-less bucket - attachable keys strictly on !!descriptor.attachProbe (status.js:600), never on the probe result.
I also enumerated contributes.client across every bundled manifest, so the comment's parenthetical is exhaustive rather than illustrative: probe-less = {claude-desktop, openclaw}, probed = {claude, codex}.
Mutation testing (each mutation applied to status.js, targeted tests run, then reverted):
| mutation | result |
|---|---|
drop attachable from the construction site |
npm run typecheck fails: TS2345 ... Property 'attachable' is missing ... but required in type 'ClientAttachReport' at status.js(604,18) |
const attachable = true (revert the gate) |
2 failures in status-probeless-client.test.js |
const attachable = false (maximal over-suppression) |
2 failures - the over-suppression guards are load-bearing, not decorative |
const inert = false |
1 failure (attach openclaw back to permanent pending) |
const inert = true |
1 failure (attach claude loses its real pending) |
Both directions are pinned. client_attach_stale (status.js:638-644) sits on the else if and requires probe.attached, so it is unreachable for a probe-less client and untouched for a probed one.
3. The required attachable field
grep -rn ClientAttachReport finds one production construction site (src/core/daemon/status.js:604) and one test-side one (test/core/status-client-error.test.js:67), both of which set it; the typecheck mutation above proves a missed site cannot land silently.
--json consumers are unbroken: renderStatusJson (src/core/commands/status.js:180-190) still emits attached as a boolean, in place, for every row - confirmed in all four runtime scenarios above, including the probe-less rows where it stays false exactly as before. I checked every reader of report.clients: wizard/index.js:294 (filter(c => c.attached)), wizard/fork.js:269 and commands/status.js:58 (both filter on configured), and hypaware-core/smoke/flows/status_diagnostics.js:298 (pins claude, a probed client). None changes answer.
On round 1's open nit (c.attachable !== false at commands/status.js:186, === false at :363): I checked whether a cross-version report could actually reach the renderers, since that would make the defensive form correct rather than redundant. It cannot - report.clients is built in-process at status.js:586-613; only daemon/sources come from status.json. So the defensive read protects nothing today, but it is consistent in both directions (a missing field degrades to the pre-PR reading in both renderers) and breaks nothing. Concurring with round 1: noted, not actionable, not worth the churn.
4. LLP 0139 amendment, and 0115 / 0143 consistency
The amendment box (llp/0139-desktop-picker-consent.decision.md:212-224) still reads correctly: the rule stands, the lookup stays, the warning stops firing for probe-less clients, and the "fired identically before consent / after decline / after success" reasoning is intact and is what carries the claim that no signal is lost. Its new sentence ("Claude Desktop was the only picker row declaring a configure_command, no plugin shipping today takes its first branch") is the claim I verified independently in section 1 - it holds, and it is correctly scoped to today's shipped set rather than stated as a permanent property.
No contradiction with the neighbours. LLP 0143's Consequences (:120-127) tell the same story from the other side and name hyp claude-desktop verify as the real check; the amendment does not restate anything 0143 denies. LLP 0115 #no-attach-on-join (:98-102) asserts only that Desktop registers no attach_probe and exposes explicit commands instead - it makes no claim about hyp status attach state at all (grepping 0115 for hyp status / attach_missing / pending / attached returns nothing), so there is nothing for the amendment to contradict. llp/0139:46-52 still describes the old repair, but it is Context, in the past tense, narrating what was observed at the time - correct as history.
5. The two other doc fixes, re-checked against a running binary
docs/ACCEPTANCE.md:302-313: every renderable claim matches what I actually printed - the clients row is - openclaw [configured, attach n/a], the action row is attach openclaw [n/a], backfill @hypaware/openclaw [pending] is present beside it, and no client_attach_missing mentions openclaw. client actions: is the real section header (src/core/commands/status.js:471), hyp attach openclaw is a real positional form (core_commands.js:276), and the LLP 0143 anchor link resolves. The other client_attach_missing mention in that file (:152) is in the Codex Desktop flow, a probed client, so it is still correct and was rightly left alone.
README.md:381: accurate now, and the table's column padding is intact ([39][84][74], matching every other body row).
6. House style
No U+2014 anywhere in the diff or in any touched file. The status.js delta is comments only, so the no-semicolon rule is untouched; no @typedef, no inline import('...'), and the new test's @import specifier is root-anchored. test/core/llp-ref-hygiene.test.js: 10/10 pass, so both #status-derives-by-the-same-gate and #repair-must-be-runnable resolve after the re-gloss.
Checks
npm test@03f6d47: 3273 pass / 0 fail / 1 skipped.npm run typecheck@03f6d47: clean.- Smokes:
status_diagnosticsok,client_attach_idempotentok. client_attach_on_join,claude_attach_detachandcli_bundled_plugins_activatedFAIL here - but they fail identically onorigin/masterin a second clean worktree (client_attach_on_joinreproduced 3/3 on master with the same message, "the attach.claude marker timestamp is unchanged"). Pre-existing, unrelated to this PR, and not counted against it. Flagging only so the failures are not mistaken for a regression by whoever runs them next.
Findings
None actionable. Nothing pushed; head remains 03f6d4757dab872a1c73fce19dbf32807cb21d6d. No findings are left open.
Previously adjudicated and deliberately not reopened: the backfill @hypaware/openclaw [pending] row, LLP 0143's open question about a registry-derived attach signal, and the residual that nothing in hyp status cues claude-desktop verify after a declined install. The accepted LLP 0167/0169 RFC that will give OpenClaw a real attach surface reverses this PR's premise for that one client in a future change set; it is not a defect here.
philcunliffe
added
the
neutral:approved
…p, json_path revival (#570) * Design: OpenClaw two-lane capture (LLP 0172) * Plan: OpenClaw two-lane capture executable tasks (LLP 0173) * openclaw config: validate sweep_cron and quiesce_ms in backfill section validateBackfillSection now accepts sweep_cron (a 5-field cron expression, validated with core's shared isCronExpression grammar so a malformed schedule is rejected the same way a sink's config.schedule is) and quiesce_ms (non-negative integer) alongside the existing on_join/window_days keys, added together so the unknown-key rejection loop recognizes both from the same merge. Task-Id: T6 * Restore json_path attach-probe format, add sweep field to BackfillContribution hypaware-plugin-kernel-types.d.ts: PluginAttachProbeManifest.format regains 'json_path' (removed by LLP 0143 after #212's orphaning danger), plus the new container_path, provider_keys, and cache_glob fields the format needs, reusing the existing marker_header. The comment at the removal site is revised, not deleted: it now explains that the runtime support LLP 0173 T2/T3 add closes the gap #212 warned about, so a manifest can only declare this format once both sides exist. BackfillContribution gains an optional sweep?: { cron: string } field for the daemon's periodic sweep (LLP 0173 T9), absent-by-default for every existing contribution. Both additions are purely additive: npm run typecheck (tsc --noEmit over the whole tree, including hypaware-core/plugins-workspace/openclaw) and npm test pass unchanged, proving no existing consumer's typecheck shifted. Task-Id: T1 * OpenClaw attach writes the two provider overrides, refusing to merge LLP 0169 reverses LLP 0152's premise: there is a real, reversible settings write for OpenClaw again, so the adapter's honest no-op attach() has nothing left to be honest about. New hypaware-core/plugins-workspace/openclaw/src/attach.js exports createOpenclawAttach({homeDir, env, fs}), mirroring the Claude adapter's attach() shape (same AiGatewayClientAttachContext, same withSpan('client.attach', ...), same dry-run branch). It reads openclaw.json through the one core settings-path seam (so $OPENCLAW_HOME relocation resolves the same file the manifest's probe will), refuses with {status:'failed', reason} when models.providers.anthropic or .openai is already there, and otherwise writes both entries whole from attachCtx.endpoint: bare origin for anthropic, endpoint + '/v1' for openai, each with the x-hypaware-upstream marker header and the mandatory empty models array. The refusal check runs entirely before the single atomicWriteFile, so there is no partial write to roll back, and it returns rather than throws, which is the whole mechanism by which a refuse during attach-on-join warns instead of failing the join. Every other key in openclaw.json is carried through by reference. Both output modes end with the 'openclaw gateway restart' instruction, since a --json caller is as blocked on the restart as a human is. index.js drops the no-op body, STEERING_PLUGIN_NAME, ROUTING_OWNED_BY_STEERING_PLUGIN_MESSAGE and the @ref LLP 0143#decision block, and wires activate() to the new effect. The registered attach() keeps the kernel's Promise<void> contract, so the outcome object is dropped there on purpose: both callers already derive success from a throw plus the one-line JSON the effect writes. Tests cover the refusal (including that the file is byte-identical after it, and that it never throws), the exact two-entry shape with the bare-origin/+v1 asymmetry, key preservation, the restart instruction on both output modes, dry-run, $OPENCLAW_HOME, and the missing/malformed config hard failures. The two attach tests in openclaw-client-registration.test.js are retargeted at the new behavior so the suite stays green; T10 owns that file's fuller rewrite. Task-Id: T4 * daemon/status.js: restore json_path attach-probe read branch Restores the probe.format === 'json_path' read branch removed by LLP 0143 / PR #510, parallel to the existing json/toml branches in probeClientAttachFromDescriptor: navigate container_path + each provider_keys entry, read headers[marker_header], and report attached when it equals the expected provider key for at least one configured key. Pure read, no ownership/backup concerns. Task-Id: T3 * openclaw backfill: quiesce window skips recently-modified session files listSessionFiles(agentsDir) gains an optional quiesceBeforeMs cutoff, and runOpenclawBackfill() computes it once per run as Date.now() - quiesceMs. quiesceMs resolves from the plugin's own config.backfill.quiesce_ms, defaulting to 180000ms (QUERY_FLUSH_DEBOUNCE_MS plus a one-minute margin), so a run never imports a session file OpenClaw is still mid-write on, or a settlement pass is still mid-flush against. Composes with the existing effectiveProviders/partitionByBackend CLI-backend logic (R10) rather than replacing it. Task-Id: T8 * Lane B sweep metadata: openclaw backfill provider + narrowed runner ctx createOpenclawBackfillProvider now populates the contribution's opt-in sweep field from config.backfill?.sweep_cron, defaulting to every 5 minutes when absent (LLP 0172#lane-b-sweep, R7). src/core/commands/backfill.js's runBackfillProvider, runProvider, and resolveOwnersForRun now declare a new BackfillRunnerContext interface (env, config, storage, backfills, backfillMaterializers) instead of the full CommandRunContext, a pure structural narrowing so the daemon-side sweep driver (LLP 0173 T9) can build one without assembling registries it never uses. Existing hyp backfill CLI-path and onboarding-finale call sites keep typechecking and passing unchanged. Task-Id: T7 * json_path detach returns: ownership, backup-not-discard, best-effort cache purge LLP 0143 pulled the `json_path` branch out of the disk-driven undo because LLP 0152 left nothing on disk for it to reverse. LLP 0169 reverses that premise, so the branch comes back - reshaped for the two provider entries attach now writes, not the single shadow provider of the old design. `detachJsonPathProviders` judges each `provider_keys` entry on what it points at, because this format has no HypAware-owned marker to replay: its undo record IS the entry. Ours (the gateway's own `baseUrl`, in either the bare origin or `+ /v1` spelling, with `marker_header` naming its own key, in the shape attach produces) is deleted. Anything else present at our key is backed up to a `_hypaware_detach_backup.<key>` sibling inside the same container before the live key goes, following the `prev_malformed` precedent of LLP 0163: never discard a value HypAware did not write. That closes the json/toml-vs-json_path asymmetry LLP 0163 flagged as worth its own look, converging on the outcome without the top-level marker key LLP 0163 correctly ruled out for this client. The derived caches (`cache_glob`, relative to the client's config home) are then purged of the same keys, best-effort: they do not self-heal, so a partial purge beats none, and one unreadable cache file is logged and skipped rather than failing a detach whose settings half already landed. An unknown gateway base URL has no safe default here - guessing either way silently deletes a foreign value or reports a finished detach over a client still routed at a dead port - so it refuses (`EXPECTED_BASE_URL_UNKNOWN`). Both callers degrade correctly: `reverse()` keeps the marker, `hyp detach` prints the reason. Both real callers thread the base URL: `detachClientViaCore` resolves it through the same three rungs manual attach already walks (live `localEndpoint()`, configured `listen`, the daemon's persisted bound port), every one optional so detach keeps working with the gateway capability unloaded; `reverse()` passes the `ctx.endpoint` `perform()` attached with. Task-Id: T2 * OpenClaw manifest: restore json_path attach_probe, retire steering-plugin copy hypaware.plugin.json gains contributes.client.attach_probe (design 1.4: json_path format, .openclaw/openclaw.json settings file, models.providers container, anthropic/openai provider keys, x-hypaware-upstream marker header, agents/*/agent/models.json cache glob). description and picker[0].summary drop every @hypaware/openclaw-steering-plugin reference and state the two capture tiers directly: live gateway capture once attached, plus a periodic transcript sweep. Claude's manifest gains the LLP 0167#onboarding line naming the claude-cli/<model> case OpenClaw's CLI-backend exclusion produces, so a user knows which picker entry an OpenClaw-routed Claude Code session belongs to. projector.js's UPSTREAM_HEADER comment no longer credits the deleted steering plugin; it now describes the config-override write attach() itself makes. Adds a manifest-shape test asserting attach_probe parses to the exact design-1.4 fields and that description/summary no longer match /openclaw-steering-plugin/. Updates the one existing assertion this manifest change makes false (the R7 "no attach_probe" descriptor check) to the restored json_path shape; the remaining behavioral rewrites of that test file are T10's scope. Task-Id: T5 * Delete openclaw-steering-plugin/ (LLP 0172 Section 5, R9) Removes openclaw-steering-plugin/ in full (src/, test/, package.json, openclaw.plugin.json, .d.ts files) and test/plugins/openclaw-steering-plugin.test.js: Lane A's config-override entries make OpenClaw route to the gateway on its own, so the credential-borrowing runtime auth shim, the live wire-parity mirror, the steering decision logic, the live warning ledger, and the gateway endpoint resolver that fed them no longer have a purpose. Also drops tsconfig.json's stray "openclaw-steering-plugin" include entry (line 19), not named in the design's own deletion inventory but found verifying the deletion against the real tree; leaving it would have left a dead include path. docs/ACCEPTANCE.md and test/plugins/openclaw-manifest.test.js still mention the package name (an onboarding rewrite reserved for T13, and a T5 regression test asserting the manifest no longer references it, respectively); neither is in this task's file list. Task-Id: T11 * openclaw-client-registration.test.js: finish T4/T5's deferred rewrite T4 and T5 already retargeted the two attach() no-op tests and the descriptor's attach_probe assertion to keep the suite green while they landed; this closes the two pieces both left for T10: - The "honest no-op" detach test's comment still credited the retired R7 no-attach_probe guard. Since T5 restored the manifest's json_path attach_probe, the no-op this test actually observes on a fresh temp HOME is detachClientFromDisk's absent-settings-file guard instead. Corrected the comment to say so. - Added a companion case that stages a real openclaw.json via the actual createOpenclawAttach() effect, then drives the same hyp detach CLI entry point (buildClientDescriptorMap's real manifest descriptor -> detachClientFromDisk's json_path branch) and asserts the ownership-based detachJsonPathProviders (T2) actually fires: changed:true, the removed baseUrl, and both provider entries gone from the file while everything else in it is untouched. Task-Id: T10 * Daemon sweep driver: run sweep-bearing backfill providers on the tick loop New `src/core/daemon/backfill_sweep.js`. `createBackfillSweepDriver({backfills, backfillMaterializers, env, config, storage})`'s `tick({now})` walks `backfills.list()`, skips any contribution with no `sweep` field or a cron that is not due (`cronMatches`, the sink driver's own due-check), and fires `runBackfillProvider` per due contribution with a `sweep-<name>-<now>` dev run id. Runs are fired unblocked: `tick()` resolves once each run has been started, never once one finishes, so a provider's transcript scan cannot stall the sink snapshots, the source-detail refresh, or `persist()` later in the same tick. Both settlements are handled, so a failing run is a logged `backfill.sweep_failed` record (component `openclaw`, operation `backfill.sweep`, `error_kind`) rather than an unhandled rejection that would take the daemon process down. A malformed `sweep.cron` is logged and treated as not due rather than thrown, so one provider's bad metadata cannot skip the rest of the list. `runtime.js`'s `runTick()` calls `await sweepDriver.tick({now})` directly after the existing sink-driver tick, riding the same `DEFAULT_TICK_INTERVAL_MS` 60-second loop: a `*/5 * * * *` schedule only needs a due-check once a minute, so this opens no second timer to start, drain, and account for at shutdown. Also repairs the typecheck this task's branch point already failed: T7's `sweep: { cron: opts.config?.backfill?.sweep_cron ?? ... }` does not compile, because the plugin's config slice is a `JsonObject` and every step below its root is a `JsonValue`. Read through a `resolveSweepCron` mirroring the `resolveQuiesceMs` helper already sitting beside it; behavior is unchanged. Externally blocked for real capture: until PR #552 (issue #543) merges, the LLP 0158 reader still reads OpenClaw v3 fields flat, so a sweep projects nothing from a real transcript. These tests passing is not evidence that it does. Tests: the due-check fires only sweep-bearing, cron-due contributions and builds the narrowed `BackfillRunnerContext` from the daemon's own runtime fields; a rejected run neither throws out of `tick()` nor lands as an unhandled rejection, and a never-settling run does not block the tick. A separate wiring test boots a real daemon with a fixture plugin whose contribution opts into a sweep and proves the tick actually runs it, which no unit test of the driver can show. Task-Id: T9 * docs/ACCEPTANCE.md: rewrite openclaw_capture for two-lane capture Drops the steering-plugin link/enable setup and the before_model_resolve/hooks.allowConversationAccess version-gate language (the plugin is deleted; Lane A depends on no OpenClaw hook API). Adds a setup step running `hyp attach --client openclaw` followed by the restart instruction it prints, a sweep step (detach to strip the live route, confirm the row is absent, confirm it lands within one sweep interval past the quiesce window), and a zero-duplicate assertion (a turn both lanes observe resolves to exactly one row for its part_id, proven against the daemon's own scheduler rather than a manual `hyp backfill`). Re-confirms LLP 0167#verify-results items 1, 3, and 4 against the current tree's attach/detach behavior instead of assuming them. Drops the retired deferred-provider-family warning-ledger step (LLP 0171 retires R13; no ledger) and the shadow-provider-id failure mode (Lane A overrides the existing anthropic/openai entries, it does not register new ids). States in the section's own Requires that the sweep/dedupe steps need PR #552 merged (the LLP 0158 reader still parses OpenClaw v3 flat), and the client_attach status-row re-confirmation needs PR #553 merged (a now-probed openclaw otherwise falls back to pre-#553 status behavior). This is a doc; the test is a human's successful run against a real OpenClaw install, which this change cannot perform. It is specified against what T2 (detach), T4 (attach), and T5 (manifest) actually implement in this tree, read directly from hypaware-core/plugins-workspace/openclaw/src/attach.js, src/core/config/client_detach_disk.js, and hypaware-core/plugins-workspace/openclaw/hypaware.plugin.json. Task-Id: T13 * Add backfill_openclaw_fixture hermetic smoke for Lane B sweep New backfill_openclaw_fixture.js under hypaware-core/smoke/flows, mirroring backfill_claude_fixture.js / backfill_codex_fixture.js: writes a minimal OpenClaw v3 session JSONL in the nested-message-envelope shape (PR #552's reader) under a temp agents/<id>/sessions/ tree with a controllable mtime, drives the real createBackfillSweepDriver (T9) through a cron-due tick, and asserts (a) a file inside the default 180000ms quiesce window is skipped, (b) a file backdated past it is captured with native message identity, and (c) rerunning the sweep on a later cron-due tick (forcing a fresh devRunId, so the ai-gateway materializer's dedupe genuinely re-scans committed partitions) nets zero new rows for the already-written part_ids. Driving a real, non-dry-run sweep write for the first time (T9's own tests only ever exercised a mocked runBackfill seam) surfaced a latent bug: writeRows/flushDataset read ctx.query, which BackfillRunnerContext never carried and the daemon's createBackfillSweepDriver(...) call never supplied, so any real sweep write actually crashed on "Cannot read properties of undefined (reading 'getDataset')" in both the smoke and the real daemon path. Threaded query through BackfillRunnerContext, BackfillSweepDriverOptions, createBackfillSweepDriver, and the daemon's sweepDriver construction, and updated LLP 0172's field enumeration and ctx samples (Sections 4.3/4.4) to match. Extended the existing T9 unit tests (test/core/daemon-backfill-sweep.test.js) to cover the new required field and its passthrough. Task-Id: T12 * openclaw-backfill.test.js: backdate fixture mtimes to close a quiesce-window race (#570) listSessionFiles compares stat.mtimeMs <= Date.now() even when a test sets config.backfill.quiesce_ms: 0 to opt out of the quiesce gate for something unrelated: 0ms only removes the margin, not the comparison. A fixture written moments earlier could race the provider's own later Date.now() call across two different clocks and occasionally lose, projecting 0 items instead of 1. Confirmed non-deterministic: PR #570's commit 0c62a21 produced both a green and a red `test (24)` CI run from the identical commit. writeSession now backdates every fixture's mtime by a small, fixed margin (FIXTURE_MTIME_MARGIN_MS), comfortably clearing the race while staying far below the real 180000ms default quiesce window, so the tests that rely on genuine freshness against that default are unaffected. The one test that writes its session file outside writeSession (the OPENCLAW_HOME relocation test) gets the same backdate applied directly. * Review round 1: make an attach refusal observable, and three smaller fixes Finding 1 (major). The registered `attach()` wrapper discarded the effect's `OpenclawAttachOutcome`, and both callers infer success from "did it throw", so a refusal recorded a `done` marker whose endpoint and assets_key matched forever: the join never retried even after the user cleared the conflicting `models.providers` entry, while the json_path attach probe kept reporting `not attached`. `hyp attach --client openclaw` printed the refusal and exited 0. LLP 0172 1.3 is authoritative (it promises the `{status:'failed', reason}` outcome is recorded and retried), so the wrapper now rethrows a failed outcome: `perform()`'s catch turns it back into that shape (recorded, warned, retried, the join's other actions untouched) and `runClientLifecycle`'s catch makes it exit 1. Rethrowing at the wrapper rather than teaching `perform()` to parse the adapter payload is what fixes both callers, since the CLI hands the adapter `ctx.stdout` directly and captures nothing to inspect. LLP 0172 1.3 gains the translation step it left implicit; the test that locked in the swallow now asserts the retryable failure, plus the reconciler and exit-code halves. Finding 2. `clientConfigHome` took the first segment of the settings path's home-relative form, which is not the config home when `$OPENCLAW_HOME` is nested inside `$HOME`: the cache glob then matched nothing and the purge silently no-opped while the settings half reported success. Derive it by stripping the manifest's own `settings_file` tail instead, the exact inverse of what `resolveClientSettingsPath` joined on. Regression test uses a two-segment `OPENCLAW_HOME`. Finding 3. `listSessionFiles`'s JSDoc claimed the CLI path runs unfiltered. It does not: `runOpenclawBackfill` computes the quiesce cutoff on every run, and `run()` is the single entrypoint for the CLI, the onboarding finale, and the sweep. Name `plan()` as the only unfiltered caller. No behavior change. Finding 4. The sweep fired a due provider with no record of what was still running, so a pass outliving its cron interval got a second concurrent run against the same datasets and mid-flush spool. Add the `maintenanceInFlight` guard shape, widened to a Set because the driver fires one run per provider, with a `backfill.sweep_skipped` / `already_running` record and clearing on both settlements. LLP 0172 4.4 states the re-entrancy rule it had left out. Co-Authored-By: Claude <noreply@anthropic.com> * Review round 2: thread the plugin config, make attach idempotent, name the sweep's component Three review findings, each with a doc edit in the same commit. 1. `sweep_cron` and `quiesce_ms` were validated then discarded. `activate()` built the backfill contribution without passing `ctx.config`, so both keys this PR adds to `validateBackfillSection` resolved to the hardcoded `*/5 * * * *` / 180000ms defaults at runtime, with no diagnostic. The existing unit tests handed `config` straight to the factory, so the missing wiring was invisible to them; the new test starts from an activation. 2. `attach()` refused on bare key presence, so it was not idempotent over its own output. This PR's manifest `attach_probe` is what makes openclaw eligible for attach-on-join, and `isCurrent()` re-performs attach on an ephemeral-port rebind (LLP 0086) or an asset-set change (LLP 0107). Every re-perform then refused: the marker churned to `failed`, `hyp attach openclaw` exited 1, and `openclaw.json` stayed pinned to the dead port while the marker-header probe still reported `attached: true`. The refusal is now ownership-aware, on the self-identifying triple detach already tests before deleting. `isOwnedProviderEntry`/`ownedBaseUrls` move out of `client_detach_disk.js` into a shared `src/core/config/provider_entry_ownership.js` so the two halves cannot disagree about the same file. Attach passes no base-URL set (on a drift re-attach its own entry carries the old origin); detach still passes one, because there the wrong answer deletes a value HypAware never wrote. Everything that fails the test still refuses, including `null`, a foreign entry, and a hand-edited one that merely kept the header. 3. The generic sweep driver stamped `component: 'openclaw'` on all five of its records while logging as `backfill-sweep`. It fires any contribution carrying a `sweep` field, so a second opt-in would have been misattributed. `component` now names the emitting module; plugin identity already rides `hyp_plugin` and `provider`. Docs updated to match: LLP 0167#attach-detach, LLP 0169's decision bullet and summary, LLP 0171 R2, LLP 0172 sections 1.2 and 2.2, LLP 0173's implementer note on the sweep's telemetry pair. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: test <test@test.com> Co-authored-by: Claude <noreply@anthropic.com>
philcunliffe
added
neutral:stuck
|
What neutral was doing: the Why it cannot proceed The mechanical part is done and verified. Exactly one textual conflict appeared, in The blocker is that this PR adds a new normative section,
All seven other Superseded LLPs in this corpus (0080, 0093, 0094, 0145, 0148, 0149, 0152) carry zero live Is this PR still needed? Yes, and this was verified rather than assumed.
What it needs from you Where should the "status derives attach state by the reconciler's own
Once that is chosen, the rest is mechanical and already scoped: rewrite the section around Secondary finding, on master's own text and not a merge artifact: How to unstick: reply with a comment on this PR (or push to the branch). Neutral monitors this thread and will re-engage with your guidance on its next tick. |
1 participant
Root cause
The attach reconciler and
hyp statusderived "is this client an attach target?" by two different rules.action_attach.desired()skips any client descriptor with noattach_probe(src/core/config/action_attach.js:114) because attach-eligibility requires reverse-capability and only the probe can reverse it. So for a probe-less client (openclaw per LLP 0143, claude-desktop per LLP 0115 / #444 / #445)perform()never runs and no marker is ever written.Three status surfaces derived against the attach contract without that gate, and each read that permanent silence as a permanent negative:
buildClientActionsReportbuiltdeclaredAttachfrom every enabled client descriptor on a joined host, so no-marker + declared +hasCentralresolved topending, forever.{ attached: false }, renderingnot attachedwhere nothing is attachable.client_attach_missingfired onconfigured && !probe.attached, ungated, printinghyp attach --client openclaw- which resolves the adapter's deliberate LLP 0143 no-op and writes no marker either, so running it clears nothing.The doc comment above
buildClientActionsReportalready stated the intended invariant ("pending/n/aare derived for declared targets the reconciler would act on but has not yet"), so this was a missed gate, not a design choice.The fix
Gate all three on
descriptor.attachProbe, the same ruledesired()uses - the wayreadAttachPolicy/readBackfillPolicyalready keep the two sides from disagreeing abouton_join:inert: trueand derivesn/a, joiningon_join: falseand non-joined as the third "the reconciler is a no-op for this target" case. Not dropped from the report - a vanished row is its own wrong answer.ClientAttachReportgains a requiredattachable: boolean. The text surface printsattach n/ainstead ofnot attached;--jsoncarriesattachablebeside the unchangedattachedboolean, so a consumer pinningattacheddoes not break and one that wants to distinguish "no marker" from "no such thing as a marker" reads the new key.client_attach_missingno longer fires for a probe-less client.A probe-less client is unattachable, not unattached. This is deliberately a gate, not a new signal: nothing here reports whether OpenClaw is in fact routing, which stays LLP 0143's open question (a registry-derived attach signal, worth its own LLP).
Docs landed in the same commit
#status-derives-by-the-same-gate(the rule, and theattachablefield), plus consequences covering Claude Desktop and the JSON shape.n/anow also covers "a clientdesired()would never name", with the reasonpendingmust be derived bydesired()'s own rule.#repair-must-be-runnableis amended, not silently contradicted: the rule stands and theconfigure_commandlookup stays, but the warning now has to be one the repair can clear. Desktop loses itsclient_attach_missing, and no signal goes with it - with no probe to read back it fired identically before the consent prompt, after a decline, and after a fully successful install.hyp claude-desktop verifyis the check that can answer that question.Test evidence
New:
test/core/status-probeless-client.test.js(3 tests). A joined host whose central layer enables both@hypaware/openclaw(probe-less) and@hypaware/claude(probed) - claude is the over-suppression guard in every one of them.Before (on the unfixed code) - all 3 fail, for the right reasons
The failure in test 2 is the issue's reported string verbatim.
After - all 3 pass
Rendered status on that same joined host, after
The probed client keeps all three of its states.
backfill @hypaware/openclaw [pending]is correct and untouched: the OpenClaw adapter really does register a backfill provider (LLP 0161 #backfill-provider), so that target resolves.Suite
npm test: 3273 pass, 0 fail, 1 skipped. The skip is the pre-existing zstd-availability guard (resolveEncodeSettings falls back to SNAPPY...), unrelated. No pre-existing failures to report -masteris green here too.npm run typecheck: clean.test/core/llp-ref-hygiene.test.jspasses, so the three new@ref LLP 0143#status-derives-by-the-same-gateannotations resolve to a live anchor.Refs: LLP 0044, LLP 0115, LLP 0139, LLP 0143, #444, #445, PR #510.
Fixes #544