Skip to content

docs+comments: fold back /v1/mint-aws-creds retirement (closes #72)#104

Merged
hanwencheng merged 2 commits into
mainfrom
claude/distracted-albattani-e67a4f
May 23, 2026
Merged

docs+comments: fold back /v1/mint-aws-creds retirement (closes #72)#104
hanwencheng merged 2 commits into
mainfrom
claude/distracted-albattani-e67a4f

Conversation

@hanwencheng
Copy link
Copy Markdown
Member

Summary

PR #96 deleted the /v1/mint-aws-creds route + handlers/mint.rs + tests/mint_v2_flow.rs, but four downstream spots in operator-facing docs + 2 stale code comments still described it as live. The next operator running the runbooks top-to-bottom would have curl'd a 404 and not understood why. This patch folds the doc + comment fixes back into the tree per the project's Runbook-fix-fold-back + Land-the-fix policies.

It also explicitly closes #72 — GitHub did not auto-close it from PR #96's body (the closing keyword likely failed regex match because the issue number was inside ** bold-markup).

What landed

  • docs/dev-setup.md:110 — rewrite the daemon-side mint description from "calls broker's POST /v1/mint-aws-creds" to the actual current flow (/v1/mint-oidc-jwt + client-side sts:AssumeRoleWithWebIdentity).
  • crates/agentkeys-broker-server/src/env.rs:24 — drop the stale "(broker-internal, used by /v1/mint-aws-creds)" parenthetical on the SessionJwt env group; name the actual consumers (email-link / OAuth2 mint paths + /v1/mint-oidc-jwt).
  • crates/agentkeys-broker-server/src/main.rs:128 — drop the stale comment about mint_v2 mirroring rows into the audit log (mint_v2 was deleted in PR Retire legacy mock-server endpoints + /v1/mint-aws-creds + /v1/auth/exchange (closes #77, #72, #78) #96); name /v1/mint-oidc-jwt as the current writer.
  • docs/operator-runbook-stage7.md:
    • ASCII trust-relationship diagram (L138-146): change the request label from /v1/mint-aws-creds to /v1/mint-oidc-jwt.
    • "Mint-time STS paths (issue broker: mint-aws-creds is broken after cloud-setup §4 federation lands #71)" section (L370+): collapse the "two endpoints" framing into a single-path section; replace the whole POST /v1/mint-aws-creds — server-side gated subsection with a one-paragraph retirement callout (operators searching for the old path find a clear "this is gone" note instead of an apparently-live description).
  • docs/stage7-demo-and-verification.md:
    • §5 intro (L1441): drop "two paths" framing.
    • §5.2 "The server-side aggregator" (L1517-1552): delete entirely (it pointed into a deleted handlers/mint.rs#L125 and a deleted tests/mint_v2_flow.rs#L201); old §5.3 renumbers to §5.2.
    • §12.2 "Idempotency-Key" (L1945+): rewrite to explain the server-side dedup layer is gone with the route — /v1/mint-oidc-jwt's short TTL + daemon-side JWT cache cover the same use case without server help.
    • §15 "Future work" bullet (L2185): update "Retire /v1/mint-aws-creds entirely" from a forward-looking TODO to "✅ Done in PR Retire legacy mock-server endpoints + /v1/mint-aws-creds + /v1/auth/exchange (closes #77, #72, #78) #96", with the rationale of where the dropped gates (try_consume / Idempotency-Key / multi-anchor) went.
    • §16 audit-trail (L2425): rewrite the "hit /v1/mint-aws-creds for server-side audit row coverage" note — that path no longer exists; AWS CloudTrail's AssumeRoleWithWebIdentity events are now the STS-side trail.

5 files changed, +65 / -122 (net deletion).

What did NOT land

All plan steps shipped. None deferred. The patch is doc + comment only; no behavioral or interface changes.

Test plan

🤖 Generated with Claude Code

The route + handler + tests were deleted in PR #96, but four downstream
spots still described it as a live endpoint with current behavior. Land
the doc + comment fixes so the next operator running the runbooks does
not curl a 404.

- docs/dev-setup.md:110 — describe the actual mint flow (OIDC JWT +
  client-side STS) instead of the deleted server-side aggregator.
- crates/agentkeys-broker-server/src/env.rs — drop the stale
  "(broker-internal, used by /v1/mint-aws-creds)" parenthetical on the
  SessionJwt env group; name the current consumers
  (email-link / OAuth2 mint paths + /v1/mint-oidc-jwt).
- crates/agentkeys-broker-server/src/main.rs — drop the stale comment
  about mint_v2 mirroring rows into the audit log (mint_v2 was deleted
  in PR #96); name /v1/mint-oidc-jwt as the current writer.
- docs/operator-runbook-stage7.md — collapse the "two endpoints" §
  framing into the single surviving path. The old request label in the
  ASCII trust-relationship diagram now reads /v1/mint-oidc-jwt; the
  whole "POST /v1/mint-aws-creds — server-side gated" subsection is
  replaced with a one-paragraph retirement callout so operators
  searching for the old path find a clear "this is gone" note.
- docs/stage7-demo-and-verification.md — drop "two paths" framing in
  §5, delete §5.2 (the server-side aggregator deep-dive that pointed
  into a deleted handlers/mint.rs and a deleted tests/mint_v2_flow.rs),
  rewrite §12.2 Idempotency-Key to explain the dedup layer is gone with
  the route (JWT TTL + daemon JWT cache cover the same use case),
  update the future-work bullet from "Retire /v1/mint-aws-creds
  entirely" to "✅ Done in PR #96", and rewrite §16's audit-trail note
  to say the broker-side row of the actual mint no longer exists (AWS
  CloudTrail is the STS-side trail).

Per CLAUDE.md Runbook-fix-fold-back + Land-the-fix policies: PR #96
shipped the code work but did not touch these descriptive doc
sections, so the operator-facing runbooks would still send the next
person reading them to a 404. This patch closes that gap and explicitly
closes #72 (GitHub did not auto-close from PR #96's body).

cargo build -p agentkeys-broker-server clean (34s, exit 0).
Codex adversarial review (via /codex challenge) caught 5 categories of
real defects the first commit on this branch missed. All P1s + P2s
addressed here:

[P1 #1] operator-runbook-stage7.md still described deleted endpoint
behavior as live in two sections the first audit missed:

  - §L540-557 "Migration window — implicit-grant fallback" pointed
    operators at src/handlers/mint.rs::mint_v2 (deleted) and described
    a Phase E flip (BROKER_REQUIRE_EXPLICIT_GRANT=true) that no longer
    has a consumption point. Replaced with a "Grant enforcement
    retired" callout noting grant CRUD endpoints remain (so masters can
    still manage grants for audit / future re-introduction) but the
    mint-time try_consume path is gone.
  - §L698-707 "Idempotency-Key" claimed the mint endpoint accepts the
    header and dedups bodies within a 5min window. /v1/mint-oidc-jwt
    does not honor Idempotency-Key — replaced with a retirement note
    pointing at BROKER_OIDC_JWT_TTL_SECONDS=300 as the only re-mint
    cost knob.

[P1 #2] My §12.2 rewrite in stage7-demo-and-verification.md invented a
"daemon caches the JWT in-process" claim that does not match
crates/agentkeys-provisioner/src/aws_creds.rs::fetch_via_broker, which
fetches a fresh OIDC JWT and assumes a fresh role every call (no cache
layer). Replaced with the truth: clients must implement batching /
dedup / rate-limiting themselves, with a code reference for verification.

[P1 #3] docs/spec/plans/issue-64/PLAN.md (and the prd.json next to it)
still describe /v1/mint-aws-creds + Phase B grant try_consume +
Idempotency-Key as live with passing acceptance criteria. Added a
retirement preamble at the top of PLAN.md flagging that the route +
gates were deleted in PR #96 and pointing readers at arch.md §17.2
for the current isolation contract. The prd.json acceptance entries
are left as-is to preserve the audit record — the preamble + this
commit message are the durable "this no longer matches reality" signal.

[P2 #4] Code-comment cleanup:
  - state.rs:42-46 (grant_store) — re-cast from "the mint endpoint
    consults this" to "backs the /v1/grant/* CRUD endpoints; mint-time
    try_consume gone with mint_v2"
  - state.rs:52-55 (idempotency_store) — note that the only consumer
    is gone and the field is slated for removal (follow-up task spawned
    via mcp__ccd_session__spawn_task — see "Remove dead IdempotencyStore code").
  - aws_creds.rs:32 — clarify the AwsTempCreds field shape matched the
    legacy /v1/mint-aws-creds response, which is now deleted.
  - aws_creds.rs::build_session_name doc comment + matches_broker_format
    test comment — drop references to handlers/mint.rs::build_session_name
    (deleted); reframe as daemon-side-only.
  - tests/grant_flow.rs module doc — drop the "covered in mint_v2_flow
    separately" claim (mint_v2_flow.rs is deleted); note CRUD-only
    surface today.
  - tests/oidc_flow.rs:181 — drop the "(parity with /v1/mint-aws-creds)"
    parenthetical.

[P2 #5] PrincipalTag terminology drift between operator runbook and
arch.md §17.2. Runbook said creds are tagged with `agentkeys_user_wallet`,
but §17.2's per-actor isolation invariant is `agentkeys_actor_omni`.
Code (oidc.rs:181) emits both for v0.1 bucket-policy back-compat.
Rewrote runbook to lead with the §17.2-canonical tag and note the
legacy tag stays for back-compat — per the "Terminology-source-of-truth
rule" in CLAUDE.md.

Also: a second pass of repo-wide grep found 3 more stale references
outside the first commit's blast radius:
  - aws_creds.rs:32 field-shape comment (fixed)
  - tests/grant_flow.rs + tests/oidc_flow.rs comments (fixed)
  - docs/spec/plans/issue-74-dev-key-service-plan.md ASCII diagram
    showing /v1/mint-aws-creds as a live arrow (fixed inline + small
    retirement note)

What stays (intentionally):
  - docs/spec/plans/development-stages.md:23 — historical "Stage 7
    phase 1 (2026-04)" table entry. Date-anchored historical record,
    accurate at that stage. Not rewritten.
  - docs/archived/**, docs/research/**, docs/spec/plans/issue-64/*.md
    (other than PLAN.md preamble), progress.txt — pre-existing
    historical / scratch content, not operator-facing.

Build still clean (cargo build -p agentkeys-broker-server -p agentkeys-provisioner, exit 0).
Test suite unchanged in behavior — all edits to test files are comments only.
@hanwencheng
Copy link
Copy Markdown
Member Author

Codex adversarial review — second pass

Ran /codex challenge against the original PR; Codex found 3 P1 + 2 P2 failures that the first pass missed. Follow-up commit 969386a addresses every one. The PR went from claim-1/2/3 FAIL to claim-1/2/3 PASS after this commit.

[P1] Stale endpoint references in operator-runbook-stage7.md

Two sections I missed kept describing the deleted endpoint as live:

  • §L540 Migration window — implicit-grant fallback pointed at deleted src/handlers/mint.rs::mint_v2 and described a Phase E flip (BROKER_REQUIRE_EXPLICIT_GRANT=true) that no longer has a consumption point.
  • §L698 Idempotency-Key claimed the mint endpoint accepts the header and dedups bodies within a 5-minute window. No surviving route honors it.

Both converted to retirement callouts.

[P1] Factually-wrong rewrite in stage7-demo-and-verification.md:1949

My §12.2 rewrite invented "the daemon caches the JWT in-process" as a rationale for dropping idempotency dedup. Codex correctly pointed out that crates/agentkeys-provisioner/src/aws_creds.rs::fetch_via_broker calls fetch_oidc_jwt + assume_role_with_jwt every invocation — no cache layer. Rewrote with the actual truth (clients must batch / dedup themselves, with a code reference).

[P1] docs/spec/plans/issue-64/PLAN.md describes deleted surface as live spec

The plan doc still treated /v1/mint-aws-creds, Phase B grant try_consume, and Idempotency-Key as live with passing acceptance criteria. Added a retirement preamble at the top of PLAN.md flagging that those surfaces were deleted in PR #96 and pointing readers at arch.md §17.2 for the current isolation contract. The prd.json acceptance entries stay as-is to preserve the audit record.

[P2] Stale code comments

  • state.rs:42-46grant_store re-cast from "the mint endpoint consults this" to "backs /v1/grant/* CRUD endpoints".
  • state.rs:52-55idempotency_store flagged as "slated for removal" (the only consumer is gone). Follow-up task spawned to delete the store + remove the field; that's beyond the scope of this PR (a doc/comment fold-back) but worth tracking.
  • aws_creds.rs:32 + aws_creds.rs::build_session_name doc + matches_broker_format test comment — drop references to deleted handlers/mint.rs::build_session_name.
  • tests/grant_flow.rs:8 — drop "covered in mint_v2_flow separately" (that test file is deleted).
  • tests/oidc_flow.rs:181 — drop "(parity with /v1/mint-aws-creds)".

[P2] PrincipalTag terminology drift

operator-runbook-stage7.md:372 said creds carry agentkeys_user_wallet, but arch.md §17.2's per-actor isolation invariant is agentkeys_actor_omni. Code emits both for v0.1 bucket-policy back-compat. Rewrote to lead with the §17.2-canonical tag, per the Terminology-source-of-truth rule in CLAUDE.md.

Additional stale references the second-pass grep caught

Codex's challenge prompted me to repeat the repo-wide grep — found 3 more spots outside the first commit:

  • aws_creds.rs:32 field-shape comment (fixed above).
  • tests/grant_flow.rs + tests/oidc_flow.rs (fixed above).
  • docs/spec/plans/issue-74-dev-key-service-plan.md:81 ASCII diagram showing /v1/mint-aws-creds as a live arrow (deleted from diagram + inline retirement note).

Out of scope (flagged separately)

Verdict

Codex re-runnable for verification:

/codex challenge against branch claude/distracted-albattani-e67a4f

All three original claims (a) all live references converted/removed, (b) factual accuracy of surviving-flow descriptions, (c) dropped gates non-load-bearing — now pass after this commit.

cargo build -p agentkeys-broker-server -p agentkeys-provisioner clean.

🤖 Generated with Claude Code

@hanwencheng hanwencheng merged commit 6cf6f0e into main May 23, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

broker: retire /v1/mint-aws-creds (issue #71 Option A end-state)

1 participant