I covered every file in my part. Line numbers are as of HEAD `622e7a01`; four commits landed while I was reading (`engine/files.py`, `controllers/base.py`, `commands/http.py` and a few more), so I re-checked the line numbers I cite.

# Refactor plan: engine, controllers, resources, commands, surfaces, migrations, top-level `src/*.py`

**Files read.** Each was read whole unless a range is given.
- **engine:** `__init__`, `attic`, `bus`, `chat`, `clock`, `collecting`, `color`, `fields`, `files`, `focus`, `gates`, `given`, `heal`, `inputs`, `keeper`, `markers`, `organization`, `package`, `paths`, `proc`, `project_files`, `ran`, `reach`, `record`, `runtime`, `seats`, `services`, `sessions`, `shell`, `state`, `stop`, `stored`, `transcript`, `typist`, `version`, `viewer`, `wording`, `worker`, `worktree`, and `events/{base,agents,engine,resources}`.
- **controllers:** all 29 files.
- **resources:** `base`, `types`, `shapes`, `pictures`, `text`.
- **commands:** `cli`, `dispatch`, `parser`, `menu`, `queries`, and `http` (all 1165 lines, in ranges).
- **surfaces:** all 6 files.
- **top level:** `__main__`, `journal`, `worker`, `channel`, `serve`, `supervisor`, `skills`, and `install` (in ranges).
- **migrations:** `__init__` read whole. For all 62 migration modules (m0000–m0061) I read the imports; these I read whole: m0000, m0001 (first 143 lines), m0009, m0011, m0014, m0019, m0020, m0022, m0043, m0060, m0061.

A scan over all git-tracked files for names that nothing references found five unused items; they are listed under B1 and B2. One item uses only two of the reader's four labels: the `Controller.action` guard (B1-14) is a latent bug and is marked as such.

## Two findings that change behaviour, for you to decide

1. **The "no such action" guard can be forced** (`controllers/base.py:271-272`). `action()` refuses unknown or `_private` names with `self._refuse`, which `--force` turns into a warning. With a reason given, a forced call carries on to `getattr(self, name)`. That gives an `AttributeError`, or calls a private method such as `_remove`. Today the CLI parser blocks this, but the guard itself is open. The fix is `raise Refused(...)` at line 272, and the same at line 266 in `method()`. Low risk; it changes behaviour only for forced calls.
2. **Forced and hard refusals are mixed with no rule.** Some policy refusals raise `Refused` directly, so `--force` cannot override them:
   - `questions.py:32`
   - `todos.py:37` and `todos.py:62`
   - `environments.py:41` and `environments.py:50`
   - `messages.py:33`

   Their neighbours use `_refuse`. The policy should be: `_refuse` is for rules a user may force, and `Refused` is for input that is invalid or impossible. Reclassifying the rows above changes what `--force` can override, so it needs your ruling.

## B1 — Controller funnels (do first; highest value)

**Files:** `controllers/{base,stored,files,links,messages,agents,works,questions,docs,reports,facts,rules,notices,notifications,faults,todos,prioritised}.py`, a new `controllers/discussion.py`, and `resources/{base,types,shapes}.py`.

1. **One persist funnel.** `Controller.save` (`base.py:103-116`) is the only path that runs write, then `_reindexed`, then emit. Three paths bypass it:
   - `read_all` (`base.py:314-330`) calls `_write_file` and `record.emit` itself.
   - `force_delete` (`base.py:284-290`) emits by itself.
   - `Agents.subagent` (`agents.py:32`) emits by itself.

   Add `Stored._persist(r)`, which writes and reindexes. `save` becomes checks, then `_persist`, then emit; `read_all` uses `_persist` per row and one emit. No behaviour change; low risk, and `read_all` gets faster.
2. **One "change data then save" funnel.** `stamp` (`base.py:201-205`) and `Agents.saw` (`agents.py:26-29`) have the same body (load, `data.update(_shaped)`, save) and differ only in the action name. Add `Controller._changed(n, action, data, **event)` and use it in both. Small behaviour change: `saw` currently runs without the record lock and would gain it.
3. **Row naming is written in five places.** The `f"{n:03d}"` row folder and `.md` name appear at `stored.py:41`, `stored.py:67`, `files.py:18` and `base.py:287`. Add `Stored._row_folder(n)` and `member(n)` and use them everywhere. No behaviour change.
4. **One file-stamp function.** `f"{st_mtime_ns}-{st_size}"` is written four times (`stored.py:170, 212, 223, 235`). Add `stamp_of(stat)`. No behaviour change.
5. **One "part of another row" test.** `row.get(PART_OF) or row.get(DRAFT_OF)` is repeated at `stored.py:132, 142, 153, 171`. Add `is_part(row)`. No behaviour change.
6. **Use `_folder()` everywhere.** `self.record.folder(self.type, self.resource.scope)` is written out by hand at `stored.py:108, 113, 117, 158, 181, 334`, `base.py:359` and `files.py:18`, even though `_folder()` exists at line 70. No behaviour change.
7. **Storage reaches up into faults.** `stored.py:257-259` imports `controllers.faults` inside a function and string-tests `type != "notice"`. Replace this with a hook method `Controller._damaged(path, error)` that calls `faults.damaged`, and have `Notices` override it as a no-op. No behaviour change.
8. **Dead code to remove:**
   - `Stored.moved` (`stored.py:112`) has no callers.
   - The `idempotency` clause in `_twin` (`base.py:144-155`) can never decide anything, because `Messages.create` (`messages.py:16-21`) already returns any message with the same key.
   - `resources/base.py`: `CLEARINGS` (line 26) and `EAGER` (line 35) are unused.
   - `AgentRow` redeclares `Field(dispatcher)` (`types.py:324`), which `Resource` already declares (`base.py:167`).

   No behaviour change.
9. **Building a throwaway resource just to read a typed field.** `self.resource(data=self._shaped(data)).X` appears at `works.py:13`, `questions.py:19` and `questions.py:31`. Add `Controller._given(data) -> Resource`. No behaviour change.
10. **One "comment on a row" funnel.** `Links.comment` (`links.py:27`) and `Messages.reply` (`messages.py:66`) both save the parent with `"commented"`. Add `Links._mark_commented(n, made)`.
    - Also: `comment`, `comments` and `react` are not links. Move them, with `FACES` and the duplicate `TWICE_WITHIN` (`links.py:5`, which repeats `base.py:28`), to a new `controllers/discussion.py` mixin named `Discussed`.

    No behaviour change.
11. **One "copy into another type" funnel.** `Reports.doc` (`reports.py:10-17`) and `Facts.promote` (`facts.py:30-34`) both create the row in the target type, copy it across, then complete the source with "became/promoted to …". `Docs.file` (`docs.py:40-48`) repeats the same create-then-sections loop as `Reports.doc`. Add:
    - `Controller._created_with_sections(...)`
    - `Controller._became(n, target_controller, how)`

    `Docs.supersede` (`docs.py:57-61`) should share one `_supersede` with `base.create`'s `supersedes=` branch (`base.py:175-177`). No behaviour change.
12. **One "find or create a notice" funnel.** `faults.once` and `faults.over` (`faults.py:40-53`) and `Rules.pin` (`rules.py:15-19`) all look for a standing notice and create one if it is missing. Move this into `Notices.raise_once(title, brief, key, **data)` and `Notices.close_titled(title, how)`. Then `faults.py` keeps only crash logging and formatting. No behaviour change.
13. **Docs "hidden" is undeclared.** `Doc.indexed=("hidden",)` (`types.py:123`) refers to a field that is never declared. As a result:
    - `Docs._standing` and `Docs.search` (`docs.py:34-38`) read `r.data.get("hidden")` directly.
    - `_standing` drops the `closed_since`/`closed_last` parameters, breaking the base signature.

    Declare `Field(FLAG, False, name="hidden")` on `Doc`, use `r.hidden`, and keep the override signature-compatible. No behaviour change. Setting `hidden_listed = False` instead would also hide hidden docs in the viewer list (`http.py` `_listed`), which is a behaviour change.
14. **Mutable module cache in the wrong owner.** `facts.SENDERS` and `facts.sender()` (`facts.py:7-19`) never invalidate their cache. They answer an `Environment` question, so move them to `Environments.launched_from(env)`. Change: the value would be re-read instead of cached for the process lifetime. Low risk.
15. **Small duplicates:**
    - `shapes.check` (`shapes.py:51`) re-implements `priority_level` (`shapes.py:27-33`).
    - `prioritised.py` is a 6-line mixin used by `Todos` and `Tickets`. It may stay, but its `priority_level` should be the one level mapping.

    No behaviour change.

## B2 — Engine plumbing (independent of B1)

**Files:** `engine/{runtime,paths,record,bus,clock,ran,chat,files,services,viewer,inputs,stop,heal,gates,worktree,typist}.py`, a new `engine/ports.py`, `controllers/{environments,types}.py`, `surfaces/updates.py`, `supervisor.py`, `install.py`, `channel.py`.

1. **One runtime-path owner.** `engine/runtime.py` has `folder()`, but runtime paths are built by hand in many places:
   - `services.runtime()` (`services.py:27`), which shadows the module name
   - `viewer.py:39, 95, 112, 229`
   - `inputs.py:50, 63`
   - `stop.py:11`, `heal.py:12`, `gates.py:28`, `faults.py:26`
   - `runtime.py:71` and `runtime.py:107`, which bypass `folder()` inside the same module
   - `record.py:231`, which duplicates `runtime.sessions`
   - `updates.py:24, 46, 52, 83` and `http.py:276-279`
   - the `"upgrading"` mark, written a second time in `http.py:279` and `install.py:297`

   Give each path a named function in `runtime.py` and call it everywhere. No behaviour change.
2. **One `free(port)`.** `services.py:93` and `viewer.py:153` are identical. Move it to `engine/ports.py`, and add `url_of(port)` there for the `f"http://127.0.0.1:{port}/"` built at `viewer.py:94, 115, 173` and `http.py` `identity_at`. No behaviour change.
3. **One transient-event funnel.** `Event(0, …)` is hand-built and sent with `bus.emit` in `clock.py:10`, `ran.py:12`, `chat.py:13`, `chat.py:16` and `files.py:220`. `record.py:114` and `record.py:126` build `Event` twice inside `emit`. Add `bus.announce(record, type, n, action, actor, **data)`. `chat.send` keeps `bus.run` for its synchronous "sending" step. No behaviour change.
4. **One environment-home function.** `engine/paths.environment_path` (`paths.py:18`) is only an alias of `contained`. Callers re-type `root / "environments"` and call it purely to validate:
   - `environments.py:80, 157, 168`
   - `record.py:70`
   - `dispatch.py:138`

   Replace it with `paths.environment_home(root, name)`. Move `controllers/types.environment_records` (`types.py:45`) to `engine/record.Record.every(root)`, and keep a re-export because migrations m0055–m0061 import it. No behaviour change.
5. **Dead code:**
   - `services.py:133`: an unreachable `return True` inside `specs`.
   - `files.tracked` (`files.py:113`): no callers.
   - `viewer.ensure` (`viewer.py:251`): no callers.
   - `worktree.workspace_removed` (`worktree.py:115`): no callers.
   - `updates.py`: unused `import subprocess` and `PACKAGE`.
   - `stale()` (`updates.py:44`) duplicates the inline check in `upstream()` (`updates.py:53-57`); make `upstream` call it.

   No behaviour change.
6. **`@cache` on `files.repositories` (`files.py:84`) defeats the 300-second rescan.** Because of it, `REPOSITORIES` and `RESCAN_SECONDS` are dead.
   - Option A, no behaviour change: delete the time cache.
   - Option B: delete `@cache` so new repositories are picked up. This changes behaviour, so it is your call.
7. **`Record` does five jobs** (`record.py`): folders and locks, the event log, cursors, state files and settings, with module-level mutable caches `RECENT`, `SETTINGS` and `SETTINGS_VERSION`.
   - Extract an `EventLog` class into `engine/event_log.py` (emit, events, recent, lines_back, trim, cursors).
   - Extract `SettingsFile` into `engine/settings_file.py`.
   - `Record` composes them and keeps its current methods as delegates.
   - `parsed()` (`record.py:269`) duplicates the try/except in `events_back` (`record.py:162-165`); merge them into `parse_event(raw)`.
   - Derive `Record.SETTINGS` (`record.py:54`) from the `Setting` descriptors instead of listing the names a second time.

   No behaviour change; medium risk, because `record` is used everywhere.
8. **`worktree.git`** (`worktree.py:219`) re-implements `proc.ran` with a 60-second timeout. `worktree.ignored` (`worktree.py:273`) calls `subprocess.run` with no error guard. Route both through `proc.ran`. The only change is that `ignored` stops raising on `OSError`.
9. **`files.repositories` (depth 2) and `worktree.repositories` (depth 1) share a name but follow different rules.** Rename them to `nested_repositories` and `top_repositories`. No behaviour change.
10. **`Environments`:**
    - `Sessions(self.record.root)` is built at `environments.py:54, 62, 90, 178`.
    - `self.sessions()` is called repeatedly, creating a new object each time.
    - `Record(root, env.title)` is built at lines 117, 130, 170 and 183.

    Add a cached `self._sessions` and `self._record_of(env)`. `Record(...)` at line 82, which is called only for its side effect, becomes the explicit `environment_home(...).mkdir`. No behaviour change.
11. **`runtime.default_env(root, prefer)`** (`runtime.py:125`) is just `prefer or env(root)`, and almost every one of its 22 callers passes no `prefer`. Replace those calls with `runtime.env`. No behaviour change; it touches feature files, so it is only for a worker cleared to edit them.
12. **The supervisor rebuilds the typist socket path and listener.** `supervisor.py:144-163` duplicates `typist.path`, `listen` and `receive`. Import `engine.typist` (with `sys.path` set up as `keeper.py` does), adding a buffer-size parameter. Medium risk: the supervisor is long-lived across upgrades.
13. **`install.py`:**
    - `PACKAGE_DIRS` (line 19) restates `PACKED_DIRS` (line 34) plus two names; derive it.
    - The `VERSION` read is repeated at `install.py:277, 336, 359, 403`; add a local `version_in(folder)`.
    - `install.py` is standard-library-only by design, so keep these helpers local.
    - `"journal.pyz"` is spelled out in 7 places; use the one constant `engine.package.ARCHIVE` (the engine and `channel` already can).

    No behaviour change.

## B3 — HTTP surface slimming (independent of B1 and B2)

**Files:** `commands/{http,dispatch}.py`, `surfaces/{summary,agent_state}.py`, new `surfaces/{listing,settings,attachments}.py`, new `providers/transcripts.py`. `http.py` is 1165 lines and holds business logic and read models.

1. **Read model out of the router.** `listing`, `_listed`, `readable`, `viewed`, `counted` and `_tally`, plus the caches `LISTED`, `VIEWED`, `TALLIED` and `ATTACHED` (`http.py:1004-1086`), move to `surfaces/listing.py`. `listed_attachments` and `attached_file` (`http.py:613-640`) move to `surfaces/attachments.py`. No behaviour change.
2. **One unread/open count.** `_tally` (`http.py:1067`) and `summary.environment` (`summary.py:107`) count rows the same way. Make `surfaces.listing.counts(controller)` the one funnel. No behaviour change.
3. **Settings policy out of the router.** `renamed`, `switches` and `settings` (`http.py:61-79`), plus the body of `post_settings` (`http.py:443-469`: switching feature rows, rebuild, nudge), move to `surfaces/settings.py` as `apply(record, body, actor)`. No behaviour change.
4. **One transcript resolver.** "Provider plus `row.transcript`" is resolved four times:
   - `http.transcript_at` (`http.py:730`)
   - `queries.transcript` (`queries.py:25-30`)
   - `queries.environment_transcript` (`queries.py:48-64`)
   - `summary.last_written` (`summary.py:22-25`)

   Move the `Transcript`/`NoTranscript` classes from `http.py:711-727` into `providers/transcripts.py` with `transcript_of(row, session=None)`. `queries.py` (owned by B4) adopts it after B3 merges. No behaviour change.
5. **Engine and feature logic sitting in routes:**
   - `unanswered` (`http.py:85-103`), which logs hook failures, goes to `runner/hooks.py` next to `answer`.
   - The folder listing in `get_project_files` (`http.py:674-693`) goes to `engine/project_files.list_folder`.
   - `get_commit` and `get_file_diff` (`http.py:897-933`) go to an `engine/git_view.py`.
   - `get_pages` (`http.py:815`) goes to `features/plugins`.
   - `PROBED` and `probe` (`http.py:524-536`) go to `engine/viewer.known_journals()`.

   No behaviour change.
6. **One way to refuse a request.**
   - `post_identity` (`http.py:310`) and `post_agent_hooks` (`http.py:417`) hand-build `Reply(400)` from a `ValueError`; raise `Refused` so `dispatch` maps it.
   - `get_agent_screen` and `post_agent_keys` (`http.py:353-370`) repeat the terminal lookup and 404; add a `terminal_or_missing(req)` that raises `Missing`.

   No behaviour change.
7. **Feature lookups in the router.** The `dev_faults` feature is looked up by name at `dispatch.py:126`, `dispatch.py:155`, `http.py:255` and `cli.py:164`. Add one `features.profiling()` accessor; the `cli.py` call site belongs to B4. Also:
   - `get_changelog` (`http.py:271`) imports `install.code`; use `engine.package.code`.
   - `get_changelog` re-imports `version` and `newer` locally.

   No behaviour change.
8. **Split the route modules** into `commands/routes/{agents,plugins,files,settings,rows,system}.py`, imported by `commands/http.py`. Do this after items 1–7, so the files hold routing only. Low risk.

## B4 — CLI routing (independent of B2 and B3; two items wait for B1)

**Files:** `commands/{cli,parser,queries,menu}.py`, `skills.py`, `engine/sessions.py`, new `agents/launch.py`.

1. **The launch flow is in the commands layer.** `asked_for`, `asked_slate`, `asked_prompts`, `asked_history`, `asked_resume`, `choose`, `defaults`, `banner`, `supervise` and `started` (`queries.py:152-349`, about 200 lines) move to `agents/launch.py`. `queries.supervise` becomes a one-line route. No behaviour change.
2. **Other business logic in `queries.py`:**
   - `decided` (`queries.py:423`) moves to `Agents.decide(why, session)`. This waits for B1, which owns `agents.py`.
   - `still_open` and `halt` (`queries.py:129-150`) move to `engine/stop.py` or the clean-slate feature.
   - `services` and `services_up` (`queries.py:378-412`) move to `engine/services.py`.
   - `verify` (`queries.py:92`) duplicates `seats.seats(root, within=10)` and re-imports `PROVIDERS` at line 85.

   No behaviour change.
3. **Write gate in the wrong layer.** `sessions.allowed` (`sessions.py:190-197`) makes `engine/sessions.py` import `resources.types` just to apply a policy. `cli.py:56` hard-codes `READS`, including the feature word `"board"`, and `cli.py:57` hard-codes `LOCAL = {"browser"}`. Replace these with:
   - a `@reading` marker next to `@internal` and `@lasting` in `controllers/marks.py`
   - `Controller._subagent_may_write(sessions)`, called from `cli.run`

   This waits for B1 to merge, because `marks.py` and `base.py` are B1's files. No behaviour change, provided every name in `READS` gets the marker.
4. **One "words of a type" funnel.**
   - `{*actions(c), *COMMANDS.get(type, {})}` is written at `parser.py:24`, `parser.py:102`, `skills.py:38` and `base.py:271`.
   - `COMMANDS.get(type).get(name) or getattr(controller, name)` is written at `parser.py:32`, `skills.py:21` and `base.py:33`.

   Add `controllers.base.words(controller)` and `command_fn(controller, name)`; the `base.py` side lands with B1. `skills.signature` (`skills.py:20-31`) and `parser.add_method` (`parser.py:29-49`) walk the same signature in two places; give them one parameter-description function. No behaviour change.
5. **Smaller items:**
   - `parser.built` calls `features.load()` a second time (`parser.py:98`).
   - `cli.context` (`cli.py:39-53`) mixes bootstrap (migrations, `features.load`) with a five-way environment fallback. Split it into `engine.runtime.resolve_env(...)` and a one-time bootstrap.

   No behaviour change.

## B5 — Migrations: can the 63 be squashed into one baseline?

**Yes, but only with a version floor. A plain squash is not safe.**

How the runner works and what the migrations are:
- `run_locked` (`migrations/__init__.py:91-113`) runs every module whose name is missing from `migrations.json`.
- Four kinds of migration exist:
  - **Data reshapes:** m0001, m0007, m0008, m0010, m0012, m0015, m0017, m0024 and others.
  - **One-shot overrides of user settings:** m0013, m0014, m0019, m0060, m0061. These must never run twice; m0060 would switch auto mode back on over a user's later choice.
  - **Content re-ships:** 27 modules (m0022, m0028–m0049, m0051–m0054) are only `shipped(root, ship, "sequences")`. m0009 and m0025 re-ship templates.
  - **Housekeeping:** m0004, m0011.

So a single baseline cannot both:
- (a) bring a behind install up to date, which needs the full chain, and
- (b) be a no-op on current installs, which need nothing re-run.

What a safe squash needs:
1. **Pick a floor release.** v2.249.x is the latest tag. Installs whose ledger lacks any of m0000–m0061 must first upgrade through the floor. That means either `install.upgrading` fetches the floor tag when the ledger is behind, or `run_locked` raises `Refused("upgrade through 2.249 first")`. How far back to support is your decision; m0001 still converts the version 1 record.
2. **The baseline is `m0062_baseline`.** It is a no-op on a fresh record and checks the floor on an existing one. The old ledger keys stay, because `migrations.json` is never pruned.
3. **Turn re-shipping into a step.** Replace the 29 re-ship modules with one ship on version change (sequences and templates), next to `updates.announce`. `install.finish` already ships sequences (`install.py:382`), but templates are not shipped there.
4. **Fix the dependents:**
   - `features/auto_update/launch.py:26` reads the ledger's `m0000_project_resources` result. It still works after the squash, but it is dead weight for installs past the floor.
   - `features/auto_update/test.py:176`, `features/revisions/test.py:41` (which imports m0024) and `features/open_viewer/test.py:133` need updating.
   - m0000 imports m0010, and m0020 imports m0050 (a forward reference). These go away with the squash.
5. **This is also the main reason to squash.** Old migrations run today's code against old data. Many import live APIs, and every refactor above must keep them importable:
   - `controllers.types` and `CONTROLLERS`
   - `controllers.stored.DRAFT_OF`
   - `Resource.dump`
   - `features.tickets`, `features.plans.progress`, `features.collections`
   - `environment_records`

   Risk is high; this is a behaviour change for installs older than the floor. Do it before B6, or have B1 and B2 keep re-exports.

## B6 — Cross-cutting follow-ups (after B1–B4 merge)

1. **Underscore methods used as public API.** These are called from other modules: `_standing` (106 calls), `_every` (52), `_titled` (43), plus `_to_primary`, `_logged`, `_installed`, `_session_or_primary`, `_shared`, `_text` and `_remove`. The underscore exists only to keep them out of the CLI, because `actions()` exposes every public function.
   - Rename them to `standing`, `every`, `by_title`, `nudge_primary` and so on, and mark them `@internal`.
   - `tests/test_every_action.py` loops over `actions()`, so anything renamed must carry `@internal` or it becomes a CLI action.

   No behaviour change; medium risk, because about 250 call sites in `features/` change.
2. **`Environments.sweep`** (`environments.py:141-148`) reaches into another controller's `rows._text` and `rows._remove` and writes with a bare `write_text`. Move this into `Stored.export_to(stage, n)`. This needs B1's `stored.py` and B2's `environments.py`, hence this batch. No behaviour change.

## Order

B1 and B2 can run in parallel; then B3 and B4 (B4 items 2-decided and 3 wait for B1); then B5 after your floor decision; then B6. Each worker should finish with `journal check sweep` (`scripts/checks/funnels.py` flags bodies written twice) and the generated tests.

### Critical Files for Implementation
- /Users/jessegall/projects/agent-journal/src/controllers/base.py
- /Users/jessegall/projects/agent-journal/src/controllers/stored.py
- /Users/jessegall/projects/agent-journal/src/commands/http.py
- /Users/jessegall/projects/agent-journal/src/engine/record.py
- /Users/jessegall/projects/agent-journal/src/migrations/__init__.py