Skip to content

OpenClaw Lane A live capture: client-header attribution, settlement match keys, Responses-API decoder (LLP 0175/0176) - #586

Merged
bgmcmullen merged 5 commits into
masterfrom
openclaw/lane-a-live-capture-fixes
Aug 4, 2026
Merged

OpenClaw Lane A live capture: client-header attribution, settlement match keys, Responses-API decoder (LLP 0175/0176)#586
bgmcmullen merged 5 commits into
masterfrom
openclaw/lane-a-live-capture-fixes

Conversation

@bgmcmullen

@bgmcmullenbgmcmullen commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

A day of live investigation found OpenClaw's Lane A (gateway live capture) completely non-functional, with three stacked causes, each fixed and re-verified against live traffic:

  1. Attribution (LLP 0175 root cause):attach.js wrote only the x-hypaware-upstream routing marker, never the x-hypaware-client: openclaw header the openclaw exchange projector's match() keys on. Anthropic-shaped exchanges fell through to the Claude projector (misattributed, never settled, and duplicated against the sweep, replaying full conversation history per capture); OpenAI-shaped exchanges matched no projector and vanished silently. Attach now writes both headers; ownership still keys on the marker alone so pre-fix entries detach cleanly and a re-attach upgrades them in place.
  2. Settlement residual (LLP 0175): OpenClaw prepends [Mon 2026-08-03 15:33 PDT] to user messages on the wire but stores bare text in its session file, so user turns content-missed while assistant turns settled. match_key.js now normalizes the wire-only stamp out of text identity, symmetrically on both builders.
  3. Responses-API blindness (LLP 0176): the projector's OpenAI branch spoke only Chat Completions; OpenClaw speaks /v1/responses. A third parse branch decodes Responses exchanges (input items, tool calls/results, reasoning-as-thinking, streamed reconstruction from the terminal response.completed payload, usage netted through the existing gross-minus-cached path). Decoded turns emit the shared block vocabulary so they settle under the same match keys the session file yields, honoring the fix-ordering constraint (settlement first, decoder second). The dispatcher's no_projector_match warn now names path/method/status/is_sse so an undecoded dialect can never again masquerade as an unattached client.

Ships LLP 0175 + 0176 (Draft, round-1 reviews included) recording the full evidence chain.

Verification

  • npm test: 3302/3303 (the 1 failure is pre-existing on master: usage-policy-fold, confirmed identical with these changes stashed)
  • hyp smoke client_attach_idempotent: green
  • Live end-to-end on a real OpenClaw install: Anthropic turn and OpenAI (Responses) turn each captured within seconds, client_name: openclaw, correct provider/model, settled onto the native session file identity, and the subsequent sweep pass added zero duplicate rows.

Out of scope (documented in the LLPs)

  • Subscription/OAuth traffic (hardcoded ChatGPT endpoint, unreachable by config; sweep is the documented ceiling)
  • Error-status exchange recording (LLP 0176 fix 2, unverified)
  • Sweep-side per-session provider stamping on mixed-provider sessions
  • Cache remediation of rows written pre-fix

🤖 Generated with Claude Code

bgmcmullenand others added 3 commits August 3, 2026 16:07
…drop the wire timestamp prefix (LLP 0175)
Live OpenClaw captures were claimed by the Claude projector and never
settled: attach wrote only x-hypaware-upstream while the openclaw
projector's match() keys on x-hypaware-client, so every gateway exchange
misattributed as claude under a fallback session and the sweep re-imported
the same turns natively - unbounded duplication (LLP 0175).
- attach.js writes x-hypaware-client: openclaw alongside the upstream
marker; ownership still keys on the marker alone, so pre-fix entries
detach and a re-attach upgrades them in place.
- match_key.js normalizes the wire-only user-message timestamp prefix
('[Mon 2026-08-03 15:33 PDT] ...') out of text identity, symmetrically
on both builders; without it every user turn content-missed while its
exchange's assistant turns settled (the one residual observed after the
header fix).
Both verified against live traffic: exchanges now attribute openclaw,
settle onto the native session file identity for user and assistant
turns, and the sweep dedupes against them instead of duplicating. LLP
0175 records the evidence, remediation, and round-1 review.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s name their path (LLP 0176)
The gateway proxied /v1/responses traffic faithfully and recorded
nothing: the openclaw projector's OpenAI branch spoke only Chat
Completions, and an exchange no projector decodes vanished with a warn
that named neither path nor status - hours of live OpenClaw OpenAI
traffic read as 'client never attached' (LLP 0176).
- projector.js grows a third parse branch, dispatched per exchange on
path/body shape: request input items (message / function_call /
function_call_output; reasoning replays skipped), instructions plus the
leading system run folded into system_text, response output arrays
(text, tool_use, reasoning-as-thinking), streamed exchanges rebuilt
from the terminal response.completed payload with a finished-items
fallback, and Responses usage re-keyed through the existing Chat
Completions netting path (gross input minus cached read, LLP 0035).
Decoded turns emit the shared block vocabulary, so they produce the
same match keys the session file yields and settle instead of
duplicating (LLP 0175's sequencing constraint, satisfied first).
- message_projector.js's no_projector_match warn now carries path,
method, status_code, and is_sse.
Verified live: the first OpenAI turn ever captured from OpenClaw landed
with openclaw attribution, gpt model and provider intact, settled onto
the native session. LLP 0176 records the evidence and round-1 review.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… any-cast helper
The projector's project() return union (projection | drop | promise)
needs the same awaiting helper the sibling shape suite uses; property
access on the bare union fails tsc.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bgmcmullenbgmcmullen added neutral:fix Delegate this issue to neutral for an autonomous fix attempt (reproduce -> fix -> PR) neutral:adopt Foreign PR adopted into neutral's reconcile scope and removed neutral:fix Delegate this issue to neutral for an autonomous fix attempt (reproduce -> fix -> PR) labels Aug 3, 2026
@philcunliffephilcunliffe added the neutral:adopted Adoption completion record: merged while carrying neutral:adopt (LLP 0031) label Aug 3, 2026
…hat residual is documented
The #root-cause section records the missing x-hypaware-client header only;
the wire-only user-message timestamp prefix and its normalization are fix
direction item 2. Adds the anchor and retargets the three refs.
Co-Authored-By: Claude <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
Contributor

Review: findings (no blockers)

The three claimed areas are real and correctly aimed. isOwnedProviderEntry keys on the marker alone, so pre-fix entries still detach and re-attach upgrades in place, and the new test proves it. The match-key strip is applied symmetrically through reduceBlock, so it cannot create one-sided drift, and it adds no session-side ambiguity because the session file never carries the prefix. Settlement's existing ambiguity guard (buildOpenclawSessionIndex deletes a key two messages share) still protects the collision case. Unmatched settlements are accounted for rather than dropped: plugin.openclaw.settlement logs unmatched and the rows keep fallback identity. x-hypaware-* headers are stripped in forwardHeaders() before forwarding, so the new client header never reaches a provider. Conventions are clean: no em dashes, no semicolons, no inline import() types, no @typedef, no dangling .d.ts specifiers.

Verified beyond the test suite: a function_call_output item and OpenClaw's role: "toolResult" record produce the same match key, in both the string-output and array-output forms.

Pushed (mechanical only)

e44bb7a - retarget the timestamp-prefix @refs. Three annotations pointed at LLP 0175#root-cause, which documents only the missing x-hypaware-client header and says nothing about the wire timestamp prefix (that is Fix direction item 2). Added {#fix-direction} to that heading in LLP 0175 (Draft, so editable) and retargeted match_key.js:100, test/plugins/openclaw-match-key.test.js:53, test/plugins/openclaw-settlement.test.js:360. The attach-side ref at attach.js:43 correctly stays on #root-cause. No behavior change.

Handed back

1. major - openclaw/src/match_key.js:95: the timestamp regex only matches US-style zone abbreviations, so the LLP 0175 residual stays open for most non-US users.

[A-Z]{1,6} matches PDT/EDT/UTC but not the offset-style short names JS emits for most of the world:

America/Los_Angeles PDT -> stripped
Europe/Berlin GMT+2 -> NOT stripped
Asia/Tokyo GMT+9 -> NOT stripped
Asia/Kolkata GMT+5:30 -> NOT stripped

Confirmed: wireMatchKey('user', '[Mon 2026-08-03 15:33 GMT+8] hi') !== wireMatchKey('user', 'hi'). If OpenClaw builds the stamp with timeZoneName: 'short' (which the observed PDT is consistent with), every user turn from a GMT+/-N zone content-misses exactly as LLP 0175 describes, and the fix silently only works where it was tested. Suggested fix: widen the zone group to accept an offset tail, e.g. [A-Z]{1,5}(?:[+-]\d{1,2}(?::\d{2})?)?, and add the GMT+2 / GMT+5:30 cases to the near-miss test table. If you can confirm from the OpenClaw source that the stamp is always an alpha abbreviation, say so in the WIRE_TIMESTAMP_PREFIX doc comment and this drops to a non-issue, but nothing in the comment or tests establishes that today.

2. minor - projector.js:1114-1118: a Responses stream cut before any response.output_item.done emits an empty assistant row and discards the text deltas it did capture.

Reproduced: with response.created then response.output_text.delta: "partial text" and nothing else, sawResponsesEvent is true, doneItems is empty, and the projector emits:

{ "role": "assistant", "content": [], "stop_reason": "error",
"attributes": { "openclaw": { "match_key": "5d008246..." } } }

The docstring says the stream "degrades to the response.output_item.done items that did finish", but with zero such items there is nothing to degrade to and a row is written anyway. Two consequences: the captured response.output_text.delta payloads are thrown away (the Chat Completions sibling stitches its deltas and keeps the partial text), and the content-free row still carries a match key and a message_index, so it is eligible for the ordinal fallback and can acquire a native message_id it has no content for. Minimum fix: if (!terminal && doneItems.length === 0) return undefined. Not pushed, because your LLP text records that "per-delta stitching of a half-finished item is deliberately not attempted", so choosing between "emit nothing" and "stitch the deltas" is your call.

3. minor - projector.js:325-336: openclawSessionId's fallback chain has no Responses branch, so an instruction-less Responses request gets a per-exchange session id.

The chain is systemText -> reqBody.messages[0].content -> exchangeId. A Responses request carries input, never messages, so a request with no instructions and no leading system/developer item falls straight to hashShort(exchangeId):

sid for exchange ex-A = 9607356ea130e4fc
sid for exchange ex-B = e54b5640f0127833 # same conversation, different session

That fragments Lane A session grouping for exactly the traffic this PR just made recordable. In practice OpenClaw always sends instructions, which is why the tests do not hit it, but the fallback exists precisely for when it does not. Suggested fix: add an input-array branch alongside the messages branch.

4. minor - LLP 0176 fix 2 is now more consequential than when it was written. With the client header landing, an error-status exchange in a matched shape no longer falls out at the unmatched-drop layer: it projects the request-side turns and no assistant row, with nothing marking it as failed. Confirmed a 400 Responses exchange writes the user turn and stops. That is the state fix 2 already describes as open, so it is not a regression from this PR, but the PR changes it from "records nothing" to "records a prompt with no reply and no error signal", and a reader of ai_gateway_messages cannot distinguish that from a turn the model never answered. Worth a sentence in LLP 0176 fix 2 noting the changed shape of the gap.

5. minor - the header-absent fallback is still wrong-but-confident, and only the issue doc says so. With no x-hypaware-client, an Anthropic-shaped exchange from any client is claimed by the Claude projector at priority 100 and stamped client_name: claude. LLP 0175 names this as a corollary and parks it in Open questions, which is fair for this PR's scope. Flagged only so it does not get lost: it is the same failure that produced the fbe6c615e2b31ab1 mess, just for the next unrecognized client rather than OpenClaw.

6. minor - test coverage gaps for the failure modes. The Responses tests are strong on the happy path and cover the cut-stream, incomplete-response, near-miss-bracket and mid-text-stamp cases well. Missing: (a) a function_call_output to session-file toolResult match-key test (verified passing by hand, but it is the reconciliation most likely to break silently under a shape change); (b) an unknown or future input item type asserting it projects nothing rather than a malformed turn; (c) a truncated response_body on the /v1/responses path asserting the request turns survive and the assistant is simply absent (also verified working, untested). None blocking.

7. nit - issue docs are still Status: Draft while code @refs them as [implements]. LLP 0175 fixes 1 and 2 and LLP 0176 fixes 1 and 3 are all marked DONE in-document. Other Issue-type docs in the corpus are Active (0095) and Accepted (0110). Consider promoting these to Active, since 0176 fix 2 remains genuinely open.

8. nit - message_projector.js:206 logs the full path, which includes the query string (proxy.js:147 sets path: requestUrl, not parsedUrl.pathname). Harmless for Anthropic and OpenAI, but a TOML-configured upstream that authenticates via a query parameter would put that credential into a warn line. The value is already persisted in the exchange row, so this is not a new exposure class, just a new surface. parsedUrl.pathname carries the same diagnostic value.

Verification

  • npm test: 3302 passed, 1 skipped, 0 failed
  • npm run typecheck: clean
  • Pushed: e44bb7a on openclaw/lane-a-live-capture-fixes

Note on LLP numbering

This PR's llp/0175-* collides in number with a doc on a neutral-owned PR (#585). That is being resolved on the other branch by renumbering it to 0178. This PR keeps 0175 and 0176 unchanged, no action needed here. Flagging it because the duplicate-numbers check validates each PR against master in isolation, so it passes on both and the duplicate would only surface on master after the second merge.

@philcunliffe

Copy link
Copy Markdown
Contributor

Review round 2: findings

The only delta since the round 1 record (535f8cc) is e44bb7a, which neutral itself pushed: three @ref anchor retargets from LLP 0175#root-cause to #fix-direction, plus the {#fix-direction} anchor definition on that heading. Four single-line changes, no behavior change, npm test and npm run typecheck clean.

Every finding in the round 1 record still stands unaddressed at this head. Verdict is unchanged: changes requested, on finding 1.

Blocking ask - openclaw/src/match_key.js:97.WIRE_TIMESTAMP_PREFIX ends its zone group with [A-Z]{1,6}, which matches alpha abbreviations (PDT, EDT, UTC) but not the GMT+N short names JS emits for most non-US zones. The result is that the settlement residual this PR sets out to fix stays open for those users, while the tests (which only exercise PDT) report success. Either widen the group to accept an offset tail and add GMT+2 / GMT+5:30 to the near-miss table, or, if OpenClaw's stamp is provably always an alpha abbreviation, record that in the WIRE_TIMESTAMP_PREFIX doc comment so the narrower pattern is justified rather than incidental.

Findings 2 through 8 are non-blocking and yours to take or leave. Findings 2 and 3 in particular are cheap guards worth considering before this lands.

Reply on this thread or push to the branch, and neutral will re-engage on its next tick.

@philcunliffephilcunliffe added the neutral:changes-requested neutral reviewed an adopted PR and requests changes (non-binding; maintainer decides) label Aug 3, 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.

Requesting changes on one finding; the full review records are in the thread (rounds 1 and 2).

openclaw/src/match_key.js:97 - WIRE_TIMESTAMP_PREFIX's zone group [A-Z]{1,6} matches PDT/EDT/UTC but not the GMT+N short names JS emits for most non-US timezones. So the settlement content-miss this PR fixes stays open for those users, while the tests (which only cover PDT) report success.

Either widen the group to accept an offset tail, e.g. [A-Z]{1,5}(?:[+-]\d{1,2}(?::\d{2})?)?, and add GMT+2 / GMT+5:30 to the near-miss test table; or, if OpenClaw's stamp is provably always an alpha abbreviation, say so in the WIRE_TIMESTAMP_PREFIX doc comment so the narrow pattern is deliberate rather than incidental.

Everything else in the review is non-blocking. The three areas this PR targets all check out otherwise, and e44bb7a (an @ref anchor retarget) was pushed on your branch under neutral:adopt.

…es session-id fallback (PR #586)
Finding 1 (blocking): WIRE_TIMESTAMP_PREFIX now accepts an offset tail
(GMT+2, GMT+5:30) alongside alpha abbreviations, since JS short zone
names are offset-style for most non-US zones; positive and near-miss
cases added to the test tables.
Finding 2: a Responses stream cut with zero finished items emits no
assistant row instead of an empty-content row carrying a match key.
Finding 3: openclawSessionId falls back through the Responses input
array, so an instruction-less Responses request groups by conversation.
Finding 8: the projection-skipped warn logs the pathname, not the full
request URL with query string.
Finding 4: LLP 0176 fix 2 notes the gap's changed shape now that matched
error exchanges record a prompt with no reply and no error signal.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bgmcmullen

Copy link
Copy Markdown
ContributorAuthor

Pushed 106609c addressing the blocking finding plus findings 2, 3, 4, and 8.

  • Finding 1 (blocking): took the widen route. Nothing in the OpenClaw source or LLP 0175 evidence proves the stamp is always an alpha abbreviation (the observed PDT is consistent with timeZoneName: 'short', which emits GMT+N for most non-US zones), so the zone group now accepts an offset tail: [A-Z]{1,6}(?:[+-]\d{1,2}(?::\d{2})?)?. Kept {1,6} rather than narrowing to {1,5} to avoid regressing any stamp the old pattern already accepted. Added GMT+2 / GMT+5:30 / GMT-7 / UTC+10 as positive cases and GMT+123 / GMT+5:3 / GMT+ to the near-miss table, and the doc comment now records why the offset form is in scope.
  • Finding 2: took the emit-nothing option (consistent with the documented no-delta-stitching stance): a cut stream with zero finished items returns no assistant row, with a test asserting the last projected message stays the user turn.
  • Finding 3: openclawSessionId falls back through the input array when messages is absent, with a test proving two exchanges sharing an instruction-less Responses conversation get one session id.
  • Finding 4: added the changed-shape sentence to LLP 0176 fix 2.
  • Finding 8: the aigw.message_projection_skipped warn now logs the pathname only.

Findings 5, 6 (beyond the tests the fixes above added), and 7 are left as-is for follow-up.

npm run typecheck clean; openclaw suites 186/186. npm test shows one failure in test/core/usage-policy-fold.test.js ("fold_tightened only when folding changed the verdict") that is pre-existing: it fails identically at e44bb7a with a clean tree on this machine, so it looks environment-dependent (NFC/NFD fold behavior), not related to this branch.

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

Copy link
Copy Markdown
Contributor

Review round 3: clean

106609c resolves the blocking finding and findings 2, 3, 4 and 8. Each was verified empirically rather than by reading, and every new test was reverted to confirm it actually holds its fix in place. No blockers remain.

Finding 1 (blocking): resolved

Built the stamp the way OpenClaw's observed shape implies (Intl.DateTimeFormat, weekday:'short', ISO date parts, hour12:false, timeZoneName:'short') across a spread of zones and compared wireMatchKey(role, stamped) against wireMatchKey(role, bare):

zoneshort namestrip
America/Los_AngelesPDTPASS
America/New_YorkEDTPASS
Europe/BerlinGMT+2PASS
Europe/LondonGMT+1PASS
Asia/TokyoGMT+9PASS
Asia/ShanghaiGMT+8PASS
Asia/KolkataGMT+5:30PASS
Asia/KathmanduGMT+5:45PASS
Asia/YangonGMT+6:30PASS
Australia/AdelaideGMT+9:30PASS
Pacific/ChathamGMT+12:45PASS
Pacific/MarquesasGMT-9:30PASS
America/St_JohnsGMT-2:30PASS
Pacific/KiritimatiGMT+14PASS
Africa/NairobiGMT+3PASS
UTCUTCPASS

16 of 16. The two-digit-hour-plus-45-minute case (GMT+12:45) and the negative half-hour case (GMT-2:30) both clear the \d{1,2}(?::\d{2})? tail, so the widened group is not merely wide enough for the common cases.

On the locale question: the observed Mon plus 2026-08-03 combination is en-CA shaped (en-US renders 08/03/2026), and en-CA emits PDT for LA and GMT+5:30 for Kolkata with ASCII signs. The fix targets exactly the formatter the evidence points at.

No over-strip. Diffed the old pattern against the new across 20 strings. The only behavior changes are the six real GMT-offset stamps; every near-miss still returns keep, including a 3-digit offset, 1-digit minutes, an empty offset, a 7-char zone, a lowercase weekday, a missing trailing space, and a non-anchored inline occurrence. The change is strictly additive, and you extended the near-miss table correctly.

Finding 2: resolved

if (!sawResponsesEvent || doneItems.length === 0) return undefined (projector.js:1124). The guard is correctly scoped rather than blanket:

terminal response.completed with output: [] -> user | assistant:[] (still records)
cut, one output_item.done -> user | assistant:[text] (degrades)
cut, zero done items, only output_text.delta -> user (no assistant row)

It removes the harmful half (a content-free row acquiring a native message_id through the ordinal fallback). It does not match Chat Completions, which stitches deltas and keeps partial text, but that asymmetry is deliberate and the docstring now states it. LLP 0176 says nothing about stitching or cut streams, so there is no doc/code mismatch.

Finding 3: resolved (array form)

input=[{role:'user',content:'first turn'}] ex-A -> 1b647f991595be7e
input=[first, assistant reply, second turn] ex-B -> 1b647f991595be7e stable as the conversation grows
input=[{role:'user',content:'different convo'}] ex-C -> 0dd257d7def2e4ef distinct

Finding 4: resolved, and the doc is accurate

Verified rather than trusted. A 400-status Responses exchange in a matched shape projects one user turn, no assistant row, and no error signal anywhere, exactly as the added sentence says.

Finding 8: resolved

(stringValue(input.path) ?? '').split('?')[0] is equivalent to parsedUrl.pathname for this purpose.

Revert tests

fix revertedtestresult
zone group narrowed back to [A-Z]{1,6}the timestamp strip accepts GMT-offset short zone namesfails (not ok 7)
|| doneItems.length === 0 removeda stream cut with zero finished items emits no assistant rowfails (not ok 9)
input-array branch removedopenclawSessionId keys on the first input item ...fails (not ok 9)

All three load-bearing. None passes both ways.

Regressions: none

attach.js, proxy.js and settle.js are untouched. Re-verified anyway: the x-hypaware-* strip at proxy.js:385; the settlement ambiguity guard at settle.js:391-392; unmatched settlements still counted at settle.js:220; and function_call_output still reconciling with role: "toolResult" (both hash to 21f5138c5b823b19, in both the string-output and array-output wire forms).

Residuals (minor, not asks, neither introduced here)

A. The empty-assistant-row fix is Responses-only; both siblings still emit it. Reproduced on Chat Completions (cut stream, role-only chunk) and Anthropic (cut stream, message_start only): both write assistant [] with stop=error and match key 5d008246, byte-identical to the Responses row round 1 quoted. All three paths converge on one canonical empty-assistant key, and two still write it with a live message_index, so the ordinal-fallback exposure you closed for Responses remains open for the other two. Predates this PR, so not a regression, but the fix is now asymmetric. Tracked separately rather than expanding this PR's scope.

B. The input branch covers the array form only; a string input still fragments.openaiResponsesMessages supports input: 'hi' (projector.js:846), and your own new cut-stream fixture uses that form, but openclawSessionId does not: two exchanges with { input: 'hi there' } yield 9607356ea130e4fc and e54b5640f0127833. Left as a hand-back rather than a push, because grouping a conversation that opens as a string and continues as an array requires the string form to hash the same way the object branch's JSON.stringify(messages[0].content) does, and picking that is your call.

Conventions

Clean across all seven touched files. No em dashes, no semicolons, no inline import() types, no @typedef, no dangling .d.ts specifiers. The delta adds no @ref annotations; LLP 0175#fix-direction and LLP 0176#fix-direction both still resolve.

Verification

  • npm test: 3305 passed, 1 skipped, 0 failed
  • npm run typecheck: clean
  • Pushed: nothing. Everything landed correct; the two residuals are design calls, not mechanical fixes.

You took the harder route on finding 1, widening the pattern rather than asserting the narrow form was safe, and it was the right one. All four findings are genuinely closed, and the tests hold them closed.

@philcunliffephilcunliffe added the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Aug 4, 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.

Approving. All four findings from the earlier rounds are resolved, and I verified each empirically rather than by reading the diff: 16 of 16 real timezone stamps now strip (including the 45-minute and negative half-hour offsets), and all three new tests were confirmed to fail when their fix is reverted, so they are load-bearing.

Widening the zone pattern rather than asserting the narrow form was safe was the right call.

Two residuals are noted in the review record as observations, not asks: the empty-assistant-row guard is Responses-only while Chat Completions and Anthropic still emit the same row shape (pre-existing, tracked separately), and openclawSessionId's new branch covers the array input form but not the string form.

Merging is the maintainer's call; neutral holds here.

@philcunliffe

Copy link
Copy Markdown
Contributor

Merge note: one line to change if this lands after #592

Not a review finding and nothing to fix here. Flagging a cross-PR interaction discovered while reviewing #592 ("a cut stream with no content emits no assistant row").

#592 adds a shared floor at the projector's row-assembly point that drops an assistant row when a stream was cut and captured no content. Its first implementation keyed cut-ness on stop_reason === 'error'; review found that conflates the synthetic marker with a value real upstreams send (OpenRouter-class gateways emit finish_reason: "error"), silently dropping genuine terminal error rows. It now carries cut-ness out of band in a WeakSet, populated only by markCutStream(message).

The interaction: this PR's responsesAssistantFromStream writes partial.stop_reason = 'error' as a bare literal. Under the original string-comparison floor the Responses path would have inherited the drop automatically; under identity-based cut-ness it does not.

Concretely: on the merged tree, a cut Responses stream whose one finished item is of a type openaiResponsesAssistant does not map (e.g. image_generation_call) emits a content-free row with a live message_index, which is the ordinal-fallback identity hazard #591 is about. Your own doneItems.length === 0 guard already covers the main case, so this is a narrow residual.

The fix, at merge time: replace partial.stop_reason = 'error' with markCutStream(partial). That is why markCutStream exists as a function rather than two statements, and its docstring says so.

Verified: the two branches merge cleanly (ort, no conflicts) and 194/194 openclaw tests pass on the merged tree. A terminal response.completed whose output really is [] is preserved either way, because openaiResponsesAssistant gives it no stop_reason at all.

Merge order does not matter; whichever lands second wants this one-line edit. This PR's own approval stands unchanged.

philcunliffe pushed a commit that referenced this pull request Aug 4, 2026
Round-2 review follow-up. The floor's safety rests on an unenforced
convention: `markCutStream` is the only thing allowed to write
`CUT_STREAM_STOP_REASON`, because the recorded value and the WeakSet
membership that actually decides the drop are halves of one fact. The
two wire shapes that exist are covered by behavior tests (verified by
mutation: introducing a copy between mark and check, or stamping the
literal directly, fails them). A wire shape added later is not, and
PR #586's Responses reconstruction is exactly that case waiting to
happen: it writes `partial.stop_reason = 'error'` as a bare literal and
has to become `markCutStream(partial)` at merge time.
Adds a source lint, the same shape as `house-style-em-dash.test.js`,
that fails when the marker is stamped anywhere but inside
`markCutStream`, and points `markCutStream`'s docstring at it.
No behavior change.
Co-Authored-By: Claude <noreply@anthropic.com>
@bgmcmullen
bgmcmullen merged commit ad1af38 into masterAug 4, 2026
9 checks passed
@bgmcmullen
bgmcmullen deleted the openclaw/lane-a-live-capture-fixes branch August 4, 2026 04:11
philcunliffe pushed a commit that referenced this pull request Aug 4, 2026
#586 landed its Responses reconstruction stamping `partial.stop_reason =
'error'` as a bare literal, which is the merge obligation recorded on that
PR's thread. Merging master into this branch reproduced it: the lint added
in cd6c8b3 fails with
not ok 18 - the cut-stream marker is only ever stamped through markCutStream
GitHub reported the merge CLEAN, because the conflict is semantic rather
than textual: no rung sees it and nothing but the lint would have caught it
before it reached master.
Routing the Responses path through markCutStream also closes the residual
round 3 identified: a cut Responses stream whose finished items produce no
mapped content emitted a content-free row with a live message_index, which
#586's own doneItems.length === 0 guard does not cover.
Fixes#591
philcunliffe added a commit that referenced this pull request Aug 7, 2026
…#592)
* A cut stream with no content emits no assistant row on any wire shape (#591)
The Anthropic and Chat Completions stream reconstructions both mark a
stream that ended without its terminal event `stop_reason = 'error'` and
return whatever arrived. When nothing arrived, that was still a row:
`content: []` with a live `message_index` and no native `message_id`, so
it stayed eligible for settlement's ordinal/time fallback and could
acquire a `message_id` belonging to another turn. Every content-free
assistant row also hashes to one canonical match key (5d008246)
regardless of which decoder built it, so empty rows could collide across
wire shapes too.
The floor now sits at the one place a row is assembled, the
projectedMessages loop in `project()`, so both shapes (and any added
later) inherit it. The condition is "cut AND empty", keyed on the
synthetic cut marker now named `CUT_STREAM_STOP_REASON`: a response that
reached its terminal event and genuinely produced nothing keeps its row,
and Chat Completions' delta stitching is untouched, so a cut stream that
did carry partial text still records it.
Co-Authored-By: Claude <noreply@anthropic.com>
* Cut-stream floor keys on object identity, not the emitted stop reason (#591)
`isEmptyCutRow` decided cut-ness by comparing the row's `stop_reason` to
the literal `'error'`, on the premise that neither API sends `error` as a
real stop reason. That premise does not cover the traffic this projector
actually reads. An unrecognized upstream is parsed as the Anthropic wire
by design, and an OpenAI-compatible endpoint reached through a config
upstream can send `finish_reason: "error"`; a terminal response that said
`error` and produced nothing is a real answer, and the row was the only
record it happened. Reproduced on all three shapes: a terminal Chat
Completions chunk with `finish_reason: 'error'`, a non-streamed body with
the same, and an Anthropic `message_delta` + `message_stop` carrying
`stop_reason: 'error'` all lost their assistant row.
The same premise covered request history: an Anthropic history turn that
replays `stop_reason: 'error'` on an empty assistant message was dropped
too, which the role test alone does not prevent.
Cut-ness is now carried out of band, in a module-level `WeakSet` the two
reconstructions add to on the branch that stamps the marker. A wire body
is parsed JSON, so it can forge any field value but never membership in
that set, which makes "cut AND empty" true by construction rather than by
an assumption about upstream stop-reason vocabularies. The emitted
`stop_reason` is unchanged, and every cut-and-empty case the fix already
dropped still drops.
Co-Authored-By: Claude <noreply@anthropic.com>
* Gate the cut-stream marker on a single stamping point (#591)
Round-2 review follow-up. The floor's safety rests on an unenforced
convention: `markCutStream` is the only thing allowed to write
`CUT_STREAM_STOP_REASON`, because the recorded value and the WeakSet
membership that actually decides the drop are halves of one fact. The
two wire shapes that exist are covered by behavior tests (verified by
mutation: introducing a copy between mark and check, or stamping the
literal directly, fails them). A wire shape added later is not, and
PR #586's Responses reconstruction is exactly that case waiting to
happen: it writes `partial.stop_reason = 'error'` as a bare literal and
has to become `markCutStream(partial)` at merge time.
Adds a source lint, the same shape as `house-style-em-dash.test.js`,
that fails when the marker is stamped anywhere but inside
`markCutStream`, and points `markCutStream`'s docstring at it.
No behavior change.
Co-Authored-By: Claude <noreply@anthropic.com>
* isEmptyCutRow's docstring names the wrong reason history is safe (#591)
The comment said request-history messages are "excluded twice over, by
the set and by the assistant-role test". Only the set excludes them. A
replayed history turn can carry role: 'assistant' with an empty content
array, which is precisely the claim round 1 of this PR made and round 2
falsified by execution, so leaving the sentence standing invites the
same mistake back. Comment only.
Co-Authored-By: Claude <noreply@anthropic.com>
* Merge master and settle the Responses cut marker through markCutStream
#586 landed its Responses reconstruction stamping `partial.stop_reason =
'error'` as a bare literal, which is the merge obligation recorded on that
PR's thread. Merging master into this branch reproduced it: the lint added
in cd6c8b3 fails with
not ok 18 - the cut-stream marker is only ever stamped through markCutStream
GitHub reported the merge CLEAN, because the conflict is semantic rather
than textual: no rung sees it and nothing but the lint would have caught it
before it reached master.
Routing the Responses path through markCutStream also closes the residual
round 3 identified: a cut Responses stream whose finished items produce no
mapped content emitted a content-free row with a live message_index, which
#586's own doneItems.length === 0 guard does not cover.
Fixes#591
---------
Co-authored-by: test <test@test.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: test <test@example.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