vault set treats relative expires_in as immediately due and can invalidate fresh grants

[feniix]

Summary

mcporter vault set persists an OAuth token carrying expires_in without converting that relative lifetime to an absolute expiry. On the next list or call, mcporter treats the imported token as immediately due and redeems its refresh token even when it was issued seconds ago with a one-hour lifetime.

This can destroy a fresh grant. A headless OAuth owner commonly has to run two separate operations after a callback:

1. mcporter vault set with the new token and DCR client information.
2. Update the refreshable_bearer config with that DCR client's ID and token endpoint.

A concurrent probe between those operations can redeem the new refresh token with missing or previous client configuration. Providers that bind a refresh token to its DCR client return invalid_grant; mcporter then correctly clears the rejected tokens but retains clientInfo. The resulting vault entry looks like a successful registration with no token.

Root cause

The two persistence paths handle expires_in differently.

• VaultPersistence.saveTokens() calls prepareStoredTokens(), whose withStoredExpiry() adds expires_at = now + expires_in.
• handleVaultSet() calls saveVaultEntry() directly with payload.tokens, so no absolute expiry is recorded.
• shouldRefreshCachedToken() has no issue time for a relative-only token and returns true whenever both expires_in and refresh_token are present.

The behavior remains present in the published 0.13.7 package.

Reduced behavior proof

I configured one local refreshable_bearer HTTP definition whose token endpoint only counted requests, then seeded this non-secret fixture through the real CLI:

{
  "tokens": {
    "access_token": "fixture-access-token",
    "token_type": "Bearer",
    "refresh_token": "fixture-refresh-token",
    "expires_in": 3600
  },
  "clientInfo": {
    "client_id": "fixture-client"
  }
}

mcporter 0.13.4
vault set exit:       0
first list immediately after set:
  token redemptions:  1

The same proof on 0.13.7 also enters the relative-expiry refresh path. Adding the absolute field that mcporter's normal save path would have derived changes only this result:

"expires_at": <current epoch seconds + 3600>

first list immediately after set:
  token redemptions:  0

No live provider or real credential was used.

Expected behavior

vault set should persist imported tokens with the same expiry normalization as VaultPersistence.saveTokens(). A token seeded with expires_in: 3600 should not be refreshed until its actual expiry window.

A focused regression could freeze time, call the vault-set path with relative expiry, read the stored entry, and assert the derived expires_at. A runtime-level test can additionally assert that the first cached-token read performs zero token requests.

[feniix]

Follow-up exact-shape proof on 0.13.4, using isolated HOME/XDG roots and a loopback token endpoint that returns 400 {"error":"invalid_grant"} without inspecting credential values:

vault set exit:       0
first list exit:      1
token redemptions:    1
stored entry keys:    clientInfo, serverName, serverUrl, updatedAt
has tokens:           false
has clientInfo:       true

So the premature relative-expiry redemption reproduces the reported partial-vault shape exactly: the unrecoverable-refresh recovery removes tokens and intentionally retains clientInfo for invalid_grant.

[clawsweeper]

Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve. Reviewed August 27, 2026, 4:39 AM ET / 08:39 UTC.

Summary
Keep this issue open: current main still bypasses expiry normalization when vault set persists imported tokens, and the linked open PR is a focused, proof-backed fix for that exact defect.

Reproducibility: yes. The reporter supplied a real-CLI, loopback-provider reproduction on current main, and current source independently shows the direct vault write followed by the relative-expiry refresh fallback.

Root-cause cluster
Relationship: canonical
Canonical: #334
Summary: This issue is the canonical report; the linked open PR is the candidate implementation for the same direct-write expiry defect.

Members:

• fixed_by_candidate: fix(oauth): normalize imported token expiry #335 - Its stated change normalizes tokens imported by vault set, explicitly closes this issue, and provides matching before/after CLI evidence.

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

Next step
The linked open PR already owns this bounded repair, so a second fix PR would duplicate contributor work.

Review details

Best possible solution:

Land the focused repair in #335 after normal maintainer review, retaining explicit expires_at/expiresAt values for delayed imports.

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

Yes. The reporter supplied a real-CLI, loopback-provider reproduction on current main, and current source independently shows the direct vault write followed by the relative-expiry refresh fallback.

Is this the best way to solve the issue?

Yes. Routing vault imports through the existing expiry-normalization path is the narrowest repair, while preserving explicit absolute expiry for credentials imported after issuance.

AGENTS.md: found and applied where relevant.

Remaining risk / open question:

• Until the linked repair merges, importing a relative-only refreshable OAuth token can redeem a newly issued refresh token on its first use and leave a provider-bound grant unrecoverable after invalid_grant.

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

Label changes

Label changes:

• add issue-rating: 🦀 challenger crab: Current issue advisory state selects this label.
• add clawsweeper:current-main-repro: Current issue advisory state selects this label.
• remove issue-rating: 🦞 diamond lobster: Current issue advisory state no longer selects this label.
• remove clawsweeper:source-repro: Current issue advisory state no longer selects this label.

Label justifications:

• P1: A current OAuth import path can consume a fresh grant on first use and break real refreshable-bearer workflows.
• impact:auth-provider: The defect directly affects OAuth token expiry handling, refresh-token redemption, and client-bound grants.

Evidence reviewed

What I checked:

• Current main bypasses normalization: The vault-set handler validates the payload then writes payload.tokens directly through saveVaultEntry, without the persistence preparation path that derives an absolute expiry. (src/cli/vault-command.ts:50, ae3d9000c320)
• Canonical persistence path differs: The normal persistence store uses prepareStoredTokens, whose withStoredExpiry preserves explicit expiry aliases but derives expires_at from finite expires_in values. (src/oauth-persistence-stores.ts:35, ae3d9000c320)
• Relative-only token is considered due: Without expires_at or expiresAt, the refresh decision returns true whenever both expires_in and refresh_token are present. (src/oauth-token-refresh.ts:188, ae3d9000c320)
• Current tests leave the reported path unguarded: Vault-command tests cover token and client-info import but contain no assertion that a relative imported expiry is stored as an absolute value. (tests/vault-command.test.ts:116, ae3d9000c320)
• Open fix explicitly owns the issue: The provided PR context identifies an open, clean PR with explicit closing syntax, real built-CLI loopback-provider proof, and coverage for fresh and explicit delayed expiry imports. (272f53af8a1a)
• Current-source provenance: Blame attributes the current vault-set write path to the checked-out history boundary at the v0.13.6 release commit; shallow history prevents attributing an earlier introducer, but confirms the behavior remains in the current source lineage. (src/cli/vault-command.ts:50, e53ef107e4c9)

Likely related people:

• Peter Steinberger: The examined vault command, expiry-normalization helper, and refresh decision all trace to the available-history boundary at the v0.13.6 release commit; older attribution is unavailable in this checkout. (role: current source provenance; confidence: medium; commits: e53ef107e4c9; files: src/cli/vault-command.ts, src/oauth-persistence-stores.ts, src/oauth-token-refresh.ts)

How this review workflow works

• 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.