docs(0152): file the self-service onboarding portal task - #172
Open
adamkoot wants to merge 1 commit into
Open
Conversation
Tranche 3 deliverable and an RFP acceptance criterion: external developers get a Prices API key without a human in the loop — Discord sign-in, key issued and shown on-screen, dashboard showing usage against quota, quickstart alongside. Two files: the task README (placement plan, implementation order, acceptance criteria, open items) and the settled epic it derives from, archived under sources/ so the resolved decisions travel with the task rather than living only in the design docs. Filed as 0152, not 0145. The shared ID sequence already had 0145 taken by backlog/0145_BUG_synth-not-run-on-infra-only-prs.md on develop, and 0144-0151 are claimed across other branches; 0152 is the first ID free across every ref. Renumbered before anything was committed, so no history carries the collision. Nothing here is code. The first implementable slice is the rate-limit change (1 req/s + monthly quota replacing the 100 req/s default), which is independent of the portal and ships on its own.
adamkoot
added a commit
that referenced
this pull request
Aug 6, 2026
PR #169 review (okarcz). The task spawned from 0124's future work claimed an ID that PR #168 had already taken for the BE-0199 USD read-surface defects. That branch was unmerged when this one was cut, so the collision was invisible in the tree. The BUG side keeps 0144: it is cited by 0145-0151 and 0154, the BE-facing reply and the phase plan. 0152 (#172), 0153 and 0154 are all claimed, so this moves to the next free ID. 0153's note reserving 0152 for this task has been overtaken by #172. References updated here: redocly.yaml's info-license-strict comment (a fifth site, not in the review's list of four) and the api-endpoints doc. The 0124 task file's two links follow in the review-record commit.
adamkoot
added a commit
that referenced
this pull request
Aug 6, 2026
* feat(lore-0124): expose the OpenAPI spec through API Gateway `/api-docs-json` was defined in the axum router but never mapped by API Gateway, so in production the spec was reachable only by running `extract_openapi` locally. Map it as a keyless Lambda proxy onto the existing api-handler, cached 3600s at the stage cache to match the handler's Cache-Control — the document is byte-identical for the life of a deployment. Anonymous by design: an API description is public documentation, and gating it behind a key the reader does not have yet is a self-service dead end. `/health` already set the precedent; the in-app gate already exempted the path. `servers` is now stamped in production. `apiBaseUrl` is new config (validated at synth for the stage-prefix trap) passed to the handler as `API_BASE_URL`, rather than derived from `api.url` — ComputeStack is a dependency of ApiGatewayStack, so reading the gateway's URL there would close a cycle. Linting the document surfaced two real gaps, both fixed: `extract_openapi` emitted a `servers`-less variant of the spec (now shares `stamp_servers` with the served one), and seven key-gated operations documented no 401/403 at all (now documented, with ErrorEnvelope as a published schema). The `x-api-key` scheme is declared and required document-wide, with /health and /api-docs-json opting out explicitly. `npm run openapi:lint` extracts the served document and runs Redocly's recommended ruleset over it; wired into the rust CI job. * fix(lore-0124): make the OpenAPI gates actually fail Review of the previous commit found two guards that looked like they worked and did not. `redocly lint` exits 0 on warnings, and under `recommended` most checks — including operation-4xx-response, the rule that found seven key-gated operations documenting no 401/403 — are warnings. The CI step would have accepted the exact regression it was added to catch. Extend recommended-strict instead, and pin that rule to error so a switch back to recommended cannot silently demote it. "Route coverage, both directions" was enforced against a hand-written list mirroring the CDK source, so it could not catch a route added to axum with a plain .route() call and never mapped — which is precisely how /api-docs-json went unroutable. Add verify-openapi-routes.mjs, deriving both sides from the synthesized template and the extracted document, and run it in CI after synth. Same reasoning as lambda-assets.sh (task 0077). Both fixes verified by confirming they fail on a seeded regression. * fix(lore-0124): restore the lock entry npm 11 pruned `npm ci` failed on CI with "Missing: yaml@2.9.0 from lock file". Adding @redocly/cli was done with npm 11.17.0 on Node 26, which pruned `node_modules/vitest/node_modules/yaml` — an optional peer entry that the Node 22.22.0 / npm 10 toolchain in .nvmrc still resolves and requires. The lock was regenerated under 22.22.0, so the diff against develop is now purely the @redocly/cli addition. Verified by running `npm ci` on a clean tree under 22.22.0: the previous lock reproduces the CI failure, this one exits 0. * docs(lore-0124): tell 0126 which knob moves `servers` The task asked to note the ordering for the custom-domain change. 0126 already said to update `servers` alongside the docs; now that 0124 has landed it can name the single config value that does it, and the two gotchas around it. * docs(lore-0124): publish the bounds the code enforces A second validator, IBM's `ibm-openapi-validator`, reported 13 errors on the document Redocly passes cleanly. Seven were real and are fixed here; all seven are cases where the code already had a bound the document did not state. - The five ledger-sequence fields are `u64` in Rust because ClickHouse returns `UInt64`, but a Stellar ledger sequence is `uint32` in the protocol's `LedgerHeader`. The document promised a range four billion times wider than reality; `maximum: 4294967295` is a domain fact, not a limit we impose. - `limit` has been rejecting `0` and anything over 200 with a 400 since it was written, and said so nowhere a client could read. Now declared. - utoipa published the whole `/health` rustdoc as `summary` — 223 characters of maintainer-facing prose where a label belongs. Split into `summary` plus `description`. The remaining six are deliberate: four are OpenAPI 3.1 constructs the validator judges against 3.0's rules, one is a maximum that does not truthfully exist (`Candle.trade_count`), and one is the `/api-docs-json` path, kept rather than renamed. The task file records the full accounting and the reasoning for each. * fix(lore-0124): close three gate holes and document 429/500 PR #169 review. Three of the four gates this task shipped had holes, all of the same shape as the two already in Issues Encountered — guards that looked like they worked. `openapi:verify-routes` did not chain `openapi:extract` the way `openapi:lint` does, so it compared the template against whatever target/openapi.json happened to hold. A stale file reads as a pass, or as drift nobody can reproduce. The rust paths filter omitted package.json and package-lock.json while @redocly/cli is a devDependency only this job runs. A PR dropping or bumping it merged green and broke the next author to touch packages/**. HTTP_METHODS included options and head, so 0126's addCorsPreflight would have failed the gate with the remedy "add a #[utoipa::path] for each" for methods OpenAPI does not conventionally describe. Both are now excluded from both sides — excluding one side only manufactures drift. ANY is rejected loudly instead: it can never match an operation key, and skipping it would hide a mapped route from the check. The fourth was not a gate. Seven key-gated operations documented 401/403 but not 429 or 500, the two statuses a partner actually meets — the usage plan throttles, and all seven reach errors::db_error. A generated client fell into its unexpected-response branch for both. The 403 description named the usage plan, which is what returns 429; 403 is the key being missing or unauthorized. Verified by injection, not by reading: an OPTIONS method added to the synthesized template is ignored, an ANY method exits 1, and deleting /v1/prices/batch from the document still reports it undocumented. cargo test --workspace 223 passed; redocly lint 0 errors 0 warnings; verify-routes agrees on all 9 routes. * fix(lore-0124): publish the trade_count ceiling Reverses a decision from this task's own validator accounting, where `Candle.trade_count` was left unbounded as "no truthful maximum exists". The premises were right — a trade count has no protocol bound, `u64::MAX` overflows JSON's safe-integer range, and a domain figure would be invented — but the conclusion did not follow. The ceiling is the safe-integer range itself. `2^53 - 1` is the largest integer an IEEE 754 double represents exactly, and JSON has no integer type, so above it a client's parser silently rounds. Publishing it states a fact about the wire format rather than a limit we impose: values above it cannot be delivered correctly whatever ClickHouse holds. Same kind of claim as the ledger-sequence bound, taken one layer down — that one is a protocol fact, this one is transport. Real Stellar volumes sit ~10 orders of magnitude below it, so it never binds and cannot make a future response contradict the document, which was the actual worry behind leaving it out. It remains a published ceiling rather than a runtime clamp, the same caveat the review raised against the ledger fields. ibm-openapi-validator --errors-only: 6 -> 5, the remainder being the four 3.1-vs-3.0 entries and the deliberate path-casing one. Redocly still 0 errors 0 warnings; cargo test --workspace 223 passed; verify-routes agrees on all 9 routes. * docs(lore-0124): record staying at 5 IBM validator errors The accounting said what the six remaining errors were but not what to do about them, and named only the utoipa downgrade as the route to zero. Two things learned since are worth not re-deriving. Zero is reachable without touching utoipa: the validator takes a Spectral ruleset, and switching off the three offending rules produces "passed the validator". Measured, not assumed. Declined anyway — the document is already correct 3.1, so the choice is between disabling rules globally (broader than the two path entries in .redocly.lint-ignore.yaml) and down-converting to 3.0 before linting, which breaks decision #9 by making the linted document stop being the served one. More important, errors are not where the ruleset stops. At warning level it demands ErrorEnvelope carry `trace` and an `errors` array — IBM's error-container shape. No toggle removes that honestly, so adopting the tool means redesigning the error body on every endpoint and breaking every client. That is an API redesign, not a lint cleanup, and it is now attached to the open question for Oskar so the cost is visible when the question gets asked. * chore(lore-0124): renumber the license task 0144 -> 0155 PR #169 review (okarcz). The task spawned from 0124's future work claimed an ID that PR #168 had already taken for the BE-0199 USD read-surface defects. That branch was unmerged when this one was cut, so the collision was invisible in the tree. The BUG side keeps 0144: it is cited by 0145-0151 and 0154, the BE-facing reply and the phase plan. 0152 (#172), 0153 and 0154 are all claimed, so this moves to the next free ID. 0153's note reserving 0152 for this task has been overtaken by #172. References updated here: redocly.yaml's info-license-strict comment (a fifth site, not in the review's list of four) and the api-endpoints doc. The 0124 task file's two links follow in the review-record commit. * fix(lore-0124): make the guards from the #169 review actually guard Four of the review's points are the same shape as the bug this task exists to fix: something that looks like a check but is not one. 1. openapi:verify-routes never ran on the PRs most likely to trip it. It lives in the rust job, whose paths filter had no infra/** entry, so an infra-only PR adding a gateway route skipped the only check that sees the gateway->spec direction. Adding a gateway route is a pure-infra edit while adding an axum route touches packages/**, so the uncovered direction was the more likely one. Listing the single stack file rather than infra/** keeps unrelated CDK edits off the ARM Rust build. 2. LEDGER_SEQ_MAX asserted a tautology. The const restated u32::MAX as u64 == 4_294_967_295 and tied the five schema(maximum) literals to nothing: retyping one to 4_294_967_296 left the build green and published a wrong bound. Replaced with a test that reads the bounds back out of the served document, with the field set derived from the document so a later ledger field that forgets the attribute fails as a missing maximum. Mutation-checked both ways. 3. fullPath() truncated silently, despite its comment promising to fail loudly. A partial path still looks like a route, so a broken template surfaced as drift on a path that was almost right rather than as the parse failure it was. Both exits now throw. The root-method check moved above the ANY check so the ANY message always has a resolved path to name. 4. The two route guards compared different method sets: the Rust test matched head/options, the mjs drops both from both sides so 0126's addCorsPreflight does not read as drift. Aligned. Also switches extract-openapi.sh off require()-ing the env JSON as a module. The stated hazard does not reproduce (node -p is still CommonJS under "type": "module", and the root package.json has no type), but the old form depended on both of those staying true and reading bytes depends on neither. * docs(lore-0124): separate /api-docs-json's cost profile from /health's The keyless posture cites /health as precedent, and for the posture it is one. For cost it is not: /health is a MockIntegration and can never invoke anything, while /api-docs-json is proxy([]), so a cache miss reaches the Lambda and the route sits outside the usage plan with only the stage-wide throttle it shares with paying traffic. The residual stays small for reasons already in the stack -- a 3600s TTL with no cache-key parameters, so every caller collapses onto one entry, and API Gateway's default requireAuthorizationForCacheControl blocking anonymous cache-busting -- but none of that was written down, so "matches the /health precedent" read as "same cost profile". States it, and names the lever for a harder bound: a method-level throttle, not a key requirement. Also records the full #169 review response in the task file and completes the 0144 -> 0155 renumber's remaining two links. * fix(lore-0124): close four holes the review found in the guards All four are cases where a check reads as covering something it does not. Each is verified by mutating the artifact it reads and confirming it now fails; three of the four passed that same mutation before. 1. Nothing checked that the deployed handler is configured with the URL the document advertises. extract-openapi.sh stamps `servers` from infra/envs/production.json and exports API_BASE_URL itself, so it never observes ComputeStack putting that variable on the Lambda. Rename it to API_BASE_URI in an unrelated refactor and synth, lint and the route gate all pass while production serves a document with no `servers` block at all. New openapi:verify-servers compares the synthesized Compute template against the extracted document, and re-asserts the stage-prefix invariant against the stage the template actually deploys so deleting the types.ts validation cannot silently remove it. The handler is identified by carrying API_BASE_URL, not by name, so a rename fails as "no function declares it". compute-stack.ts and types.ts join the rust paths filter, which is what makes the new check run on the PRs that would break it. 2. Dropping head/options from both route guards fixed the disagreement between them by removing the coverage. A documented HEAD was then checked by neither guard in either direction — the same unroutable documented route 0124 exists to close, reopened for two verbs. HEAD is compared normally now; OPTIONS stays skipped on the gateway side only (0126's addCorsPreflight), and a documented OPTIONS is refused outright by both guards instead of ignored. 3. fullPath() still truncated silently. The rewrite threw at two exits, but the truncation happened earlier: any ParentId that was not `{Ref}` became null, and null is the walk's "reached the root" signal. An imported RestApi or a cross-stack split (0126) emits Fn::ImportValue and would have produced `/status` for `/v1/backfill/status` — drift reported on a path that is almost right, or worse, genuine drift passing if the truncation collides with a documented path. ParentId is now classified into ref/root/unresolved, and PathPart and HttpMethod are rejected unless they are literal strings. 4. The ledger-ceiling test matched a `_ledger` name suffix, so its own promise — "a ledger field added later without the attribute fails" — held only for that name shape, and the count assertion could not see a field the filter never matched. Replaced with two rules: by type over the schemas reachable from the /v1/backfill/status response $ref (every integer there is a ledger sequence), and by name over the whole document using `contains`, not a suffix. * fix(lore-0124): document the 429 both anonymous routes can return The lint exception for /health and /api-docs-json claimed they "genuinely have no 4xx to document". The same branch's own stack comment says otherwise: neither route is in the usage plan, but both sit under the stage-wide `/*` `*` throttle, so API Gateway can 429 either one. /api-docs-json can also 5xx, because unlike /health it is a Lambda proxy and a cache miss reaches the handler. Left as it was, a partner generating a client from this document gets no error branch for either route: a 429 arrives as `{"message": …}` and the client tries to deserialize it as the OpenAPI document. That is the exact failure a0b9b29 fixed for the seven key-gated operations; these two were excepted rather than fixed. Both responses are documented without a body, because API Gateway produces them and its shape is not ErrorEnvelope — same asymmetry already recorded for 403/429 on the data routes. .redocly.lint-ignore.yaml is now empty and stays in the tree carrying the reason, so the exceptions cannot quietly come back as the fix. * fix(lore-0124): stop serving a deployment's spec after it is gone Both caches on /api-docs-json were 3600s, justified by "the document is byte-identical for the life of a deployment". True, and beside the point: the caches outlive the deployment that filled them, and nothing dropped either one when a build shipped. A partner who fetched the document minutes before a release kept generating clients from the old one for the rest of the hour, with no staleness signal — at exactly the moment integrators go look at it. Split by who controls the cache: - Gateway stays 3600s and is now FLUSHED on deploy. `make -C infra deploy-production` and `deploy-production-compute` both run flush-production-cache, which reads the REST API id from the SSM parameter the stack already publishes. API Gateway has no per-route flush, so it drops the whole stage cache — harmless, every other TTL there is 10-60s on self-correcting data. - The handler's Cache-Control drops to 300s, because a partner's HTTP cache is the one we cannot flush. Revalidating every 5 minutes costs nothing: those requests land on the gateway cache, not the Lambda. This is the one place the cache_control tiers and the stage TTLs deliberately disagree, so both sides say why. Also replaces the `{}` fallback in lib.rs. A serialization failure served a syntactically valid EMPTY document as 200 OK — no log, no metric — then cached it. Every generator run in that window produced a client with zero endpoints and nothing reported a fault. Failing to start is louder and shorter, and matches extract_openapi, which already .expects the same call. README said openapi:lint runs Redocly's `recommended`; it runs `recommended-strict`, and the distinction is the whole point of the gate (plain `redocly lint` exits 0 on warnings). Someone trimming config back to "the documented ruleset" would have disarmed it. * feat(lore-0124): throttle the anonymous /api-docs-json route Every other Lambda-backed route is apiKeyRequired, so it carries two limits from the usage plan: the per-key rate and the daily quota. /api-docs-json is anonymous by design and therefore has neither — its only limiter was the stage-wide bucket it SHARES with paying partners. Throttling is evaluated before the cache, so an anonymous loop on the documentation route draws that bucket down and a partner inside their contracted 100 req/s starts seeing 429s from a route they never called. 10 req/s aggregate, burst 20: far above any legitimate use of a static ~40 KB document cached for an hour at the edge, and 5% of the stage ceiling. A local constant rather than a config key, because it follows from the route's shape (anonymous, cached, static) rather than from an environment's capacity. The methodSettings block moves out of the `if (cacheEnabled)` branch. It was skipped wholesale when the stage cache is off — which is the configuration where an unbounded keyless route costs the most, since every request is then a billed Lambda invocation. Cache TTLs stay conditional; throttles no longer are. The route gets ONE entry carrying both, since method settings are keyed by resourcePath+httpMethod and two entries would collide. Kept as its own commit deliberately. The #169 review looked at this route's posture and accepted it ("the residual is small"), asking only for the cost profile to be written down. This goes further than that, so `git revert` this one commit if you would rather it did not. * docs(lore-0124): record the self-review round and correct what it invalidated The task file described the branch as it stood three commits ago, and several of its claims were made false by this session's own changes — which is the same failure mode the branch keeps finding in its guards, so it does not get to stay in the file that documents them. Corrected: - AC said the document passes Redocly's `recommended` ruleset. It passes `recommended-strict`, and now with 0 ignored rather than 2 — the two `operation-4xx-response` exceptions were deleted, not preserved. - AC and Implementation Notes said the cache was "3600 s, gateway + handler agreeing". It is 3600 s at the gateway (flushed on deploy) and 300 s at the client, deliberately disagreeing. - Design Decision #3 asserted that agreement as a decision. Struck through rather than rewritten: it was made, shipped, and then found wrong, and that sequence is the part worth keeping. - Verification carried stale counts (223 workspace / 8 openapi, "2 ignored") and said cdk synth had not been re-run. It has: 225 / 9 / 0 ignored, and synth needed a workaround now recorded, since a plain dev checkout has no Lambda bootstrap assets and fails with CannotFindAsset before rendering anything. Added: the self-review round, its mutation-check table, and the finding that matters more than any individual item — three of the four fixes made for the #169 review were incomplete in the same way the originals were. The review caught a class of defect and the fixes reproduced it one layer down; that is what a later reader needs, not the four bugs. Also records the one item still open: the /api-docs-json throttle goes beyond what #169 accepted, so it is isolated in 479548c pending okarcz's call. * docs(lore-0124): defer the directory conversion to archive time The file is ~680 lines against the ~150-line threshold in lore/1-tasks/CLAUDE.md, and larger than any existing task README here. Recording the decision so a later session reads it as deliberate rather than as an oversight to re-litigate: the archive move is a `git mv` anyway, so converting then is one operation instead of two, and doing it now would hand the PR reviewer a large rename on the file he is reading. Carries the proposed split, the target README size, and the two things that break if it is done without care — the current-task symlink and the inbound links from 0128 and 0155.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
onboarding-apiRust package, new DynamoDB-backedOnboardingStack, newonboarding-portalSPA, newportal-stack.ts), the implementation order, acceptance criteria, and the open items still needing sign-off.sources/so the resolved decisions travel with the task instead of living only in the design docs.backlog/0145_BUG_synth-not-run-on-infra-only-prs.mdon develop, and 0144–0151 are claimed across other branches. Renumbered before anything was committed, so no history carries the collision.Documentation only — no code, no CI-visible surface. The first implementable slice is the rate-limit change (1 req/s + monthly quota replacing the current 100 req/s default), which is independent of the portal and ships on its own.