Skip to content

test(gateway): distinguish final CLI results from draft text - #139605

Merged
steipete merged 1 commit into
mainfrom
codex/test-audit-cli-result-precedence
Sep 6, 2026
Merged

test(gateway): distinguish final CLI results from draft text#139605
steipete merged 1 commit into
mainfrom
codex/test-audit-cli-result-precedence

Conversation

@steipete

@steipete steipete commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Related: #139428

What Problem This Solves

The CLI stream-JSON extraction test gave the assistant draft and terminal result identical text, so it could not detect reversed precedence.

Why This Change Was Made

Give the draft distinct text while keeping the existing terminal expected value and case. The extractor is unchanged.

User Impact

The existing test now rejects a regression that returns draft text instead of the terminal CLI result. No new cases or production changes; test LOC is unchanged.

Evidence

A task-local negative control reversing extractor precedence failed this case while the plain payload case still passed. The extractor was restored byte-for-byte. All 18 owner/sibling tests, full changed checks, formatting/diff checks, deslop and independent P2 review pass.

@clawsweeper

clawsweeper Bot commented Sep 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: XS maintainer Maintainer-authored PR labels Sep 6, 2026
@steipete
steipete marked this pull request as ready for review September 6, 2026 01:45
@clawsweeper clawsweeper Bot added P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 6, 2026
@clawsweeper

clawsweeper Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed September 5, 2026, 9:50 PM ET / September 6, 2026, 01:50 UTC.

ClawSweeper review

What this changes

Give the Gateway test’s draft reply different text so it detects an extractor returning the draft instead of the final CLI result.

Merge readiness

✅ Ready for maintainer review

Keep open: this is a useful, focused improvement to regression coverage that remains absent from current main and the latest release. No actionable patch defect was found.

Priority: P3
Reviewed head: a8d22f8f296f0a5e810d72097e2a15e08ccd8481

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, useful test improvement with a credible negative control and no identified correctness or security defect.
Proof confidence 🌊 off-meta tidepool Not applicable: The maintainer-authored PR changes only a test fixture, so the external-contributor runtime-proof gate does not apply; its reported negative control directly targets the strengthened assertion.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The maintainer-authored PR changes only a test fixture, so the external-contributor runtime-proof gate does not apply; its reported negative control directly targets the strengthened assertion.
Evidence reviewed 7 items Verified introduced change: The pinned merge-base-to-head diff changes only the assistant fixture text; the terminal result and expected value remain unchanged. Repository ownership was verified with git remote -v.
Existing extraction contract: The complete helper parses assistant and result records separately and returns resultText before assistantText. Distinct values therefore make reversed precedence observable. This is an OpenClaw test-helper contract; the changed case does not depend on the Codex harness.
Current-main necessity: Fetched main still gives the draft and terminal result identical text, leaving the specific coverage gap addressed by this PR.
Findings None None.
Security None None.

How this fits together

Gateway live tests use a shared helper to extract readable text from agent results, including CLI JSON streams. This change strengthens the helper’s unit coverage without changing runtime behavior.

flowchart LR
  A[Agent result payload] --> B[Shared test extractor]
  B --> C[Read CLI stream records]
  C --> D[Prefer final result over draft]
  D --> E[Gateway live test assertions]
  F[Distinct draft and final fixtures] --> B
Loading

Before merge

None.

Agent review details

Security

None.

PR surface

Tests 0. Total 0 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 1 1 1 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 1 1 0

Review metrics

None.

Root-cause cluster

Relationship: partial_overlap
Canonical: #139428
Summary: This PR improves one retained test within the broader audit campaign and does not complete its runtime or removal targets.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Technical review

Best possible solution:

Retain the existing extraction behavior and make its current test independently distinguish final output from draft output.

Do we have a high-confidence way to reproduce the issue?

Not applicable to a production bug: this is a test-sensitivity improvement. Source inspection establishes the blind spot, and the PR reports a targeted negative control that exposes it.

Is this the best way to solve the issue?

Yes: changing the existing fixture is the narrowest way to enforce final-result precedence without adding cases, support code, or production seams.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against a475d1d629c1.

Labels

Label changes:

  • add P3: This strengthens existing test coverage without changing production behavior.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The maintainer-authored PR changes only a test fixture, so the external-contributor runtime-proof gate does not apply; its reported negative control directly targets the strengthened assertion.

Label justifications:

  • P3: This strengthens existing test coverage without changing production behavior.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The maintainer-authored PR changes only a test fixture, so the external-contributor runtime-proof gate does not apply; its reported negative control directly targets the strengthened assertion.

Evidence

What I checked:

  • Verified introduced change: The pinned merge-base-to-head diff changes only the assistant fixture text; the terminal result and expected value remain unchanged. Repository ownership was verified with git remote -v. (src/gateway/test-helpers.agent-results.test.ts:25, a8d22f8f296f)
  • Existing extraction contract: The complete helper parses assistant and result records separately and returns resultText before assistantText. Distinct values therefore make reversed precedence observable. This is an OpenClaw test-helper contract; the changed case does not depend on the Codex harness. (src/gateway/test-helpers.agent-results.ts:54, a8d22f8f296f)
  • Current-main necessity: Fetched main still gives the draft and terminal result identical text, leaving the specific coverage gap addressed by this PR. (src/gateway/test-helpers.agent-results.test.ts:25, a475d1d629c1)
  • Latest-release comparison: The v2026.9.2 test also uses identical draft and final text; this coverage improvement is not present in the supplied latest release. (src/gateway/test-helpers.agent-results.test.ts:25, 3928bad9badf)
  • Validation and campaign context: The captured PR body reports that reversing extractor precedence failed this case while the plain payload case passed, followed by restoration of the helper and 18 passing owner/sibling tests. Its linked campaign, Test audit: reduce redundant test code and full-suite runtime #139428, explicitly includes improving retained tests. These are contributor-reported results, not executions performed during this review. (a8d22f8f296f)
  • Historical routing: Available file history identifies Peter Steinberger in earlier work on the shared helper, including the Claude CLI live-test repair. The commit metadata and its recorded parent were readable, but missing historical objects prevented exact-line introduction verification; no introducing-author claim is made. (src/gateway/test-helpers.agent-results.ts, e331694df659)

Likely related people:

  • Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete merged commit 11303cf into main Sep 6, 2026
168 checks passed
@steipete
steipete deleted the codex/test-audit-cli-result-precedence branch September 6, 2026 03:00
@steipete

steipete commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

@steipete steipete mentioned this pull request Sep 6, 2026
Open
vincentkoc added a commit that referenced this pull request Sep 6, 2026
@vincentkoc
Merge origin/main into refactor/codex-elicitation-snapshot-boundary
7e92563 
* commit 'a2886546df92c3a601db538b1b70125e3519a16f':
  test(reports): deduplicate native process census setup (#139596)
  fix(ui): avoid refetching unrelated catalog hosts (#139570)
  test(compaction): consolidate classification proof at its owner (#139564)
  fix(apple): harden native state file permissions (#139623)
  improve(skills): review retained conversation for durable learning (#139656)
  fix(docker): stage bundled plugin runtime dependencies under the packaged plugin root (#139117)
  test: close HTTP fixtures before destroying their sockets (#139595)
  refactor(agents): simplify attempt execution and recovery (#139597)
  test(gateway): share HTTP authority scenarios (#139611)
  test(gateway): distinguish final CLI results from draft text (#139605)
  fix(shell-env): preserve CLI terminal ownership during login imports (#139308)
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Sep 6, 2026
@steipete
test(gateway): distinguish final CLI results from draft text (opencla…
e7c81d8 
…w#139605)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

gateway Gateway runtime maintainer Maintainer-authored PR P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@steipete