Skip to content

test(credential-sources): close mutation gaps in refresh, gateway db migration and adopt - #2023

Merged
freeqaz merged 3 commits into
mainfrom
test-gaps-32
Oct 10, 2026
Merged

freeqaz merged 3 commits into
mainfrom
test-gaps-32

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 10, 2026

Copy link
Copy Markdown
Collaborator

Problem

A mutation sweep over four recent changes left 34 of 73 hand-written mutants alive on main (6ae2814). The changes were:

Each surviving mutant is a behavior that no test pins, for example:

  • A failed static gateway update records no failure audit, or records the refresh reason code.
  • A refresh update goes ahead after the gateway lost the source.
  • A first mint that is still pending counts as minted.
  • A mint with an unknown outcome deletes the record instead of keeping it for DELETE.
  • Reads and deletes of static sources call the refresh role.
  • The 70 s registration fence boundary is not pinned.
  • OpenShell's catalog could drop issuerRotated from refresh_token.
  • Doctor spawn errors and signal exits are not named.
  • A flipped Kubernetes migration guard died only by 600 s timeouts.
  • Adopt compared only the API Deployment's images.

Change

Tests only, all in existing files and helpers:

  • credential-source-occ
    • Static update failures are audited (CREDENTIAL_GATEWAY_UPDATE_FAILED), including when the gateway throws.
    • A refresh update after the gateway lost the source is refused before any refresh call, with no failure event.
    • A static-type rotate refusal records no failure event.
    • A failure event that cannot be written keeps the mint error.
    • Static sources get no refresh status and no removeRefresh.
    • A gateway-absent refresh source is read without a refresh call.
    • A pending first mint fails registration and removes the record. An unknown mint keeps the record deleting until DELETE.
    • A deletion that wins against a refresh registration also removes the refresh state.
    • The registration fence holds at 69.999 s and opens at 70 s. The fixture gained advanceClock.
    • Registration request IDs use the name-based (version 5) layout.
    • The refresh role is checked before any Secret read.
    • registerDriver refuses a refresh Driver that lacks one of its four operations.
    • An OpenShell Backend refuses an empty credential_refresh ID and requires the selected refresh Driver to be a member.
  • sandbox-driver-startup: only oauth2-refresh-token.refresh_token is issuer-rotated in the OpenShell catalog.
  • runtime-gateway-state-migration
    • An unreadable or missing agent database is left to OpenClaw.
    • A Doctor spawn error is named error-ENOENT, and a Doctor killed by a signal is named by that signal.
    • SIGINT is forwarded to Doctor as SIGINT.
    • The Kubernetes guard check now includes its length > 0 condition.
    • A Docker case fails the re-read after Doctor and expects exit 1 without starting OpenClaw.
  • split-layout-adopt: only the worker is upgraded, so apply refuses and names the worker. After a rollback, apply continues, and the API and worker images still differ.

Verification

Risks

Low. Test-only. The fence boundary case pins the documented 70 s constant (2 x the 30 s gateway timeout + 10 s), so changing that constant now requires a test update. The Kubernetes guard check is a source-text check, like the existing one it extends.

…cases

A mutation sweep over #2016, #1749 (OCC side), #1986 and #1987 left 34 of
73 hand-written mutants alive on main. These cases kill 29 of them:

- credential sources: failure audits for static updates, a gateway that lost
  a refresh source, a static-type rotate refusal and an unwritable failure
  event; refresh status only for ready refresh sources; static deletes skip
  removeRefresh; pending and unknown first mints; deletion racing a refresh
  registration; the 70 s registration fence boundary; name-based registration
  request IDs; the refresh capability contract and Backend membership.
- OpenShell catalog: only oauth2-refresh-token.refresh_token is issuer-rotated.
- Gateway state migration helper: unreadable databases, Doctor spawn errors,
  signal exits and SIGINT.
- split-layout adopt: each writer is compared with its own recorded images.
…ocker unexpected-error exit

The shared-helper refactor on main moved the #1986 migration step into
runtime-startup.ts. Re-run against it, a flipped Kubernetes guard (length >= 0)
died only by 600 s timeouts in kubernetes-compute and plugin-compute, and the
Docker program's exit on an unexpected migration error survived. The include
check now covers the guard condition, and a Docker case fails the post-Doctor
re-read outside the per-database check.
Add a positive control before the incomplete refresh Driver refusals, tie each
delete call to its source, assert the racing deletion deferred on the fence,
and reword three comments.
@freeqaz
freeqaz requested a review from a team as a code owner October 10, 2026 01:54
@clawsweeper

clawsweeper Bot commented Oct 10, 2026 •

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.

@freeqaz
freeqaz merged commit d8eb03e into main Oct 10, 2026
19 checks passed
@freeqaz
freeqaz deleted the test-gaps-32 branch October 10, 2026 01:58
@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Oct 10, 2026
@clawsweeper

clawsweeper Bot commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR strengthens existing tests for credential refresh, Gateway database migration, and split-layout adoption without changing production code.

Example: A credential registration times out, leaving a record that needs cleanup.

  • Before: The test exercised deletion after advancing the clock by 71 seconds.
  • After: The test verifies that deletion remains deferred at 69.999 seconds and completes at 70 seconds.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Useful, focused coverage with no identified defect; confidence rests on source inspection, controlled harnesses, and reported mutation results while CI remains incomplete.
Proof confidence 🦐 gold shrimp (3/6) The repository state identifies a member-authored test-only PR; it changes no shipped behavior, and its harness results do not establish live OpenShell, Doctor, or Kubernetes execution.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Test only · Worth it: Yes · Fix scope: Complete
User problem: Regressions in credential cleanup, failure auditing, startup migration, and adoption safeguards could escape existing coverage.
Reason: The patch protects existing behavior without introducing new product choices or production seams.

Merge readiness

✅ Ready for maintainer review

Keep this PR open for maintainer triage: it adds useful coverage for existing contracts, and no actionable introduced correctness or security defect was found.

Priority: P3
Reviewed head: fd4c0e6b8870e537250b7ab991fab43f5bd7bfa5

Before merge

None.

Findings

None.

Agent review details

How this fits together

The tests exercise OCC credential lifecycle and audit decisions, generated Gateway startup programs, and the adoption script using controlled Driver, process, and Kubernetes fixtures.

flowchart TD
  T[Regression tests] --> O[OCC controller]
  O --> C[Credential Drivers]
  O --> S[Lifecycle and audit state]
  T --> G[Gateway startup programs]
  G --> D[SQLite and Doctor fixture]
  T --> A[Adoption script]
  A --> K[Kubernetes fixture]
Loading

Technical review

Best possible solution:

Retain the focused coverage and land it after the existing required checks pass.

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

This PR addresses coverage gaps rather than a current runtime failure; its mutation results are author-reported and were not rerun.

Is this the best way to solve the issue?

Extending existing suites is proportionate, and the assertions generally follow documented contracts through real production owners with explicitly controlled dependencies.

AGENTS.md: found and applied where relevant.

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

Provenance checked

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Likely related people:

  • freeqaz: Raw commit 63a28da adds tests/conformance/runtime-gateway-state-migration.test.mjs:178 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 63a28da57903; files: tests/conformance/runtime-gateway-state-migration.test.mjs)
  • Mrunal Patel: Raw commit 6bbdc49 adds tests/integration/sandbox-driver-startup.test.mjs:411 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 6bbdc492f1bf; files: tests/integration/sandbox-driver-startup.test.mjs)

Review metrics

Metric Value Why it matters
Production versus test changes Production 0; tests +364/-35 Growth stays within four existing suites and adds distinct failure, lifecycle, ordering, and boundary observations.

Labels

Label changes:

  • add P3: This is low-risk test coverage work with no change to shipped behavior.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P3: This is low-risk test coverage work with no change to shipped behavior.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only; repair and merge need explicit maintainer commands such as @clawsweeper autofix or @clawsweeper automerge.

Reviewed October 9, 2026, 9:57 PM ET / October 10, 2026, 01:57 UTC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant