Skip to content

OpenClaw backfill: CLI-backend denylist replaces the provider allowlist, rows carry per-turn provider (LLP 0193/0194) - #643

Merged
bgmcmullen merged 3 commits into
masterfrom
openclaw-cli-backend-denylist
Aug 6, 2026
Merged

OpenClaw backfill: CLI-backend denylist replaces the provider allowlist, rows carry per-turn provider (LLP 0193/0194)#643
bgmcmullen merged 3 commits into
masterfrom
openclaw-cli-backend-denylist

Conversation

@bgmcmullen

@bgmcmullenbgmcmullen commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Closes the Lane B half of #640.

What

  • LLP 0193: Lane B's projection gate flips from the {anthropic, openai} vendor allowlist to a two-rung CLI-backend denylist. A record is excluded when its effective backend stamps api: "cli" (the mechanism marker, verified live on OpenClaw 2026.7.1-2) or its provider matches a sibling-adapter prefix (claude-cli, codex); an unresolvable backend still fails closed as unknown. Everything else projects, so ollama and any future direct-API provider are captured at transcript fidelity from birth, with no HypAware release per vendor. The (provider, api) pair smears as a unit so a neighbor's provider is never stitched to a different neighbor's api. SIBLING_ADAPTER_COVERAGE becomes load-bearing for the exclusion (was labeling only); PROJECTABLE_PROVIDERS is deleted.
  • LLP 0194: AiGatewayProjectedMessage gains an optional per-message provider; the gateway row builder prefers it over the exchange value (the LLP 0026 model precedence extended to the column it skipped); the OpenClaw backfill stamps every row from its turn's smeared backend, prompts and tool results included. Mixed-provider sessions stop folding minority turns into the first projected turn's vendor. Projectors that never set the field (all live capture) are byte-for-byte unchanged.
  • LLP 0161 gets the Extended-by forward-ref its living-list section called for; stale allowlist glosses in session_file.js / projector.js updated.

Why

An OpenClaw turn on the ollama provider was recorded nowhere: the allowlist silently excluded it from the sweep while LLP 0167's coverage statement claimed it covered ("every OpenClaw turn is captured at least at transcript fidelity"). The allowlist direction was chosen when the CLI-backend provider strings were unverifiable (LLP 0161 s10); live verification has now expired that ignorance. The accepted fail-open residual (a future sibling-captured mechanism stamped neither cli nor a known prefix) is recorded in LLP 0193.

Verification

  • npm test: green (one pre-existing usage-policy-fold failure, also fails on clean master)
  • npm run typecheck: clean
  • Smokes: backfill_openclaw_fixture, backfill_openclaw_session_shape, gateway_claude_capture all ok
  • New tests: unknown vendor with wire-shape api projects; ollama in its exact live stamping projects with per-row provider asserted through materialization; api: "cli" excludes an unrecognized provider with no covered_by; real claude-cli+cli stamping excludes with covered_by: claude_transcript; gateway-level row-by-row provider override and fallback
  • Live end-to-end on a real install: a native-Ollama turn ("Who is the president of Mexico?") recorded by the scheduled sweep 8.5 minutes after the turn, rows stamped provider=ollama, model=gemma4:12b; the all-claude-cli session still projects nothing
  • 154/154 @ref annotations validate

Still open in #640

Lane A (live gateway) ollama capture (NDJSON native dialect or the openai-compat reroute experiment), and the one-turn probe confirming a codex-backend turn stamps api: "cli".

🤖 Generated with Claude Code

…st, rows carry per-turn provider (LLP 0193/0194)
Lane B's projection gate flips from the {anthropic, openai} vendor
allowlist to a two-rung denylist: a record is excluded when its effective
backend stamps api "cli" (the mechanism marker verified live on OpenClaw
2026.7.1-2) or its provider matches a sibling-adapter prefix; a record
whose backend is unresolvable still fails closed as unknown. Everything
else projects, so ollama and future direct-API providers land at
transcript fidelity from birth (the LLP 0167 coverage statement, now true
as written). The (provider, api) pair smears as a unit so a neighbor's
provider is never stitched to a different neighbor's api.
Mixed-provider sessions also stop reading as their first projected turn's
vendor: AiGatewayProjectedMessage gains an optional per-message provider,
the row builder prefers it over the exchange value (the LLP 0026 model
precedence extended to the column it skipped), and the OpenClaw backfill
stamps every row from its turn's smeared backend, prompts included.
Verified end-to-end on a live install: a native-Ollama turn recorded by
the scheduled sweep within the interval, rows stamped provider=ollama,
model=gemma4:12b; all-CLI sessions still project nothing.
Lane A (live gateway) ollama capture remains open in #640.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bgmcmullenbgmcmullen added the neutral:adopt Foreign PR adopted into neutral's reconcile scope label Aug 5, 2026
@philcunliffephilcunliffe added the neutral:adopted Adoption completion record: merged while carrying neutral:adopt (LLP 0031) label Aug 5, 2026
@philcunliffe

Copy link
Copy Markdown
Contributor

Neutral review round 1 of 2 (adopted PR, neutral:adopt) - 9ebba5e

Reviewed across four lenses: exclusion correctness and double-capture, the per-message provider contract, LLP and @ref hygiene, and test rigor. CI green, MERGEABLE/CLEAN. Verdict: one HIGH, several MEDIUM, changes requested.

The design work here is strong and the doc trail is unusually good. The problem is not the denylist idea, it is that flipping fail-closed to fail-open exposed a latent weakness in the smear, which the allowlist was masking.


First, a correction in your favour

"one pre-existing usage-policy-fold failure, also fails on clean master"

It does not fail on either. Measured on this Linux host:

runresult
usage-policy-fold at clean master 7116f9523 pass / 0 fail
usage-policy-fold at 9ebba5e23 pass / 0 fail
full npm test at 9ebba5e3578 pass / 0 fail / 1 skip, exit 0

The suite is completely green. That file exercises case-insensitive volume folding, so the failure you saw is almost certainly macOS-specific. Your verification section undersells the PR. tsc --noEmit exit 0 and all three smokes exit 0, both confirmed.

Verified sound

  • "Projectors that never set the field are byte-for-byte unchanged" is TRUE, and I went looking hard for the falsy-override hole. message_projector.js:712 uses stringValue(ctx.message.provider) ?? ctx.projection.provider, and stringValue collapses undefined, null, '' and non-strings alike to undefined. Exhaustive grep finds exactly one site in the non-test codebase that assigns provider onto a projected message, and it is yours. All five construction paths build messages field-by-field rather than spreading wire bodies, so no vendor payload can leak a provider key in.
  • The kernel type change is purely additive, not breaking: an optional property on an interface plugins construct.
  • Unresolvable-fails-closed holds.partitionByBackend:414 requires backend?.provider !== undefined; no path lets an unresolvable backend through.
  • The (provider, api) pair really does smear as a unit. I could not construct a case that stitches one neighbour's provider to another's api. That claim is true.
  • The LLP 0161 edit is legitimate under the immutability rule: a pure Extended-by: forward-ref, no body text touched.
  • @ref hygiene is clean. All 9 refs added or moved resolve, and every gloss is accurate about both the code and the cited section. llp-ref-hygiene passes 10/10. Zero stale PROJECTABLE_PROVIDERS references in code, tests, docs or smokes. No em dashes anywhere in the diff.
  • Partitioning, dedupe and settlement are provably unaffected: partition.js:234 reads provider only as the third fallback behind client_name and conversation_source, both of which OpenClaw sets.

HIGH - a sibling-owned record projects when its turn's anchor record is missing

backfill.js:452-470. Double-capture is reachable today, with the current verified stamping. This is a regression, not a pre-existing hazard.

The smear preference is "nearest following anchor, else nearest preceding". A record is an anchor iff it states provider or api. There is no notion of a turn boundary, so a CLI turn whose assistant record is absent from the list has no anchor of its own and inherits the next turn's backend.

Demonstrated. Session in append order:

u1 role:user <- prompt routed to the claude-cli backend
e1 type:"error" <- CLI child exited nonzero; not type "message",
so parseOpenclawSessionMessage drops it entirely
u2 role:user
a2 assistant provider:"ollama" api:"ollama"

Result: projected: [[u1,'ollama'], [u2,'ollama'], [a2,'ollama']], excluded events: [].

u1 is the prompt of a claude-cli turn. Per LLP 0147 the claude child already wrote it to ~/.claude/projects/<slug>/<uuid>.jsonl, so @hypaware/claude captures it under client_name='claude'. It now also lands under client_name='openclaw', and per LLP 0193's own Context the two adapters mint disjoint identities, so part_id dedupe cannot collapse them. No excluded_backend event fires, so the duplicate is invisible in telemetry, which is exactly the "visible, retroactively recoverable" property the allowlist was chosen for.

Under the allowlist the smeared value ollama was not projectable, so u1 was excluded. The flip widens the misattribution target from {anthropic, openai} to every non-CLI provider, which is unbounded.

The mirror case is silent data loss: an errored ollama turn's prompt smears onto a following claude-cli turn, and three records are dropped as covered_by: claude_transcript when the Claude transcript does not contain them.

Suggested fix: make the smear turn-scoped. At minimum, do not let an unanchored record cross a role:'user' boundary in either direction; a record that ends a turn with no following anchor should resolve to unknown (excluded) rather than borrowing the next turn's pair.

This also makes one claim false today.backfill.js:381 and LLP 0193 state that the turns the sibling adapters own "stay out and never double-count". Whichever way you fix the code, that sentence needs to narrow to what it delivers.

MEDIUM - the import window truncates records before backends are resolved

backfill.js:311-312. Two reviewers hit this independently from different directions, which is why it is listed separately from the HIGH even though it is the same family.

constwindowed=filterByWindow(records,window)const{ projectable, excludedByProvider }=partitionByBackend(windowed)

filterByWindow is per record, so it can delete the only anchor of a turn while keeping the rest. With --until falling between a prompt and its assistant, the prompt backward-smears to the previous turn's backend and projects, duplicating a row the Claude transcript owns, again with zero exclusion events. filterByWindow also keeps timestamp-less records unconditionally, producing the same orphaning with no window boundary crossed.

LLP 0194's own Consequences note that backfill dedupe skips existing part_ids, so a later full-window sweep will not correct it. The misattribution is permanent short of purge-and-reimport.

@hypaware/hermes already solved this shape by making --since select which sessions to import rather than which of a session's messages (hermes/src/backfill.js:41-46, "identity is always computed from a session's full row set, never a partial window").

Suggested fix: resolve effectiveBackends over the full records array, then window the partition result. Two-line reorder, removes the class entirely.

MEDIUM - prefix matching has no delimiter boundary

backfill.js:483. lowered.startsWith('codex') matches codexcloud, codex-proxy, codex-router. OpenClaw providers are user-authored ids in openclaw.json, so a direct-API provider whose id happens to start with codex is silently excluded and labelled covered_by: 'codex_sessions_rollout', a false claim that another route holds the data.

Fix:lowered === prefix || lowered.startsWith(prefix + '-') || lowered.startsWith(prefix + '/'), matching the claude-cli/* model-ref form LLP 0147 documents.

MEDIUM - LLP 0172 and 0173 are now false and got no forward-ref

llp/0172:710-714 (Status: Active, and it has no Extended-by: line at all) asserts about the current tree: "backfill.js's PROJECTABLE_PROVIDERS = new Set(['anthropic', 'openai']) and effectiveProviders()'s forward/backward fill are untouched". Both are deleted here. llp/0173:222,399 repeat it.

You applied the forward-ref discipline correctly to 0161; this is the same mechanical edit, allowed on Active docs, and it is the only stale-doc consequence of the deletion.

MEDIUM - LLP 0193 undersells the codex half of the residual

llp/0193:114-120 calls the fail-open residual "speculative" and says "the two-rung key requires both conventions to change before it bites". But lines 61-64 of the same doc admit codex stamping is unverified, and 51-58 establish that OpenClaw stamps the wire shape rather than cli in a sibling file kind. A codex-backed turn stamped provider:"openai" + api:"openai-responses" clears both rungs and double-counts against @hypaware/codextoday, with no convention change.

(It double-counted under the allowlist too, so not a regression, but "speculative" is the wrong word for a path the doc itself flags as unverified.) Point the bullet at {#verify}.

MEDIUM - two coverage holes, both proven by mutation

Mutation testing against the real provider: 4 of 5 core mutations caught (removing either rung, making unresolvable project, ignoring per-message provider). Two survivors:

  1. The (provider, api) pairing invariant is completely untested. Replacing the paired smear with two independent per-field smears leaves the entire suite green. Not a no-op mutation: on a session of [user, assistant(provider:'ollama', no api), user, assistant(provider:'claude-cli', api:'cli')] the head projects two ollama rows, while the mutant projects zero and mislabels the exclusion as an ollama exclusion. That is the silent-drop class the JSDoc calls load-bearing.
  2. The codex entry in SIBLING_ADAPTER_COVERAGE has zero tests.grep -n codex test/plugins/openclaw-backfill.test.js returns nothing; deleting the entry leaves the suite green. Under the allowlist it only affected an event label; under the denylist it is load-bearing for exclusion, so its coverage should have gone up in this PR.

LOW

  • backfill.js:439 - whitespace flips the gate open.api: "cli " projects, no exclusion event. Case is folded, whitespace is not. Same for " claude-cli". Add .trim().
  • backfill.js:455 - a record stating api but no usable provider becomes a blocking anchor. It is correctly excluded as unknown, but it also shadows the real anchor behind it for every preceding unanchored record, so a session with one blank-provider assistant loses that record and the preceding prompt where master projected both. Fail-closed direction, but silent data loss introduced here. Anchor only on records that state provider, carrying that record's own api along; the pair stays atomic either way.
  • backfill.js:439 - rung 1 excludes CLI backends nothing covers. A future OpenClaw CLI backend with no HypAware adapter is dropped with no covered_by and nothing else capturing it, which is the LLP 0167 coverage-gap failure this PR exists to fix, re-created on the other axis. The direction is defensible; make it loud. Emit the coverage-less exclusion at warn, not info.
  • backfill.js:223 - stale comment. Says effectiveProviders/partitionByBackend; this PR renames it to effectiveBackends.
  • Prefix and case normalisation untested.startsWith to === survives, and dropping either .toLowerCase() survives. One prefixed-but-not-equal provider and one api:'CLI' assertion close all three.
  • openclaw-backfill.test.js:647 - the comment states a precondition the code does not enforce: "provided its records carry a wire-shape api". api is not required; {provider:'some-future-vendor'} with no api at all projects. That is the most dangerous fail-open cell of the new denylist, and it is both untested and mis-documented. Correct the comment and pin the no-api case deliberately.
  • llp/0193:121-124 overstates test stability. "the suite's existing R10 assertions hold without edits" - the fixtures are indeed unedited, but the R10 block header was reworded and one test rewritten. Narrow to "the claude-cli fixtures' assertions hold".
  • The {#verify} codex probe has no durable home.docs/ACCEPTANCE.md's openclaw_capture is untouched, so the pre-acceptance obligation lives only in an LLP verify list nobody runs.
  • LLP 0194 should record the semantic reservation of the provider name for out-of-tree projectors. A third-party adapter that spreads an OpenRouter-style body (which carries a top-level provider) previously had it ignored and now has it silently win on every row. No in-repo projector does this, so it is a doc and release-note item, not a code one.
  • The "154/154 @ref annotations validate" figure is not reproducible from any obvious scope (repo-wide is 1944; the changed files total 72). Nothing is broken by it, but it should not be quoted as repo-wide validation.

What I did not do, and why

Neutral can push to this branch and healing an adopted PR is normally its job. I have deliberately not pushed fixes here, because the HIGH and the two MEDIUMs above (backfill.js:311, :452, :455) are all one design question, how the smear should be scoped to a turn, and the answer is yours to make. It also needs LLP 0193's "never double-count" claim amended to match whatever you choose, and that is your design record, not neutral's to rewrite.

If you would rather neutral took the mechanical subset off your plate, say so on this thread and it will push them: the delimiter boundary, the .trim(), the stale comment, the LLP 0172/0173 forward-refs, and the four missing tests. The turn-scoping fix and the LLP 0193 wording should stay with you either way.

@philcunliffephilcunliffe added the neutral:changes-requested neutral reviewed an adopted PR and requests changes (non-binding; maintainer decides) label Aug 5, 2026

@philcunliffephilcunliffe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested at 9ebba5e. Full review record with evidence is in the review-record comment on this thread.

Blocking: one HIGH.backfill.js:452-470 - a sibling-owned record projects when its turn's anchor record is missing from the list, because the smear has no notion of a turn boundary and borrows the next turn's (provider, api) pair. Demonstrated: a claude-cli prompt whose assistant record was dropped (a type:"error" CLI abort) projects under ollama, duplicating a row @hypaware/claude already owns under a disjoint identity that part_id dedupe cannot collapse, and no excluded_backend event fires, so the duplicate is invisible. Under the allowlist that smeared value was not projectable; the flip widens the target from {anthropic, openai} to every non-CLI provider.

The mirror case is silent data loss, and the same root cause drives the MEDIUM at backfill.js:311 (the window truncates records before backends resolve, which two reviewers found independently) and the LOW at :455.

Also needing your call: backfill.js:381 and LLP 0193 currently state that sibling-owned turns "never double-count", which is not true today whichever way the code is fixed.

Not blocking, but worth doing in the same pass: prefix matching has no delimiter boundary (codex matches codexcloud and mislabels it covered_by), llp/0172/0173 are now false and want the same mechanical Extended-by: you correctly gave 0161, LLP 0193 undersells the codex residual, and mutation testing found two live coverage holes (the (provider, api) pairing invariant and the whole codex denylist entry both survive deletion with the suite green).

In your favour: the usage-policy-fold failure you flagged as pre-existing does not reproduce on either tree. The full suite is 3578 pass / 0 fail at your head, and on clean master too. Your verification section undersells the PR.

The byte-for-byte-unchanged claim for live capture verifies rigorously, the kernel type change is purely additive, unresolvable-fails-closed holds, the (provider, api) pair genuinely does smear as a unit, the LLP 0161 forward-ref is legitimate, and all 9 new @refs resolve with accurate glosses. The design is sound; it is the smear's turn-scoping that needs the fix.

philcunliffe pushed a commit that referenced this pull request Aug 6, 2026
PR #643 (branch openclaw-cli-backend-denylist) is open and older, and it
already claims both 0193 and 0194. Master's highest is 0192, so scanning
only master made 0193 look free. The minting rule takes the next number
past the highest across the default branch and every branch, so this PR
yields and moves to 0195, the first number free everywhere.
Mechanical: the file, its title line, and the four @refs that point at
it. No prose or decision content changes.
Co-Authored-By: Claude <noreply@anthropic.com>
…r not at all (review fixes for LLP 0193)
The borrow in effectiveBackends ran file-wide, so a turn whose anchor
record was missing (CLI abort, crash mid-write, turn still in flight)
inherited the NEXT turn's (provider, api) pair: a sibling-owned prompt
projected as a duplicate row part_id dedupe cannot collapse, with no
excluded_backend event, and the mirror case silently dropped direct-API
records. Resolution is now fenced at turn boundaries (every user record
opens a turn; forward preferred, backward within the turn as fallback);
a turn that states nothing resolves to unknown, excluded visibly, and
since exclusion writes nothing an in-flight turn imports intact one
sweep later.
Also from the review: resolution happens BEFORE the date window so a
--since/--until cut cannot sever a record from its turn's anchor
(partitionByBackend now takes the window and filterByWindow is unused
here); the sibling prefix match is delimiter-bounded (codex matches
codex/codex-*, not codexcloud); the "never double-count" overclaim is
rewritten in code and LLP 0193 to what is actually guaranteed; LLP 0193
records the codex-stamping residual honestly plus the queued-prompt
fail-closed residual; LLP 0172/0173 carry Extended-by forward-refs.
Six new tests pin the behavior, including killers for the two mutants
that survived the first cut (the codex denylist entry and the
pair-as-a-unit smearing invariant); the codex-entry mutant was verified
to fail the suite before restore.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bgmcmullen

Copy link
Copy Markdown
ContributorAuthor

Review fixes pushed in 1a22e26, all four blocking/needs-a-call items addressed:

  • HIGH (cross-turn borrow): resolution is now fenced at turn boundaries. Every user record opens a turn (including one adjacent to another, since the reader drops a CLI abort's non-message line and merging consecutive prompts would recreate the demonstrated failure); within the fence the borrow is forward-then-backward; a turn that states nothing resolves to unknown, excluded through the existing excluded_backend path, so the failure is visible instead of borrowed. Exclusion writes nothing, so an in-flight turn imports intact one sweep later, while the old behavior wrote a misattributed row whose native part_id pinned the error permanently.
  • MEDIUM (window truncation): resolution runs before the date window; partitionByBackend takes the window and applies it per record (same keep-if-unstamped rule as filterByWindow), so a --since/--until cut bounds what projects but can never sever a record from its turn's anchor.
  • "Never double-count" overclaim: rewritten in the code comment and LLP 0193 to the statement that actually holds: attributable sibling-owned turns are excluded; unattributable turns are excluded rather than guessed about; the fail-open residual for a future mechanism stamped neither way remains and is documented, now including the honest note that codex stamping is unverified so codex coverage rests on the prefix rung alone until the probe runs.
  • Prefix boundary: delimiter-bounded (codex, codex-mini; codexcloud neither denied nor mislabeled), with a test.

Also done: Extended-by: LLP 0193 on LLP 0172/0173; a documented fail-closed residual for queued prompts (unresolvable positionally against the abort case; the #659 trajectory join is the clean fix); six new tests including killers for both surviving mutants, and the codex-entry mutant was re-run to confirm it now fails the suite.

On usage-policy-fold: it fails identically on clean master in my environment and passed in yours on both trees, so it appears environment-sensitive rather than tree-dependent; happy to file it separately.

…exclusions, probe home (LLP 0193/0194)
- isCliBackend and siblingCoverageFor trim before folding, so "cli "
and " claude-cli" no longer slip a gate their trimmed forms close.
- Only provider-stating records anchor the borrow: a record stating api
alone keeps its own pair (excluded as unknown) but neither feeds the
borrow nor shadows the turn's real anchor behind it, so a
blank-provider assistant no longer drops the preceding prompt with it.
- A CLI-denied exclusion with no covered_by logs at warn: dropped here
and recorded nowhere else is the LLP 0167 coverage-gap failure on the
other axis, and it should be loud. unknown stays info (routinely a
turn in flight, re-evaluated every sweep).
- Stale effectiveProviders comment updated.
- Tests: codex-mini pins prefix (not equality) semantics; " CLI" and
" claude-cli" pin both normalizations; the api-only shadow case is
pinned; the no-api provider-only cell, the denylist's widest fail-open
cell, is pinned deliberately and its test comment corrected (the gate
never required an api).
- LLP 0193 narrows the fixtures-stability claim and names the probe's
durable home; docs/ACCEPTANCE.md openclaw_capture gains step 8, the
codex-backend stamping probe, with the fail-open interpretation of a
wire-shape result spelled out. LLP 0194 records that the provider
message-field name is now semantically reserved for out-of-tree
projectors that spread wire bodies (release-note obligation).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bgmcmullen

Copy link
Copy Markdown
ContributorAuthor

LOW pass pushed in cbf96a2; every remaining item from the review record is addressed:

  • Whitespace: both rungs trim before folding (" CLI" denied, " claude-cli" covered), pinned by a test that kills dropping either the trim or the case fold.
  • api-only blocking anchor: only provider-stating records anchor the borrow now; an api-only record keeps its own pair (excluded unknown) but neither feeds nor shadows, so the blank-provider-assistant case projects the preceding prompt again. Test pins it.
  • Coverage-less CLI exclusions log at warn (unknown stays info: routinely a turn in flight, re-evaluated next sweep), with the rationale in a comment.
  • Stale effectiveProviders comment fixed.
  • Prefix/case mutants: codex-mini pins prefix-not-equality; the normalization test pins both .toLowerCase() calls and both trims.
  • The no-api cell: pinned deliberately ({provider, no api} projects) and the test comment corrected, it wrongly claimed a wire-shape api was required.
  • LLP 0193: fixtures-stability claim narrowed to the claude-cli fixtures; verify section names the probe's durable home.
  • Probe home: docs/ACCEPTANCE.mdopenclaw_capture gains step 8, the codex stamping probe, including what a wire-shape result means (the fail-open residual observed live) and the required SIBLING_ADAPTER_COVERAGE follow-up.
  • LLP 0194: records the semantic reservation of the provider message-field name for out-of-tree projectors that spread wire bodies.
  • Ref-count scoping: the earlier figure was scoped to the files touched; this round's is stated as such: 84/84 across the openclaw plugin src, the backfill test file, ACCEPTANCE.md, and LLPs 0193/0194.

Full suite at cbf96a2: 3584 pass / 1 fail, the failure being usage-policy-fold, which also fails on clean master in this macOS environment and passed on both trees in yours; per your note it exercises case-insensitive volume folding, which fits a case-insensitive-filesystem sensitivity. Filed nothing yet; happy to open a small issue for it if you agree it's worth tracking.

@bgmcmullenbgmcmullen removed the neutral:changes-requested neutral reviewed an adopted PR and requests changes (non-binding; maintainer decides) label Aug 6, 2026
@philcunliffe

Copy link
Copy Markdown
Contributor

Neutral review round 2 of 2 (adopted PR, neutral:adopt) - cbf96a2 - verdict: clean

The commit title undersells the work.1a22e26, which round 1 never saw, is a full
turn-scoping redesign that fixes the HIGH and both structural MEDIUMs; cbf96a2 is the
LOW pass on top. Every round-1 finding is addressed in code, tests, or docs. Nothing
blocking remains.

Round 1 declined to push fixes because the HIGH and two MEDIUMs were one design question
(how the smear should be scoped to a turn) that was yours to answer. You answered it, and
the answer is a segmented turn fence rather than the ad-hoc guard that was suggested.

Round-1 finding disposition

#FindingStatusEvidence
HIGHSibling-owned record projects when its turn's anchor is missingfixedbackfill.js:502-517 segments at every role:'user'; fillSegment (:535-548) is fenced to [start,end). Round 1's exact demonstrated session now yields projected:[u2,a2], excluded:[['unknown',1]], pinned at openclaw-backfill.test.js:783-802; mirror case :809-834; trailing in-flight prompt :838-857
MEDImport window truncates before backends resolvefixedfilterByWindow import dropped; partitionByBackend(records, window) (:313, :432) resolves over the full list and applies the window inline at :442-446, byte-equivalent to scan_util.js:65-73 including keep-if-unstamped. Pinned :857-877
MEDPrefix matching has no delimiter boundaryfixedsiblingCoverageFor:560-567 requires rest === '' || /^[^a-z0-9]/ - broader than the suggested -// fix, correctly so. Pinned :900-925 (codex, codex-mini denied; codexcloud projects and is not mislabeled)
MEDLLP 0172/0173 made false, no forward-reffixedBoth gain **Extended-by:** LLP 0193, matching corpus convention
MEDLLP 0193 undersells the codex residualfixedllp/0193:128-136 drops "speculative", states the codex stamping is UNVERIFIED, points at {#verify}, names the consequence
MEDCoverage hole: (provider, api) pair invariantfixedTest :878-897. Mutation-verified: an independent-per-field smear now fails 2 tests (round 1's version survived)
MEDCoverage hole: codex denylist entryfixedDeleting the entry now fails 1 test
LOWx8 (whitespace gate, anchor shadowing, loud coverage-less exclusion, stale comment, normalization tests, test-comment precondition, LLP test-stability claim, {#verify} home, LLP 0194 field reservation)all fixed.trim() at :476/:561; fillSegment anchors only on records[i].provider !== undefined (:944-966); :322 warn/info split; :219-222; :900-943; :661-672; docs/ACCEPTANCE.mdopenclaw_capture step 8

Independent correctness checks

  • part_id is <message_id>#<part_index> (:597-599, :664), not positional, so the
    new transient unknown exclusion cannot shift identity. The "in-flight turn imports
    intact one sweep later" claim holds and re-import will not duplicate.
  • The turn fence does not chop agentic loops. OpenClaw spells tool output
    role:"toolResult", not "user" (match_key.js:26,233, settle.js:565,
    backfill.js:702). Had it been "user", every tool result would have opened a turn.
    The tests use the correct spelling. This was the main way the redesign could have gone
    wrong, and it does not.
  • provider/api/role parse through nonBlankString, so the bare !== undefined
    anchor test cannot be tripped by '' or a non-string.

Mutation testing (24 mutants against the 39-test suite)

Killed: turn-fence removal (3 fails), merging consecutive user records, anchor-on-any-stated
in the backward pass, both .trim()s, api case fold, delimiter boundary, startsWith===,
codex entry, claude-cli entry (6), window-before-resolution, either denylist rung,
unresolvable-projects (3), per-message provider stamping, forward-borrow removal (19),
independent-field smear (2).

Two survivors, both non-blocking preferences, neither a defect:

  1. backfill.js:545 - anchoring the forward pass on effective[i] !== undefined rather
    than records[i].provider !== undefined survives. Not a no-op: on
    [user, assistant(ollama), assistant(api-only), toolResult] the head resolves the
    trailing record to ollama, the mutant to unknown. The backward pass has the
    symmetric guard and is covered; only the forward direction is unpinned. Fail-closed
    either way and the shape is exotic. One appended record on the :944 fixture closes it.
  2. backfill.js:322 - the warn/info split is unpinned; the tests assert events, not levels.

Fresh pass on the new commits

No new findings. No em dashes, no semicolons introduced, all new @refs resolve
(LLP 0193#decision, 0194#decision, 0147) with glosses accurate to both the code and
the cited section.

Worth flagging for whoever merges, as a note rather than a finding: LLP 0193 :137-141
records a genuine new fail-closed residual - a queued prompt (two user records
answered by one assistant) now loses the earlier prompt as unknown, where master would
have projected it. It is visible (excluded_backend, counted in records_excluded),
unverified as a real OpenClaw shape, and has a named clean fix (#659). Documenting it
rather than guessing is the right call.

Verification - and an important caveat about CI

  • node scripts/run-tests.js3586 pass / 0 fail / 1 skipped, exit 0.
  • npx tsc -p tsconfig.json --noEmit → exit 0.
  • Smokes backfill_openclaw_fixture, backfill_openclaw_session_shape,
    gateway_codex_capture → all exit 0.
  • Run twice independently, in clean isolated worktrees at cbf96a2.

GitHub CI has not validated this head. All three workflow runs for cbf96a2 are stuck
queued with zero jobs dispatched; the status-check rollup at this head is empty. The
earlier runs that showed red were cancelled, not failed - steps: [], all four jobs
terminated at the identical second 15m02s in, against a workflow timeout-minutes: 5 that
never engaged. That is a runner-queue cancellation, not a signal about your code, and no
code change was made or requested on account of it. Neutral re-queued those runs.

So the local runs above are currently the only execution evidence for this head. This
review verdict covers code correctness, not CI. Treat CI as still owed before merge.

@philcunliffephilcunliffe added the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Aug 6, 2026
@philcunliffe

Copy link
Copy Markdown
Contributor

Verdict: approved - held for a maintainer

Round 2 came back clean at cbf96a2. Every round-1 finding is resolved, including the
HIGH; the turn-scoping redesign in 1a22e26 is a better answer than the guard round 1
suggested, and mutation testing (24 mutants) found only two unpinned preferences, neither
a defect. Details in the review record above.

neutral:approved is set. Neutral does not merge or ready a contributor's PR - that
call is the maintainer's.

Two things to carry into that decision:

  1. CI has still not validated this head. All three runs for cbf96a2 are stuck
    queued with zero jobs and the check rollup is empty. The earlier red was
    cancelled, not failed (no steps ran; a runner-queue cancellation, not your code).
    Neutral re-queued the runs. Local runs are the only execution evidence so far:
    3586 pass / 0 fail, typecheck clean, three smokes green, twice in clean worktrees.
    This approval covers code correctness, not CI.
  2. LLP 0193 :137-141 documents a new fail-closed residual (a queued prompt loses the
    earlier prompt as unknown), visible in excluded_backend and with a named fix in
    OpenClaw backfill: trajectory-verified turn attribution (join *.trajectory.jsonl when present) #659. Documented rather than guessed, which is the right call, but it is a behaviour
    change worth knowing at merge time.

@bgmcmullen
bgmcmullen merged commit 19ef78c into masterAug 6, 2026
1 of 9 checks passed
@bgmcmullen
bgmcmullen deleted the openclaw-cli-backend-denylist branch August 6, 2026 19:44
philcunliffe added a commit that referenced this pull request Aug 10, 2026
…93 records why the gateway idles (#653) (#658)
* An upstream-less ai-gateway source idles instead of throwing (#649)
A picker run selecting `hermes` and nothing else composed
`@hypaware/ai-gateway` with `{ upstreams: [] }` and then failed at source
start with "ai-gateway: at least one upstream must be configured before
start", so a reachable first-run choice produced a broken install.
The gateway plugin does two separable jobs: at activation it contributes
the `ai_gateway_messages` dataset and the shared
`ai_gateway.projected_exchange` materializer, and at source start it runs
the proxy. `@hypaware/hermes` wants the first alone. It is "never
modified, configured, or proxied" (LLP 0119) so it contributes no
`gateway_upstream`, yet the materializer is a hard `requires.plugins`
dependency (LLP 0120) so its picker row must still compose the gateway
plugin.
So the source now treats an empty compiled routing table as idle: no
listener bound, `state.listen` left unset (`localEndpoint()` keeps
throwing rather than handing an attach a URL nothing is listening on),
`status()` reporting `listening: false` with a reason. A `reload()` that
brings an upstream binds normally, so idling is recoverable.
Co-Authored-By: Claude <noreply@anthropic.com>
* An idle gateway that was configured with upstreams is visible in status (#650 review)
Letting an upstream-less `ai-gateway` source idle rather than fail its
start is right for the config that wants it (hermes composes the plugin
for its materializer alone, LLP 0120), but the same path swallowed a
real misconfiguration. A gateway-only install whose TOML says `url =`
where `base_url` was meant has every entry dropped by `compileUpstreams`
without complaint, and `diagnoseV1Config`'s `gateway_missing_*_upstream`
check does not fire for that shape. On master that install read
`[failed]` / `degraded`; on this branch it read `[started]` / `healthy`
while the user's client got ECONNREFUSED, with the reason living only in
`status.message`, which the daemon snapshot never captures.
`hyp status` now emits a non-degrading `gateway_idle_no_upstreams`
warning when the gateway reports `listening: false` and a non-empty
`details.upstreams`. That field is the raw configured names, pre-compile,
so it discriminates exactly: a hermes-only install publishes `[]` and
stays quiet and healthy, a dropped or misspelled upstream publishes
`['anthropic']` and warns, naming the entry and the field that drops it.
Also: `aigw.idle_no_upstreams` logs at `warn` with the configured count
and names when the config listed upstreams and compiled to none, staying
at `info` for the genuine hermes-only case; and `status()` reads the
context `reload()` handed it rather than the boot-time closure, so
`details.upstreams` describes the config in force (pre-existing, but the
new diagnostic reads that field).
Tests: the diagnostic fires for the dropped-upstream shape and not for
the hermes-only one, and never degrades `overall`; a reload that removes
every upstream tears a live listener down and idles without error, which
ends capture for already-attached clients and is now pinned deliberately.
Co-Authored-By: Claude <noreply@anthropic.com>
* An idle gateway whose upstreams have no usable name is visible too (#650 review 2)
The `gateway_idle_no_upstreams` warning gated on `details.upstreams`, the raw
configured names, so it could not see the config it most needed to see.
`compileUpstreams` drops an entry for a missing `name` exactly as silently as
for a missing `base_url`, and a nameless entry contributes no name at all, so
`provider = "anthropic", base_url = "..."` published `upstreams: []` and read
as hermes-only: `healthy`, no diagnostic, ECONNREFUSED at the client. The
repo's own `diagnoseV1Config` affirms that config, because it matches an
upstream by its `provider` field.
The gateway source already computed the count (its idle log is a `warn` on the
strength of it); it just never published it. It now does, as
`details.upstreams_configured`, and core gates on the count instead of the
names. The names still ride along and still make the message concrete when
they exist; a status file from a build that predates the count falls back to
counting the names, so an older daemon's dropped `base_url` stays visible.
Hermes-only (0 configured) stays quiet and healthy, and the diagnostic remains
non-degrading.
The repair lines pointed at `hyp config validate`, which prints `config ok` and
exits 0 for exactly this config: `@hypaware/ai-gateway` registers no config
section, so nothing checks upstream shape. Replaced with the edit that fixes
it, naming the config file and both required keys, followed by the restart
that makes it take effect.
Co-Authored-By: Claude <noreply@anthropic.com>
* A partially dropped gateway upstream is visible in status (#653)
`compileUpstreams` drops an upstream entry missing `name` or `base_url`,
per entry and silently. #650 made the all-dropped case visible; a partial
loss stayed invisible, because the proxy binds, `listening` is never set,
and the diagnostic only fired when it was `false`. A user who typo'd one
of two upstreams got a working gateway that routed nothing for that
provider.
The gateway source now publishes `upstreams_dropped` (raw configured
entries minus compiled routes) and the names it can attribute, and warns
at boot when the routing table is smaller than the config asked for.
`hyp status` reads the difference: one comparison, two mutually
exclusive kinds - `gateway_idle_no_upstreams` when nothing survived,
`gateway_upstreams_dropped` when some did. Both non-degrading, following
the `gateway_port_fallback` precedent.
Also mints LLP 0193, recording the "idle rather than throw" decision
#650 established (which lived only in `proxy.js` and a PR thread) and
its accepted visibility tradeoff.
Co-Authored-By: Claude <noreply@anthropic.com>
* Renumber the gateway-idle decision from LLP 0193 to 0195 (#658)
PR #643 (branch openclaw-cli-backend-denylist) is open and older, and it
already claims both 0193 and 0194. Master's highest is 0192, so scanning
only master made 0193 look free. The minting rule takes the next number
past the highest across the default branch and every branch, so this PR
yields and moves to 0195, the first number free everywhere.
Mechanical: the file, its title line, and the four @refs that point at
it. No prose or decision content changes.
Co-Authored-By: Claude <noreply@anthropic.com>
* Fix gateway_upstreams_dropped message overclaim, pluralization, dupe names
The non-idle status message claimed traffic for a dropped upstream is
"not proxied and nothing is captured" unconditionally, but
mergeUpstreams backfills a registered adapter preset (e.g. @hypaware/claude's
`anthropic`) whenever its name is not already in the compiled config table,
so a typo'd override can still route through the preset's default endpoint.
Reword to hedge on that case, and fix the message's hardcoded plural to match
the idle branch's configured/upstream(s) treatment (reachable at
dropped === configured === 1 exactly via the preset-backfill path).
Also dedupe readConfiguredUpstreams' droppedNames, which could print a
name twice when two same-named config entries both fail to compile, and
split the LLP 0195 @ref on gateway_upstreams_dropped so the "does not
flip overall" half cites #consequences instead of #visible-when-unintended,
which never says that.
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: test <test@example.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: test <test@test.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

neutral:adoptForeign PR adopted into neutral's reconcile scopeneutral:adoptedAdoption completion record: merged while carrying neutral:adopt (LLP 0031)neutral:approvedneutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@bgmcmullen@philcunliffe