Merge branch 'v2/main' into v2/fix/1919-rfc6570-uri-template-expansion

This commit is contained in:
Cliff Hall 2026-08-16 23:16:27 -04:00 • committed by GitHub
commit 63250a6c52
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -45,7 +45,13 @@ v2/main/
│ │ # providers.redirectToAuthorization, the only
│ │ # seam that sees the SDK-built authorize URL;
│ │ # authorization request only, never the token
│ │ # request — #2018)
│ │ # request — #2018;
│ │ # endpointOverrides.ts per-server
│ │ # authorization/token URL overrides — a fetch
│ │ # wrapper that rewrites the discovered AS
│ │ # metadata document, the one seam SDK v2 routes
│ │ # BOTH endpoints through (neither reaches the
│ │ # OAuthClientProvider) — #1906)
│ │ ├── browser/ # Browser-side OAuth (sessionStorage, BrowserNavigation)
│ │ ├── node/ # Node-side OAuth (NodeOAuthStorage, OAuthCallbackServer,
│ │ │ # runner-interactive-oauth loopback callback flow)
@ -746,6 +752,12 @@ The ⚠️ option-deletion hazard, the snapshot rule, and the recovery recipe ab
- Use type guards and type assertions to handle potential type mismatches and ensure type safety
- Take advantage of TypeScript's advanced features like generics, type aliases, and conditional types to write more expressive and reusable code
- Regularly review and refactor TypeScript code to ensure it remains well-structured and adheres to evolving best practices
- **NEVER leave a promise floating.** `@typescript-eslint/no-floating-promises` is enabled at `error` in **all five** ESLint scopes — `clients/{web,cli,tui,launcher}` and the root `core/` + shared gate (#1959). Every promise must be awaited, returned, terminated with `.catch(…)`, or explicitly discarded with the `void` operator.
- **The class it catches is invisible at review time.** A floated call reads like an awaited one minus four characters, and the unhandled rejection it produces surfaces somewhere else entirely — a different test, a different file, a stack pointing at SDK internals. Two un-held `client.callTool(...)` promises made `npm run ci` unpassable in #1947: `disconnect()` rejected them with `Connection closed`, the unhandled rejection failed the whole vitest run, and the chain aborted at `coverage`, silently skipping `verify:build-gate`, `smoke`, and Storybook. Attributing it took a full investigation; the fix was two lines.
- **Prefer holding and settling the promise.** `void` is an escape hatch, not a fix — it is visible at review time (strictly better than nothing) but still discards the rejection. Reach for it only when the callee already owns its failures (it ends in a `catch` that surfaces the message) or the caller genuinely cannot await — a synchronous `useEffect` body, an Ink `useInput` key handler, a Hono `stream.onAbort` listener. Say **which** of those it is in a one-line comment; an unexplained `void` is a review finding. Where the callee does *not* own its failures, give it a `catch` rather than voiding the call (see `handleDisconnect` in `clients/tui/src/App.tsx`), or terminate with `.catch(…)` at the call site (see `open(url)` in `clients/web/server/{server,vite-hono-plugin}.ts`).
- **In Storybook play functions, `expect(...)` from `storybook/test` returns a promise.** Storybook instruments it so the interactions panel can trace each assertion, so every `expect` in a play function is awaited — as is any shared helper that wraps one (`src/test/scrollAreaStoryAssertions.ts` is `async` for this reason).
- **The rule is type-aware, so each scope's ESLint config carries a parser project.** Each client's config must name **every leaf project covering its lint surface**, since the parser needs a program that literally *contains* the file — for cli, tui, and launcher that is the two they already typecheck (`tsconfig.json` + `tsconfig.test.json`, `src` in the first and `__tests__` only in the second), and for **web it is four** (`tsconfig.app.json`, `tsconfig.node.json`, `tsconfig.storybook.json`, `tsconfig.test.json`) — web's `tsconfig.json` is a solution file with `files: []`, so naming it alone would contain nothing. Adding a leaf project to a client means adding it here too. The root config instead points at **`tsconfig.lint.json`**, which exists solely to give the parser a program covering `core/**`, `test-servers/src/**`, and `vitest.shared.mts` — none of which is rooted in a tsconfig of its own. That file emits nothing and gates nothing: type *checking* for those sources stays where it was (`core/` through `clients/web`'s `tsc -b`, `test-servers/src` through `clients/cli`'s test project). It sets `moduleResolution: bundler` deliberately — `core/` uses extensionless relative imports, which NodeNext fails to resolve, degrading every import to `any` so the rule silently stops seeing promises at all. **Its `include` must stay a superset of the root config's type-aware `files` globs** — a file the lint block matches but the project omits fails outright with "was not found in any of the provided project(s)" rather than being checked, so widening one means widening the other (that is why the `include` carries `test-servers/src/**/*.tsx`, which nothing has produced yet). Note also that a tsconfig `include` does **not** expand braces — `core/**/*.{ts,tsx}` matches nothing; list the extensions separately.
- Type-aware linting costs real time: `clients/web`'s `eslint .` went from ~8s to ~19s, and `lint` runs inside `validate`, the fast inner-loop check. That is the price of the guarantee; if it needs reducing later, narrowing the projects each scope loads is the lever, not dropping the rule.
## Web source layout: `src/lib` vs `src/utils`