Restore SDK credential behavior coverage

Author: RhysSullivan

MISTAKES.md

@@ -0,0 +1,124 @@
+- During the Cloudflare deploy incident, I did not first compare the active
+  Worker version's secret bindings against the previous known-good version. The
+  failing `/api/auth/login` route was caused by the current Worker version
+  missing `WORKOS_API_KEY`, `WORKOS_CLIENT_ID`, and `WORKOS_COOKIE_PASSWORD`;
+  `wrangler versions view` exposed this quickly.
+- Ran `bun run format packages/core/sdk/src/oxlint-plugin-executor.test.ts`, but the repo script expands to `oxfmt .` and ignores file arguments, so it formatted the whole checkout instead of only the intended file. Use `./node_modules/.bin/oxfmt <files>` for scoped formatting.
+- Initially treated the HTTP payload schema check as good after direct imports passed, but the focused test caught that an aliased `HttpApiEndpoint` import (`HttpApiEndpoint as Endpoint`) was missed. Import aliases need explicit symbol/declaration handling in compiler-backed lint rules.
+- Tried to run SDK package-local tests through root `vitest run <package paths>` again; root Vitest only includes root `tests/**/*.test.ts`, so use the owning package script or run Vitest from that package directory.
+- Repeated the same root `vitest run <package paths>` mistake while checking MCP OAuth changes; use `bun run --cwd <package> test -- <src test paths>` for package-local Vitest files.
+- I treated "OAuth required but no dynamic client registration" as an OAuth-only state in the MCP add-source UI, which trapped users instead of letting them use PAT/header auth or save a deferred source.
+- I used raw JSON.parse in OpenAPI parsing even though this repo's lint policy requires Effect Schema JSON parsing at boundaries.
+- Ran a broad `find . -path '*node_modules*autumn*'` while debugging Autumn package code; it walked too much of the checkout. Use `rg --files | rg 'autumn-js'` or target `node_modules/.bun/<package>` directly.
+- Ran root `vitest run apps/cloud/src/api/autumn-customer.test.ts`; root Vitest only includes root tests, so app tests need to run from `apps/cloud` or with that package's Vitest config.
+- Ran root `bunx vitest run packages/react/src/api/oauth-popup.test.ts`; root Vitest only includes root `tests/**/*.test.ts`, so React package tests need `bun run --cwd packages/react test <src test path>`.
+- I treated `alchemy plan` as a non-mutating verification command. In Alchemy v2 beta.36 it can update/deploy the Cloudflare state store before planning application resources, so do not run it casually against shared profiles.
+- I tried to replace the real-port MCP Miniflare e2e harness with Alchemy dev-mode local Worker deployment before proving the local runtime could boot this app. Alchemy v2 beta.36's sidecar stayed in resource creation and hit its internal timeout, so keep that harness on the known-passing path until a smaller reproduction is solved.
+
+- Appended to LEARNINGS.md with shell redirection instead of apply_patch. This violated the repo editing preference; use apply_patch for manual file edits, including notes files.
+
+- During the Alchemy migration, I used a shell/Node file rewrite for apps/cloud/src/mcp-session.ts instead of apply_patch. Manual code edits should use apply_patch; only formatting/bulk tooling should write files directly.
+- While updating the cloud usage-tracking tests for Effect v4, I used the old `Effect.zipRight` API. Use `Effect.andThen` for this sequencing shape in this repo.
+- I probed marketing with `bunx vite build`, which pulled a temporary latest Vite and reported saving the lockfile. Use the package's own build script or Alchemy dry-run instead of `bunx` probes in this repo.
+- During the Alchemy fork work, I renamed the source workspace package from `alchemy` to `@rhyssul/alchemy`, which broke Alchemy's own `workspace:*` references. Keep the checkout package named `alchemy` for local development and use a publish staging copy for scoped npm releases.
+- Published `@rhyssul/alchemy@2.0.0-beta.36-rhyssul.0` before replacing Alchemy's `catalog:` dependency ranges, making the tarball unusable for direct installs outside the monorepo. Stage and inspect the package manifest before publishing the next scoped build.
+- Published `@rhyssul/alchemy@2.0.0-beta.36-rhyssul.1` with concrete deps but without rewriting Alchemy's internal self-imports from `alchemy/...` to `@rhyssul/alchemy/...`, so the CLI failed when installed from the scoped tarball. Scoped vendored packages need self-import rewrites in the staging artifact.
+- Tried to publish a prerelease `@rhyssul/alchemy` version without an npm dist-tag. `npm publish` requires `--tag beta` for prerelease versions, so include it in the staging publish command.
+- Prepared a staged `@rhyssul/alchemy` publish artifact but ran `npm publish` from the repo root, which attempted to publish the private workspace and failed. Always set the working directory to the staging folder for vendored package publishes.
+- I treated the 0.x `alchemy-run/alchemy` `TanStackStart` example as if it
+  should be manually ported into the Effect v2 fork. That mixed API lines.
+  For v2, update/rebase from `alchemy-run/alchemy-effect` and only carry
+  deliberate fork patches; don't import 0.x framework resources by hand.
+- Repeated the staged-package directory mistake with `npm pack --dry-run` while preparing the vendored Alchemy package. Treat every npm packaging command as needing an explicit staged-package `workdir`, not just `npm publish`.
+- Initially modeled a pure Cloudflare Worker service binding with a `Type` field. In Alchemy, `Type` makes the object look like a provisioned resource and the platform tries provider resolution; binding-only markers should follow the Images/Artifacts `kind` marker pattern.
+- Tried to derive the cloud app's global `Env` from `Cloudflare.InferEnv<ReturnType<typeof cloudWorker>>` while that worker binds `McpSessionDO`. Because the DO implementation itself depends on the global `Env`, this creates a type cycle (`McpSessionDO` referenced in its own base expression). Keep the cloud binding env explicit until Alchemy can model self-referential DO envs without an app-local declaration cycle.
+- Ran `bun run --cwd apps/cloud test -- <unit test files>` expecting the file filters to apply only to the default Vitest run. The package script runs default Vitest and then node-config Vitest, so the same filters made the node-config half fail with "No test files found." Use direct `node ../../node_modules/vitest/vitest.mjs run <files>` from `apps/cloud` for default-config file filters.
+- Ran the Alchemy fork's `Cloudflare/Workers/Worker.test.ts` as a local verification check. That test performs live Cloudflare worker lifecycle operations and also spawned Node against raw TypeScript, which failed under Node 25 strip-only mode on parameter properties. Prefer app dry-runs plus focused fork type/build checks unless the fork test environment is explicitly prepared.
+- While adding local static asset support to Alchemy's sidecar, I first let the
+  asset middleware check every GET/HEAD path. The disk service returned 200 for
+  `/`, so SSR routes were incorrectly served as static assets. Local asset
+  middleware should only intercept known static asset paths and forward normal
+  routes to the user worker.
+- I treated the initial `performance.now` prelude fix as verified after a
+  shallow root/assets smoke test. The generated bundle later overwrote
+  `globalThis.performance` with an undefined unenv placeholder, so delayed
+  request traffic still crashed. For Alchemy/workerd fixes, inspect the final
+  `.alchemy/bundles` output and run repeated request probes before calling the
+  runtime fixed.
+- While verifying the storage-drizzle fix, I ran the same package-local command
+  twice with a repo-root file path that did not exist from that working
+  directory. When switching `workdir`, adjust file paths or split search and
+  package script commands.
+- I again ran the cloud package `bun run test` script with a file/config filter.
+  That script invokes both Vitest configs, so forwarded `--config` args can
+  duplicate and fail. For one cloud test file, run
+  `node ../../node_modules/vitest/vitest.mjs run --config vitest.node.config.ts <file>`
+  from `apps/cloud`.
+- While restoring Alchemy MCP e2e coverage, I first adapted Alchemy's typed
+  Durable Object namespace back into the old Cloudflare `DurableObjectNamespace`
+  ambient type with `as unknown` casts. The correct long-term shape is to type
+  `Env.MCP_SESSION` as Alchemy's native
+  `DurableObjectNamespaceResource<McpSessionShape>` and pass the namespace
+  through directly.
+- I tried to quiet workerd request-stream stderr by disabling MCP POST body
+  fingerprinting without first proving the root cause. That made the local
+  worker less stable. Keep the passing realistic behavior test first, then
+  isolate noisy local-runtime logs separately.
+- I treated a clean git source tree as enough evidence that a production deploy
+  artifact was clean. The ignored `apps/cloud/dist` directory still contained
+  stale Alchemy output from another branch. For deploy debugging, inspect and
+  clean generated output as part of the diagnosis, not just tracked files.
+- While looking for local Claude Code MCP credentials, I let a command print a
+  short-lived refresh token into the tool output. Never print credential JSON
+  directly; use `jq` to select only non-secret metadata or pipe secret-bearing
+  fields into commands without echoing them.
+- During the Alchemy package republish, I published
+  `@rhyssul/alchemy@2.0.0-beta.36-rhyssul.10` before checking that the
+  packaged manifest had concrete dependency versions. The package still had
+  monorepo `catalog:` ranges and was unusable from this repo; inspect the packed
+  manifest before publishing vendored packages.
+- While diagnosing the MCP regression, I initially used broad grep-style
+  searches across auth/session files. Even with later redaction, that is the
+  wrong default around credential stores. Use targeted JSON parsers that print
+  only non-secret metadata, and pass tokens directly to test commands without
+  echoing them.
+- While verifying PR 263's MCP host tests, I first passed Jest's
+  `--runInBand` flag to Vitest. Vitest 4 rejects that option; run package
+  tests with `bun run --cwd <package> test <path>` instead.
+- I briefly switched an existing plain async Vitest file to import from
+  `@effect/vitest` without converting it to `it.effect`, which failed at suite
+  load. Convert tests intentionally or keep the file's current harness for a
+  small scoped change.
+- While restarting the PR 263 local MCP server, I used `pkill -f` with a
+  pattern that also matched the active restart shell command. Stop known PID
+  files/process groups first, then use narrower process checks instead of a
+  broad `pkill -f` pattern embedded in the same command string.
+- After moving code between workspace packages, I first ran
+  `bun install --lockfile-only`, which updated the lockfile but left local
+  workspace links and newly added dependencies unresolved. Run full
+  `bun install` after package.json dependency changes before typechecking.
+- While iterating on dynamic UI mutation state, I initially added server-side
+  regex rejections for common React Query patterns like `autoRenew: !autoRenew`
+  and stale success labels. That treated a runtime consistency issue as model
+  misuse. Prefer making the generated UI runtime preserve expected library
+  semantics, and reserve hard validation for code that cannot evaluate safely.
+- While investigating a production WorkOS/Axiom issue, I started an `op run`
+  command against `.env.production` in a non-interactive tool session and it
+  hung waiting for 1Password. Prefer the available MCP tools for production
+  support queries, or use an explicitly interactive path before invoking `op`.
+- While counting FumaDB LOC from the parent repo, I used `git -C` to list files
+  but ran `wc` from the wrong working directory, producing missing-file noise
+  and bogus totals. Run path-consuming commands from the same repo root or
+  prefix the listed paths before piping them onward.
+- In the storage SDK MVP, I initially shaped the prototype around a new
+  `install/context` API instead of mirroring the existing
+  `definePlugin(() => ({ storage, extension }))` SDK shape. For architectural
+  MVPs, preserve local API vocabulary first so the prototype is readable
+  against the real codebase.
+- In the same MVP, after moving plugin-owned storage out of core, I
+  overcorrected by removing plugin examples entirely. Keep plugin packages in
+  the monorepo as external SDK consumers when testing core/plugin separation.
+- During the FumaDB cutover test restoration, I tried to use `python` for a
+  small text rewrite and the command failed because this environment does not
+  have `python` on PATH. Use `apply_patch`, `perl`, or `node` for local edits
+  here instead of assuming Python is available.