Skip to content

test(gateway): retain saved source in reload fixtures - #140453

Merged
steipete merged 1 commit into
mainfrom
codex/gateway-reload-fixture-followup
Sep 6, 2026
Merged

test(gateway): retain saved source in reload fixtures#140453
steipete merged 1 commit into
mainfrom
codex/gateway-reload-fixture-followup

Conversation

@steipete

@steipete steipete commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Two remaining Gateway reload fixtures discarded saved config fields when constructing unrelated logging/UI updates. On current main, the config writer rejected both updates for removing roughly half the saved config, so the tests stopped before completing their SecretRef-auth and Control UI origin checks.

Read the current saved source before each update and retain existing logging/UI fields. The SecretRef test still rotates runtime credentials, authenticates the new client, and verifies that the original file token remains persisted. The origin test still checks seeded origins and logging overrides before and after reset. All existing assertions and timeouts remain unchanged.

Follow-up to #140433. Validation on pinned main b9a80868: both original cases reproduce CONFIG_WRITE_REJECTED; the repaired cases and four auth-reload siblings pass together, six real Gateway cases total, using this worktree's fresh canonical E2E build. Production LOC delta: 0; test LOC delta: +4.

The complete changed-file gate and independent P2 review pass.

@steipete steipete self-assigned this Sep 6, 2026
@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
@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 6, 2026, 5:23 PM ET / 21:23 UTC.

ClawSweeper review

What this changes

The PR preserves saved configuration during Gateway reload tests so logging and UI edits can reach the existing credential and origin assertions.

Merge readiness

✅ Ready for maintainer review

This remains a useful, focused fixture repair absent from the inspected current main. No introduced correctness defects were found; the merged sibling repair explicitly left these cases for follow-up.

Priority: P3
Reviewed head: 47ce669dbec84a151a3b48e095d7aee40a701a9d

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A narrow, coherent fixture repair preserves meaningful assertions and follows the existing source-preserving pattern.
Proof confidence 🌊 off-meta tidepool Not applicable: The maintainer-authored fixture repair is exempt from contributor proof requirements; its body reports six real Gateway cases covering credential rotation, origin retention, and logging overrides, which were not independently rerun here.
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 fixture repair is exempt from contributor proof requirements; its body reports six real Gateway cases covering credential rotation, origin retention, and logging overrides, which were not independently rerun here.
Evidence reviewed 7 items Verified introduced patch: The pinned merge-base-to-head diff changes only fixture inputs; existing assertions, timeouts, server lifecycle, and production code remain unchanged.
Still necessary on inspected main: Comparing the affected file against fetched main shows all four proposed source-preserving edits remain absent there.
Config rejection mechanism: The writer flags saved configurations of at least 512 bytes when the replacement falls below half their size; size drops block writes unless explicitly allowed. The patch preserves fields instead of bypassing that guard.
Findings None None.
Security None None.

How this fits together

Gateway reload tests write configuration and exercise a running Gateway through its config and WebSocket interfaces. Saved source values must remain separate from runtime credentials, logging overrides, and automatically seeded browser origins.

flowchart TD
  A[Saved configuration] --> B[Read source snapshot]
  B --> C[Apply logging or UI edit]
  C --> D[Config write safety checks]
  D --> E[Gateway reload and runtime overrides]
  E --> F[Credential and origin assertions]
Loading

Before merge

None.

Agent review details

Security

None.

PR surface

Tests +4. Total +4 across 1 file.

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

Review metrics

None.

Technical review

Best possible solution:

Retain saved source fields in these fixtures while preserving the config writer’s safety checks and the existing runtime-overlay assertions.

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

Yes, the affected fixtures and size-drop rejection path provide a clear source-backed reproduction route, corroborated by the author’s before/after report; this review did not execute it.

Is this the best way to solve the issue?

Yes. Reading saved source immediately before each edit matches the merged sibling repair and avoids weakening the config guard or persisting runtime-only values.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • add P3: This repairs test fixture construction without changing shipped Gateway 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 fixture repair is exempt from contributor proof requirements; its body reports six real Gateway cases covering credential rotation, origin retention, and logging overrides, which were not independently rerun here.

Label justifications:

  • P3: This repairs test fixture construction without changing shipped Gateway 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 fixture repair is exempt from contributor proof requirements; its body reports six real Gateway cases covering credential rotation, origin retention, and logging overrides, which were not independently rerun here.

Evidence

What I checked:

  • Verified introduced patch: The pinned merge-base-to-head diff changes only fixture inputs; existing assertions, timeouts, server lifecycle, and production code remain unchanged. (src/gateway/gateway.test.ts:513, 47ce669dbec8)
  • Still necessary on inspected main: Comparing the affected file against fetched main shows all four proposed source-preserving edits remain absent there. (src/gateway/gateway.test.ts:513, c1fbb4622ee2)
  • Config rejection mechanism: The writer flags saved configurations of at least 512 bytes when the replacement falls below half their size; size drops block writes unless explicitly allowed. The patch preserves fields instead of bypassing that guard. (src/config/io.write-safety.ts:160, 47ce669dbec8)
  • Source and runtime remain distinct: Snapshot creation separately retains sourceConfig and materialized runtimeConfig. Reading sourceConfig does not copy the runtime credential override or seeded origins into the fixture's next write. (src/config/io.snapshot.ts:345, 47ce669dbec8)
  • Origin assertion remains meaningful: Startup seeds browser origins only in runtime configuration, and the reload overlay restores them when absent from source. Preserving saved source therefore does not turn the origin assertion into a persisted-value self-check. (src/gateway/server-startup-bootstrap.ts:379, 47ce669dbec8)
  • Related merged work and routing history: Provided GitHub metadata identifies test(gateway): preserve saved config in auth reload fixture #140433 as merged at b9a8086. Its body explicitly defers these two fixtures. Local path history lists Peter Steinberger for that repair and several earlier Gateway test changes; deeper line-introduction verification was unavailable because required historical objects could not be read. (src/gateway/gateway.test.ts:275, b9a808683919)

Likely related people:

  • steipete: 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 c2941a8 into main Sep 6, 2026
153 checks passed
@steipete
steipete deleted the codex/gateway-reload-fixture-followup branch September 6, 2026 21:28
@steipete

steipete commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

@steipete steipete mentioned this pull request Sep 6, 2026
Closed
@github-actions github-actions Bot mentioned this pull request Sep 6, 2026
Open
vincentkoc added a commit that referenced this pull request Sep 6, 2026
@vincentkoc
chore: merge main into cloud worker IP fix
d9895d4 
* commit '294df333c58f2da104eccffc623ccb3c4d43d4d8': (1802 commits)
  fix: clear stale diagnostics after Gateway changes (#140426)
  fix: keep plugin catalogs intact after discovery timeouts (#140441)
  perf(tui): prepare session picker rows once per overlay (#140462)
  refactor(config): reuse environment substitution for write-back (#140460)
  refactor(discord): remove retired auto-presence templates (#140463)
  fix(ui): finish roster refreshes during session activity (#140459)
  fix(setup): match prepared credentials to the model provider (#140461)
  fix: keep MiniMax discovery on the selected credential (#140438)
  refactor(tests): remove unused parent context cache reset (#140456)
  fix: avoid repeated xAI OAuth refresh during discovery (#140434)
  perf(routing): reuse prepared event session targets (#140454)
  fix(media): keep deferred audio and extraction providers discoverable (#139231)
  refactor(codex): remove redundant app-server forwarding (#140451)
  fix(test): avoid false native polling timeouts (#140119)
  test(gateway): retain saved source in reload fixtures (#140453)
  fix(gateway): reduce repeated model work when listing sessions (#140414)
  fix(feishu): fall back to post mode over table limit (#136382)
  fix: use the bundle-compatible Proxyline runtime under Bun (#140411)
  refactor(trace): reuse the canonical usage counter type (#140444)
  refactor(node-host): share execution policy result fields (#140446)
  ...
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Sep 7, 2026
@steipete
test(gateway): retain saved source in reload fixtures (openclaw#140453)
ccb7408
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

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