Skip to content

fix(vault): describe the accepted clientInfo shape in vault set help - #302

Merged
steipete merged 1 commit into
openclaw:mainfrom
Yigtwxx:docs/vault-set-help-clientinfo
Aug 10, 2026
Merged

fix(vault): describe the accepted clientInfo shape in vault set help#302
steipete merged 1 commit into
openclaw:mainfrom
Yigtwxx:docs/vault-set-help-clientinfo

Conversation

@Yigtwxx

@Yigtwxx Yigtwxx commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Summary

mcporter vault set --help still presents clientInfo as client_id alone. Since #288 the validator accepts the RFC 7591 dynamic client registration shape — redirect_uris, grant_types, response_types and contacts arrays, the client_id_issued_at / client_secret_expires_at timestamps, and provider metadata outside the spec — so the only place a headless operator looks before writing a payload now under-describes what the command takes.

That gap is not hypothetical. Issue #286 was filed with exactly the expectation this line creates, and the reporter's payload was a full registration response.

Current output

src/cli/vault-command.ts:224

Payload:
  { "tokens": { "access_token": "...", "token_type": "Bearer" }, "clientInfo": { "client_id": "..." } }

docs/config.md:259 documents the payload as { "tokens": { ... }, "clientInfo": { ... } } without naming fields, so nothing else in the project contradicts the help text either way.

Fix

Four lines appended under Payload:, naming the field groups the validator now accepts. The existing one-line example is unchanged, so the shortest usable payload still reads first.

Payload:
  { "tokens": { "access_token": "...", "token_type": "Bearer" }, "clientInfo": { "client_id": "..." } }

  clientInfo accepts a full dynamic client registration response, including the
  redirect_uris, grant_types, response_types and contacts arrays, the
  client_id_issued_at and client_secret_expires_at timestamps, and provider
  metadata outside RFC 7591.

The wording deliberately names field groups rather than restating the whole rule table: OAUTH_CLIENT_STRING_FIELDS alone is 15 entries, and a help text that enumerates a validator is a second copy free to drift from it. "Provider metadata outside RFC 7591" is the one behavior a reader cannot infer from the spec — validateOAuthClientInfo iterates its own field lists rather than the payload, so registration_client_uri and registration_access_token reach the vault untouched.

No validator, payload, or persistence behavior changes.

Behavior proof

Real CLI through tsx src/cli.ts.

=== BEFORE (main @ e404ed5) ===
Payload:
  { "tokens": { "access_token": "...", "token_type": "Bearer" }, "clientInfo": { "client_id": "..." } }

=== AFTER (this branch) ===
Payload:
  { "tokens": { "access_token": "...", "token_type": "Bearer" }, "clientInfo": { "client_id": "..." } }

  clientInfo accepts a full dynamic client registration response, including the
  redirect_uris, grant_types, response_types and contacts arrays, the
  client_id_issued_at and client_secret_expires_at timestamps, and provider
  metadata outside RFC 7591.

Tests

tests/cli-vault-help.test.ts is new, in the shape of the existing tests/cli-auth-help.test.ts: it drives runCli(['vault', '--help']) and asserts the named field groups reach the output. vault had no help coverage before, so this also pins the Usage: line and the exitCode 0 the help shortcut sets.

Reverting only src/cli/vault-command.ts to main and keeping the test fails it on the dynamic client registration response assertion — 1 failed of 1.

Gates

Windows 11, Node 22.20.0, pnpm 10.34.5.

  • pnpm exec vitest run tests/cli-vault-help.test.ts tests/vault-command.test.ts — 7 passed.
  • oxfmt --check and oxlint --type-aware --deny-warnings on both touched files — clean. tsc --noEmit — clean.
  • Full pnpm check reports formatting in tests/cli-list-stdio-logs.test.ts and tests/list-inline-stdio.test.ts, which this branch does not touch and which report identically on unmodified main at e404ed5 on this machine. This box is below the engine floor the repo declares (node >=24); CI settles it.

CHANGELOG.md is untouched.

Since openclaw#288 the validator accepts RFC 7591 registration arrays and timestamps,
but the help text still lists clientInfo as client_id alone, which reads as the
whole contract and is the expectation issue openclaw#286 arrived with.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JeEyXV4E6bT9jTSmmWpWmu
@clawsweeper

clawsweeper Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

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

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. 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. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. labels Aug 9, 2026
@clawsweeper

clawsweeper Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed August 9, 2026, 7:09 PM ET / 23:09 UTC.

ClawSweeper review

What this changes

Expands mcporter vault set --help to describe accepted dynamic OAuth client-registration fields and adds coverage for that CLI help output.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

This PR remains necessary: current main accepts the expanded OAuth client metadata but its vault help still shows only client_id. The focused help text and matching CLI coverage are accurate, low-risk, and ready for ordinary maintainer merge review.

Priority: P3
Reviewed head: 2935d6f0294e54ef0a8c1a54276be555903e2b14

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, accurate CLI UX patch with direct runtime proof and matching regression coverage.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (terminal): The PR body provides a redaction-safe before-and-after real CLI transcript that directly shows the new help output.
Patch quality 🦞 diamond lobster (5/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The PR body provides a redaction-safe before-and-after real CLI transcript that directly shows the new help output.
Evidence reviewed 5 items Current validator contract: Current main validates the four named string-array fields and the two named timestamp fields, while leaving unlisted provider metadata untouched; the proposed help text accurately summarizes that behavior.
Current help remains incomplete: Current main’s vault help example presents clientInfo with only client_id, so the documentation gap has not already landed.
Focused patch and regression coverage: The branch adds five help-text lines and one 40-line CLI help test; the test follows the established auth-help test pattern and checks the command’s help shortcut and documented field groups.
Findings None None.
Security None None.

How this fits together

The vault command imports OAuth tokens and client registration metadata from stdin or a JSON file into MCPorter’s local credential store. Its help output guides headless operators before input validation and persistence.

flowchart LR
  A[Headless operator] --> B[vault set command]
  B --> C[CLI help output]
  B --> D[Payload validation]
  D --> E[OAuth credential vault]
  C --> F[Correct registration payload]
  F --> D
Loading

Before merge

  • Complete next step (P2) - No repair lane is needed; the patch has no actionable finding and only needs ordinary maintainer merge review.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production versus test delta production +5, tests +40 The small user-facing help change is backed by focused command-output coverage.

Technical review

Best possible solution:

Merge the narrow help and regression-test update so headless users can discover the OAuth registration payload shape already supported by MCPorter.

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

Not applicable as a defect reproduction: the PR includes a direct before-and-after real CLI transcript for the changed help output.

Is this the best way to solve the issue?

Yes. Updating the command’s own help text and pinning it with a focused CLI test is the narrowest maintainable way to expose the already-supported payload contract.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against e404ed575a47.

Labels

Label justifications:

  • P3: This is a focused CLI-help and test improvement with no runtime behavior change.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body provides a redaction-safe before-and-after real CLI transcript that directly shows the new help output.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides a redaction-safe before-and-after real CLI transcript that directly shows the new help output.

Evidence

What I checked:

  • Current validator contract: Current main validates the four named string-array fields and the two named timestamp fields, while leaving unlisted provider metadata untouched; the proposed help text accurately summarizes that behavior. (src/cli/vault-command.ts:187, a8b5a681b339)
  • Current help remains incomplete: Current main’s vault help example presents clientInfo with only client_id, so the documentation gap has not already landed. (src/cli/vault-command.ts:215, a8b5a681b339)
  • Focused patch and regression coverage: The branch adds five help-text lines and one 40-line CLI help test; the test follows the established auth-help test pattern and checks the command’s help shortcut and documented field groups. (tests/cli-vault-help.test.ts:1, 2935d6f0294e)
  • Merged behavior provenance: The accepted DCR metadata behavior dates to the merged OAuth vault fix; current main’s relevant lines blame to the subsequent v0.13.2 release preparation commit. (src/cli/vault-command.ts:191, 4e8e37df2004)
  • Real behavior proof: The PR body records before-and-after output from the real tsx src/cli.ts command, directly showing the changed help text without exposing credentials. (src/cli/vault-command.ts:224, 2935d6f0294e)

Likely related people:

  • feniix: Authored the merged OAuth DCR validation fix that established the contract this help text describes. (role: introduced accepted OAuth DCR behavior; confidence: high; commits: 4e8e37df2004; files: src/cli/vault-command.ts, tests/vault-validation.test.ts)
  • Peter Steinberger: Current main attributes the vault validator and help lines to the v0.13.2 release preparation snapshot. (role: recent release-area contributor; confidence: medium; commits: a8b5a681b339; files: src/cli/vault-command.ts)

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.

History

Review history (10 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-09T12:00:00.402Z sha 2935d6f :: needs maintainer review before merge. :: none
  • reviewed 2026-08-09T15:18:32.127Z sha 2935d6f :: needs maintainer review before merge. :: none
  • reviewed 2026-08-09T16:58:30.681Z sha 2935d6f :: needs maintainer review before merge. :: none
  • reviewed 2026-08-09T17:02:46.040Z sha 2935d6f :: needs maintainer review before merge. :: none
  • reviewed 2026-08-09T18:14:27.714Z sha 2935d6f :: needs maintainer review before merge. :: none
  • reviewed 2026-08-09T19:58:14.799Z sha 2935d6f :: needs maintainer review before merge. :: none
  • reviewed 2026-08-09T20:28:02.526Z sha 2935d6f :: needs maintainer review before merge. :: none
  • reviewed 2026-08-09T21:44:10.829Z sha 2935d6f :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. labels Aug 9, 2026
@steipete
steipete merged commit dcba5cc into openclaw:main Aug 10, 2026
13 checks passed
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. proof: sufficient Contributor real behavior proof is sufficient. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants