Six leftovers from the simplification refactor, one commit each - #1571
Merged
Conversation
`src/index.ts` exported 341 names. Nothing imports this package by name, nothing inside `src/` imports the barrel, and `bin.ts` — the only thing that ships — takes `runCli` from `cli.js` directly. Its consumers were the dashboard, which imports it 85 times and every one of them `import type`, and the dev-mode Vite plugin, which pulled the whole barrel out of `dist/index.js` to get `runDaemon`. 269 of the 341 were referenced nowhere outside the module that defines them. That is the same shape A7 dissolved one level down — a 406-symbol export surface with one consumer — surviving at the top. It costs more than tidiness: an exported name reads as "in use" to anyone grepping, so 269 of them made every future deletion pass start by proving a negative. That fog is where the `run.json` and `sessions/` back-reads sat unnoticed until someone went looking. So the file is now the 41 types the dashboard actually names, grouped by the module that defines each, and nothing else: - type-only, because a value re-exported here would pull its module into the browser bundle — the failure `client.ts` exists to prevent; - the dev plugin imports `dist/daemon.js`, the module that has `runDaemon`, rather than a barrel that had it among 340 others; - `main`, `types` and the `exports` map go with it. They described a library API for a package that publishes a CLI: `bin` and `files` stay, and what someone installs is unchanged. The rule the file now states about itself is the part worth keeping: every name in it is imported by a file under `dashboard/`, so a name nothing renders cannot quietly live on in it. Verified: both typechecks, a full build, 1445 node tests, 774 dashboard tests, and `dist/daemon.js` imported in a real Node process — the dev plugin's path is the one line here that no test covers. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PGg6YdQthErYV63HRVPzrw
…s only reader Correcting my own review as I fix it: I reported this field as written and never read, off a grep that covered `src/` and not `dashboard/`. It had exactly one reader — `isMetaPublishing` in `dashboard/lib/live-state.ts` — and its own comment says what it is: "the version gate is the old-records guard". That makes it the same class #1545 removed everywhere else, not dead weight. A meta written before #1455 folded `handoffReport` has no report even though its handoff answered long ago, so the gate stopped a list surface from reading that absence as "still publishing". With nothing carrying a pre-fold record, the gate only ever answers true, and the field only exists to be gated on. So both go: the guard, then `version` itself — the constant, the interface field, the three writes, and the fixtures that carried it through eleven suites. The rail's rule is now the one that describes today's records: an agent that ended, armed a push, and has not reported yet is still publishing. The test that pinned the pre-fold record's behaviour is gone with the behaviour; the case that replaces it is the live one — an agent that never armed a push has no window to wait on. Records already on disk keep an extra `version` key. Nothing reads it, and nothing rejects it: an unknown key in a parsed snapshot has never been an error. Verified: both typechecks, a full build, 1445 node tests, 774 dashboard tests. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PGg6YdQthErYV63HRVPzrw
`removeProject` had no caller: no RPC, no CLI path, no control in the dashboard. A project is registered by activating a repo and stays registered — the only thing that ever removed one was its own three tests. Its doc gave away how long it had been that way: "the project's own overrides (#840) go with it, so re-adding the path starts clean" — the tier that promise was about was deleted in B5. The token-survival test kept it as one of "the other registry mutators"; it now uses a preference patch, which is a mutator that exists. If unregistering a project is wanted, it is a feature with a surface, not a function waiting in a file for someone to find it. Verified: both typechecks, 1442 node tests, 774 dashboard tests. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PGg6YdQthErYV63HRVPzrw
`AGENTS_DIR` and `ARCHIVE_DIR` were both `'agents'`, both exported, both re-exported through the store barrel. D5 renamed `runs/` and `sessions/` into one name and left the two constants behind — in the module whose whole rename was about not having several words for one thing. What actually differs between the two placements is the user segment, not the name: `<user>/agents/` is the committed copy, top-level `agents/` is the transient one, and `archiveDir(dir, user?)` is where a caller picks. That is now what the one constant's doc says. Verified: both typechecks, 1442 node tests, 774 dashboard tests. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PGg6YdQthErYV63HRVPzrw
… called
`SESSIONS_RULE` is `ARCHIVE_RULE`. Its value was already built from `ARCHIVE_DIR`
— only the identifier still said `sessions`, which is the word D5 retired.
The same word survived in the prose around it, so this takes that with it: the
modules that write, commit and ignore the archive called it "the session
archive" throughout, and the line the install writes into every user's repo said
"session state is transient; the session archive is committed". It says agent
now, in the vocabulary MEMORY.md pins.
Two places still speak the old word and are deliberately left alone, because
changing them changes what a user sees rather than what a reader reads: the
commit message the committer writes ("[The Framework] a session"), and the
daemon's no-op reason ("no session changes"). Both are worth renaming; neither
is a comment.
Verified: both typechecks, 1442 node tests, 774 dashboard tests.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PGg6YdQthErYV63HRVPzrw
F3 replaced the daemon's `/api/*` reads with RPCs called by name, and the comments around them kept describing the routes: `GET /api/projects`, the `/api/docs` payload, "each read endpoint (`/api/logs`, `/api/runs`, `/api/docs`) resolves a `?project=<id>`", and a read model "backed by the same readers the daemon's legacy /api/* endpoints use". `daemon.test.ts` already called them retired. Same class as the ~93 Telefunc and Vike comments #1536 swept — these four were just missed, and a reader following them goes looking for a mount that isn't there. Verified: both typechecks, 1442 node tests, 774 dashboard tests. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PGg6YdQthErYV63HRVPzrw
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
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.
Findings 1–6 from a review of what #1536 left behind, each in its own commit. Finding 7 (SPEC structure) is a separate PR.
9e770be— the barrel ends where its consumers end: 341 exports become 41The headline.
src/index.tsexported 341 names. Nothing imports this package by name, nothing insidesrc/imports the barrel, andbin.ts— the only thing that ships — takesrunClifromcli.jsdirectly. Its consumers were the dashboard (85 imports, every oneimport type) and the dev-mode Vite plugin, which pulled the whole barrel out ofdist/index.jsto getrunDaemon. 269 of the 341 were referenced nowhere outside their defining module.That is the shape A7 dissolved one level down — a 406-symbol export surface with one consumer — surviving at the top. It costs more than tidiness: an exported name reads as "in use" to anyone grepping, so 269 of them made every future deletion pass start by proving a negative. That fog is where the
run.jsonandsessions/back-reads sat unnoticed.The file is now the 41 types the dashboard names, grouped by defining module, type-only (a value re-exported there would pull its module into the browser bundle — the failure
client.tsprevents). The dev plugin importsdist/daemon.js.main,typesand theexportsmap go with it: they described a library API for a package that publishes a CLI.binandfilesstay, so what someone installs is unchanged.49389b2— the meta'sversionfield, and the old-records guard that was its only readerCorrecting my own review here. I reported this field as written and never read, off a grep that covered
src/and notdashboard/. It had exactly one reader —isMetaPublishingindashboard/lib/live-state.ts— whose own comment calls it "the old-records guard".That makes it the same class #1545 removed everywhere else. A meta written before #1455 folded
handoffReporthas no report though its handoff answered long ago, so the gate stopped a list from reading that absence as "still publishing". With nothing carrying a pre-fold record, the gate only ever answers true — and the field exists only to be gated on. Both go.5f3ebd0— nothing removes a project, so nothing keepsremoveProjectNo RPC, no CLI path, no dashboard control; only its own three tests. Its doc gave away how long: "the project's own overrides (#840) go with it" — the tier that promise was about was deleted in B5.
286bd8d— one name for the archive directory, not two spelling the same stringAGENTS_DIRandARCHIVE_DIRwere both'agents', both exported, both re-exported. What differs between the two placements is the user segment, not the name — which is what the surviving constant's doc now says.b748fe4— the archive rule is named for the archiveSESSIONS_RULE→ARCHIVE_RULE; its value was already built fromARCHIVE_DIR. The sweep extends to the prose that carried the same retired word, including the line install writes into every user's repo. Two deliberately left alone, because they change what a user sees rather than what a reader reads: the commit message ([The Framework] a session) and the daemon's no-op reason (no session changes). Both worth renaming; neither is a comment.eec7ad1— four comments describe HTTP routes that no longer existGET /api/projects, the/api/docspayload,/api/logs+/api/runs, and a read model "backed by the ... legacy/api/*endpoints".daemon.test.tsalready called them retired.Verification
After
pnpm clean: both typechecks, a full build, 1442 node tests, 774 dashboard tests (81 files). The dev plugin's new import was additionally checked by importingdist/daemon.jsin a real Node process — that line is the one thing here no test covers.Two observations, not fixed here
update-check.tspolls npm for@gemstack/the-framework, which has never been published, so the CLI's "up to date?" footer silently reads "unknown" on every run. It degrades correctly; it just never says anything..the-framework/.gitignorestill carries the pre-D5 rules (!*/sessions/, plus!LOGS.mdand!conversations/from records B3 collapsed). The archives already tracked stay tracked, andensureArchiveIgnoredappends the current rule on the next agent run, so it self-heals — but until then a new user directory's archive would be ignored.Generated by Claude Code