chore: remove dead modules, collapse duplicated helpers, fix the drift they caused - #451
Conversation
Thirteen modules had zero importers, in production or test. Each was created
by an extraction refactor that never switched its caller over, so the original
kept an inline copy and the new file was unreachable from its first commit:
dev/agent-api-wire, dev/epoch-staging, dev/eval/{eval-artifact-reader,
eval-event-subscription,eval-service-error}, dev/mcp-apps/mcp-app-protocol,
dev/playground/{native-playground-evidence,playground-close-errors,
playground-store-codec,playground-store-layout,playground-subscriptions},
eval/run-store-codec, and workbench mcp/mcp-session-trace-client.
The surviving duplicates now share one owner. dev/http.ts already held the
canonical request/response helpers and artifact-routes.ts already used it;
eval-routes, mcp-app-routes, runtime-routes, and foreground-server each kept a
private clone including its own RequestDiagnostic type, and now import it.
foreground-server keeps a thin responseDiagnostic wrapper so it can go on
attaching diagnostics carried on the error. mcp-app-bridge takes validIcons and
validIsoDateTimeWithOffset from mcp-app-action-validation. The conventional
entry probe that config/normalize.ts and routes/graph.ts had copied to avoid an
import cycle moves to the leaf module config/conventional-entry.ts, which
cannot close the cycle.
AGENTS.md gains a Code hygiene section describing how this slop forms and how
to catch it; CLAUDE.md points there rather than restating it.
No public export, route, diagnostic code, or runtime behavior changes.
Verified: build, typecheck, lint, test:unit (3016), test:route-unit (44),
test:projection (145).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The browser log client re-declared devLogKinds, devLogLevels, devLogProducers,
hasControlOrSeparators, and safeContextKeys, all of which contracts/dev-logs
already exports and which log-client already imported types from. The copy had
drifted: its `hook` kinds omitted lifecycle.replay.{started,completed,failed}
and its context allow-list omitted routeId, so every record emitted by
lifecycle-replay-service failed isDevRecord on the kind and isContext on the
context, and never reached the Logs view. The client now imports the shared
vocabulary; safeContextKeys moves into the dependency-free dev-log-kinds module
so the service and the client read one table. A regression test covers all three
lifecycle kinds with their routeId context — it fails against the previous
client.
Also removes symbols nothing referenced: abortableNdjsonStream and its
NdjsonStream interface, decodeRequestContextProvenance, expectCanonicalPayload
(internal to rsc-runtime, absent from its exports map), the superseded AuditShelf
wrapper — audit_library composes AuditSummary and AuditFileCards with the async
LibraryAnalysis instead, and the curator README said so already — five orphaned
type aliases, and the unused withWorkbenchProjectServer e2e helper.
The six workbench client suites now share tests/support/recording-fetch.ts
rather than each carrying a private copy. That support module was itself an
unadopted extraction and had gone stale: its session reply predated cookieName.
It now carries the current reply plus the superset that records body and signal.
Verified: build, typecheck, lint, test:unit (3017), test:route-unit (44),
test:projection (145).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ssifier project-client registers one SSE listener per entry in its own projectEventTypes list, and that list omitted 'dev.host.sync' even though host-install-manager publishes it and events.ts admits it, so those events never reached the browser. Adds it. The two lists stay separate on purpose: 'replay.gap' is a client-only wire type that ProjectEventPayloadMap cannot describe, so unifying them would widen server admission. playground-store carried a private sensitiveKey and a byte-identical copy of the provider-credential regexes rather than using core/credentials, whose header names playground traces among the surfaces sharing its one definition. The local key classifier dropped the providerKeyPatterns clause; the segment heuristic subsumes those patterns today, so this is drift risk rather than a live gap, but it is the exact divergence the comment warns about. Now imports isCredentialKey and containsProviderCredential. Removes six more modules with no importer, each an extraction whose caller was never switched over: native-playground-catalog (the live catalog publishing lives in native-playground-service), playground-values, playground-protocol (reachable only from playground-values), playground-durability, eval-service-types, and run-store-types. native-playground-catalog read as reachable because 'native-playground-catalog' is also an EpochCleanupResource string literal and a test tmpdir prefix. Verified: build, typecheck, lint, test:unit (3018), test:route-unit (44), test:projection (145). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019pNZFtdRajKNVDJtSSN71o
🦋 Changeset detectedLatest commit: 5a66185 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38231ecc94
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| --- | ||
| "agent-bundle": patch | ||
| --- |
There was a problem hiding this comment.
This commit adds three changeset files for one PR even though the repository requires exactly one, and all three list only agent-bundle despite the change to packages/rsc-runtime/src/state/contract.ts. Consolidate the entries into one changeset that also includes @agent-bundle/runtime, and rewrite its summary to use the required user-facing (#PR) format; otherwise the release metadata will contain three separate agent-bundle entries while omitting the changed runtime package.
AGENTS.md reference: AGENTS.md:L97-L105
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in 5a66185: the three changesets are consolidated into one .changeset/deslop-dead-modules.md that lists both agent-bundle and @agent-bundle/runtime (patch), with a user-facing summary ending in (#451). Note expectCanonicalPayload was never re-exported from state/index.ts, so the runtime entry is a patch, not a breaking bump.
…d include @agent-bundle/runtime Codex review on #451: one PR, one changeset; the rsc-runtime package changed (dead expectCanonicalPayload removed) so it is listed too.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a66185896
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Workbench Logs view now shows `lifecycle.replay.started`, `.completed`, and | ||
| `.failed` Dev Log records and records carrying `routeId` (the browser log | ||
| client's private copy of the `agent-bundle/contracts/dev-logs` vocabulary had | ||
| omitted them); the Workbench now subscribes to `dev.host.sync` project events, | ||
| which `project-client` had left out of its SSE listener list; and the |
There was a problem hiding this comment.
These lines announce user-visible changes to the Workbench Logs view and live dev.host.sync handling, but this commit does not update the matching English or Chinese Workbench documentation. Add the behavior to both website/docs/en/** and website/docs/zh/** so the public documentation ships with the changes it describes.
AGENTS.md reference: AGENTS.md:L71-L77
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
No docs change is needed here: both fixes restore behavior the site already documents rather than adding new behavior. website/docs/{en,zh}/guide/development/workbench.mdx describes the Logs view as grouping hook-producer events (which is where lifecycle.replay.* records belong) and documents dev.host.sync — including the AB7202 diagnostic it carries — as an event the Workbench receives; the Workbench simply failed to show those records before. The changeset entry names the fix because it is user-visible in the release notes, but the documented contract is unchanged, so this is a bug fix against the existing pages, not a new page section. (PR is merged; replying here per the merged-PR thread rule.)
1 participant
Summary
Removes 19 modules that nothing imported (3,869 lines), collapses the duplicates that survived onto their canonical owners, and fixes three defects those duplicates had caused by drifting. Net: 62 files, +300 / −4,660. No public export, route, or diagnostic code changes.
Dead modules
Every one had zero importers in production and test, and every one was born the same way: an extraction refactor lifted helpers into a new file and never switched the original over, so the monolith kept its inline copy and the new file was unreachable from its first commit.
dev/agent-api-wire,dev/epoch-staging,dev/eval/{eval-artifact-reader,eval-event-subscription,eval-service-error,eval-service-types},dev/mcp-apps/mcp-app-protocol,dev/playground/{native-playground-catalog,native-playground-evidence,playground-close-errors,playground-durability,playground-protocol,playground-store-codec,playground-store-layout,playground-subscriptions,playground-values},eval/{run-store-codec,run-store-types}, and workbenchmcp/mcp-session-trace-client.Two were invisible to reference scans:
epoch-stagingonly matched asSymbol('epoch-staging'), andnative-playground-catalogonly as anEpochCleanupResourcestring literal.Consolidated duplicates
dev/http.tsalready owned the request/response helpers andartifact-routesalready used it;eval-routes,mcp-app-routes,runtime-routes, andforeground-servereach kept a private clone including its ownRequestDiagnostictype. All four now import it.mcp-app-bridgetakes its validators frommcp-app-action-validation; the conventional-entry probe copied to dodge an import cycle moves to the leaf moduleconfig/conventional-entry.ts.tests/support/recording-fetch.tsinstead of six private copies — that support module was itself an unadopted extraction and had gone stale.effect/boundary.ts, one cross-package label table, two parallel validators with different diagnostic codes).Defects fixed
log-clientre-declared the vocabulary thatcontracts/dev-logsexports, and the copy had drifted: itshookkinds omittedlifecycle.replay.{started,completed,failed}and its context allow-list omittedrouteId. Records fromlifecycle-replay-servicefailed both checks. Regression test added; it fails against the previous client.dev.host.sync.project-clientregisters one SSE listener per entry in its own list, which omitted that type. The lists stay separate on purpose —replay.gapis client-only.core/credentials, whose header names playground traces among the surfaces sharing its one definition. The gap was drift risk rather than a live leak (the segment heuristic currently subsumes the omitted patterns), but it is exactly the divergence the comment warns about.Guidance
AGENTS.mdgains a Code hygiene section written from these specific failures;CLAUDE.mdpoints there rather than restating it.Deliberately left alone
packages/agent-bundle/fixtures/contracts/epoch-atomicity/local-linux.json— unreferenced, but two commits explicitly retain it as spike evidence.docs/superpowers/**,docs/plans/**,.superpowers/**— 16k lines of unlinked plan docs; owner chose to keep them.examples/mcp-apptest:browser-app— a documented browser test with no runner. Wiring it intocheckadds Playwright toexamples:check; needs an owner call.Verification
pnpm build,pnpm typecheck,pnpm lint,test:unit(3018),test:route-unit(44),test:projection(145),test:integration:run(975, on the first commit). Rebased ontomainat #449.🤖 Generated with Claude Code
https://claude.ai/code/session_019pNZFtdRajKNVDJtSSN71o