docs: operator docs hardening (file mode flip, vocabulary, daemon-as-service)#228
Conversation
📝 WalkthroughWalkthroughAdds operator-facing docs for running middle as a service, a canonical label vocabulary plus a docs↔code vocabulary drift check, expanded file-mode operator docs and doctor checks, CLI support for in-process foreground start, and corresponding tests and cross-links. ChangesOperator Docs Hardening
Sequence diagram (foreground start flow): sequenceDiagram
participant CLI as mm start --foreground
participant StartCmd as runStartCommand
participant ForegroundRunner as runForegroundDaemon
participant Daemon as runDaemon (in-process)
participant ServiceMgr as systemd/launchd
CLI->>StartCmd: foreground=true
StartCmd->>ForegroundRunner: runForegroundDaemon()
ForegroundRunner->>Daemon: dynamic import + runDaemon()
Daemon->>Daemon: dispatcher runs (no fork, no pidfile)
ServiceMgr->>Daemon: SIGTERM
Daemon->>StartCmd: exit 0 on clean shutdown
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Verification gates — phase #216✅ All 4 verification gate(s) passed for phase #216.
format — ✅ pass (0.3s)lint — ✅ pass (0.1s)typecheck — ✅ pass (2.1s)test — ✅ pass (86.9s) |
Verification gates — phase #217✅ All 4 verification gate(s) passed for phase #217.
format — ✅ pass (0.3s)lint — ✅ pass (0.1s)typecheck — ✅ pass (2.0s)test — ✅ pass (94.6s) |
|
|
||
| ```bash | ||
| mm stop # if the dispatcher is running | ||
| mm init <repo-path> --epic-store=file # idempotent: re-init flips the mode |
There was a problem hiding this comment.
Decision (#216): documenting the existing-repo flip as mm init --epic-store=file, not a hand TOML edit. mm init writes both the .middle/<slug>.toml and the daemon-db repo_config.epic_store row the dispatcher's per-repo gateway router reads (readEpicStoreConfig); nothing re-derives the db row from the toml at boot or dispatch. A toml-only edit would flip the recommender's config-only read but not dispatch routing — a silent half-switch. mm init is idempotent and preserves committed policy. See planning/issues/209/decisions.md.
| * whole file-mode design rests on, surfaced where an operator can act on it. | ||
| */ | ||
| function checkEpicStore(store: EpicStoreConfig, opts: DoctorOptions): Check { | ||
| function checkEpicStore(store: EpicStoreConfig, opts: DoctorOptions): Check[] { |
There was a problem hiding this comment.
Decision (#216): checkEpicStore returns Check[] so file mode emits three granular rows (epics_dir / state_file / Epic-file round-trip) instead of one ambiguous line — matching the sub-issue's "run the file-mode checks". Only marker-led files are round-tripped, so the scaffold's own README.md (not an Epic) isn't falsely failed.
| * Exits 0 only when both hold. (Extra documented labels are allowed — the doc may | ||
| * cover operator conventions the code doesn't key on.) Returns the process exit code. | ||
| */ | ||
| export function runVocabularyCheck(opts: Pick<DoctorOptions, "vocabularyDocPath"> = {}): number { |
There was a problem hiding this comment.
Decision (#217): the recommender's classification is LLM-driven (no deterministic classifier to replay), so "docs and code agree" is realized as a coverage drift guard: every label the code deterministically keys on (NEEDS_DESIGN_LABEL, STATE_LABEL, middle-owned NON_FEATURE_LABELS) must be documented, plus the canonical vocabulary is complete. Rename a constant without updating the doc → this fails. The sub-issue explicitly authorized "an extended mm doctor flag". Full rationale in decisions.md.
| * owns start/stop/restart, so middle must not fork or leave a pid file the manager | ||
| * doesn't know about. Blocks until the daemon shuts down (SIGTERM → exit 0). | ||
| */ | ||
| async function runForegroundDaemon(opts: StartOptions): Promise<number> { |
There was a problem hiding this comment.
Decision (#218): --foreground bypasses the fork/pid-file path and runs runDaemon in-process so a service manager (systemd Restart=on-failure / launchd KeepAlive) owns the lifecycle. The daemon entry is pulled in via a dynamic import so the default background path never loads the dashboard/daemon modules; the runForeground seam keeps unit tests fast while the integration test boots the real binary. runDaemon already installs SIGTERM→drain→exit(0).
Reviewer's brief — Epic #209 (operator docs hardening)Three operator docs + their integration tests. All three sub-issues (#216, #217, #218) are closed; the branch is How to run itbun install
bun test # 1383 pass, 0 fail
bun run typecheck # clean
bun run lint # clean
# The three new operator-facing surfaces, against the real CLI:
bun packages/cli/src/index.ts doctor --vocabulary-check # ✓ docs and code agree — 6 code-keyed labels documented
bun packages/cli/src/index.ts start --help # shows --foreground
bun packages/cli/src/index.ts init --help # shows --epic-storeWhat to verify (and what "correct" looks like)
How to review
Fragile bits that want extra eyes
No follow-ups filed — scope delivered in full. Out of scope (sibling-noted): Windows service templates, Docker deployment. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/daemon-as-a-service.md`:
- Around line 81-110: The plist example contains literal "<you>" in XML text
nodes (seen in the ProgramArguments entry "/Users/<you>/.bun/bin/mm", the PATH
string, StandardOutPath and StandardErrorPath) which breaks XML parsing; update
those placeholders by escaping the angle brackets (e.g., replace "<you>" with
"<you>>" or better use a plain token like YOUR_USERNAME) so the example
is valid XML and can be parsed by launchctl/plutil.
In `@docs/operator.md`:
- Around line 72-76: The fenced code block containing the lines "planning/epics/
# one <slug>.md per Epic (README.md explains the format)", ".middle/state.md
# the ranked dispatch state (the file-mode \"state issue\")", and
".middle/<owner>-<name>.toml # the [epic_store] config above" should be
labeled with a language (e.g., tag the opening triple backticks as ```text) so
update that fenced block in docs/operator.md to use ```text instead of unlabeled
``` to silence markdownlint and keep the tree readable.
In `@packages/cli/src/commands/start.ts`:
- Around line 176-179: The early return on opts.foreground bypasses the pidfile
preflight in runStart; before calling runForegroundDaemon(opts) reuse the same
preflight/check logic that runStart runs (the pidfile live/stale check and
stale-pidfile cleanup) so we don't start a second daemon or leave stale files;
modify the start flow to call the existing preflight helper (or inline the same
checks used by runStart) prior to invoking runForegroundDaemon, referencing the
existing runStart preflight logic and the opts.foreground branch to ensure
identical behavior for foreground starts.
In `@packages/cli/test/start-foreground.test.ts`:
- Around line 77-79: Replace the hard-coded PORT constant (PORT = 41877) with
code that picks an ephemeral free port at runtime (e.g., using get-port or by
creating a short-lived net.Server and listening on port 0 to read
server.address().port), then write that discovered port into the same temp
config file the test uses before starting the daemon; ensure the test uses this
dynamic port wherever PORT was referenced (start-foreground.test.ts: PORT
variable and the temp config creation logic) so the daemon is launched and
asserted against the ephemeral port instead of a fixed one.
In `@planning/issues/209/plan.md`:
- Around line 60-61: The ATX heading "`#217`'s integration is realized as a
**docs↔code drift guard** via `mm doctor --vocabulary-check`, not a
deterministic replay of the recommender's *classification*:" is missing a space
after the hash and is flagged by markdownlint (MD018); fix it by inserting a
space so the heading reads "# 217's integration ..." (i.e., change "`#217`'s
integration..." to "# 217's integration..."), keeping the rest of the line
including the inline code `mm doctor --vocabulary-check` and emphasis intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ed0266d5-a94d-4152-88ab-01808daddb0c
📒 Files selected for processing (17)
README.mddocs/daemon-as-a-service.mddocs/operator.mddocs/vocabulary.mdpackages/cli/src/bootstrap-assets/skills/creating-github-issues/SKILL.mdpackages/cli/src/bootstrap-assets/skills/implementing-github-issues/SKILL.mdpackages/cli/src/bootstrap-assets/skills/recommending-github-issues/SKILL.mdpackages/cli/src/commands/doctor.tspackages/cli/src/commands/start.tspackages/cli/src/index.tspackages/cli/test/doctor.test.tspackages/cli/test/start-foreground.test.tspackages/skills/creating-github-issues/SKILL.mdpackages/skills/implementing-github-issues/SKILL.mdpackages/skills/recommending-github-issues/SKILL.mdplanning/issues/209/decisions.mdplanning/issues/209/plan.md
|
🔁 Reconciled with main (merged-new-work-as-base) after 128056e |
c78e1cf to
64be4ae
Compare
Reviewer brief — #228 (Epic #209 operator-docs hardening)This PR was stale + DIRTY for a few hours after #229 + #231 merged; the rebase was a single How to run itgh pr checkout 228
bun install # populate the worktree's node_modules
bun run typecheck # clean
bun run lint # clean
bun test packages/cli/test/doctor.test.ts # 30 / 30 pass, the merge surface
bun test # 1467 / 1467 full sweepWhat to verify
Fragile bits
|
…hecks Add the 'Enable file mode on an existing repo' how-to to docs/operator.md: the mm init --epic-store=file flip (not a hand TOML edit), the directory layout, a round-tripping worked-example Epic file, and the operator-visible differences. Extend the doctor file-mode check to three rows — epics_dir exists, state_file present, and every marker-led Epic file under epics_dir round-trips byte-for-byte — and assert it in doctor.test.ts by lifting the worked example straight from the doc as the fixture, so docs and code can't drift apart (#216).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/cli/src/commands/start.ts (1)
6-6: ⚡ Quick winAdd a type-level TSDoc block to the exported
StartOptions.The properties are documented, but the public export itself is missing a contract-level TSDoc/JSDoc comment.
Proposed patch
+/** + * Options for `mm start` execution modes and startup side effects. + * Guarantees: + * - background mode uses pidfile-based lifecycle (`mm stop`); + * - foreground mode runs in-process and writes no pid file. + */ export type StartOptions = {As per coding guidelines "Every public export in a module must carry a TSDoc/JSDoc comment. Comments must describe behavior and contracts — what it does, what it guarantees, what it assumes — not restate the identifier's name."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/commands/start.ts` at line 6, Add a TSDoc block above the exported StartOptions type that describes the contract and behavior of the options object (what it configures, guarantees, and any assumptions), not just restating the name; reference the StartOptions type and its existing properties (e.g., any timeout, env, port, etc.) in the description so callers know the effects and expectations of each field and whether fields are optional or have defaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/operator.md`:
- Line 122: Update the documentation text that currently reads "github mode" to
use the correct product capitalization "GitHub mode"; locate the string "github
mode" in docs/operator.md (the sentence describing mm run-recommender rewriting
.middle/state.md and dispatcher poller cadence) and replace it with "GitHub
mode" to ensure consistent naming throughout the doc.
---
Nitpick comments:
In `@packages/cli/src/commands/start.ts`:
- Line 6: Add a TSDoc block above the exported StartOptions type that describes
the contract and behavior of the options object (what it configures, guarantees,
and any assumptions), not just restating the name; reference the StartOptions
type and its existing properties (e.g., any timeout, env, port, etc.) in the
description so callers know the effects and expectations of each field and
whether fields are optional or have defaults.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9106b57b-1262-442f-b7d2-869eb4129c1e
📒 Files selected for processing (17)
README.mddocs/daemon-as-a-service.mddocs/operator.mddocs/vocabulary.mdpackages/cli/src/bootstrap-assets/skills/creating-github-issues/SKILL.mdpackages/cli/src/bootstrap-assets/skills/implementing-github-issues/SKILL.mdpackages/cli/src/bootstrap-assets/skills/recommending-github-issues/SKILL.mdpackages/cli/src/commands/doctor.tspackages/cli/src/commands/start.tspackages/cli/src/index.tspackages/cli/test/doctor.test.tspackages/cli/test/start-foreground.test.tspackages/skills/creating-github-issues/SKILL.mdpackages/skills/implementing-github-issues/SKILL.mdpackages/skills/recommending-github-issues/SKILL.mdplanning/issues/209/decisions.mdplanning/issues/209/plan.md
✅ Files skipped from review due to trivial changes (6)
- packages/cli/src/bootstrap-assets/skills/recommending-github-issues/SKILL.md
- planning/issues/209/plan.md
- packages/skills/creating-github-issues/SKILL.md
- docs/vocabulary.md
- packages/cli/src/bootstrap-assets/skills/implementing-github-issues/SKILL.md
- planning/issues/209/decisions.md
🚧 Files skipped from review as they are similar to previous changes (5)
- README.md
- packages/cli/src/bootstrap-assets/skills/creating-github-issues/SKILL.md
- packages/cli/src/index.ts
- docs/daemon-as-a-service.md
- packages/cli/test/doctor.test.ts
| ### What changes in file mode | ||
|
|
||
| - **References are slugs, not `#numbers`.** `mm dispatch`, `mm status`, and the dashboard show `retry-webhooks`, not `#123`. | ||
| - **The recommender ranks files.** `mm run-recommender` ranks the Epic files under `planning/epics/` and rewrites `.middle/state.md` instead of the GitHub state issue. The dispatcher's poller picks up state changes on its normal cadence (≈120s), same as github mode. |
There was a problem hiding this comment.
Capitalize “GitHub” consistently.
Line 122 says “github mode”; use “GitHub mode” for correct product naming.
Proposed patch
-- **The recommender ranks files.** `mm run-recommender` ranks the Epic files under `planning/epics/` and rewrites `.middle/state.md` instead of the GitHub state issue. The dispatcher's poller picks up state changes on its normal cadence (≈120s), same as github mode.
+- **The recommender ranks files.** `mm run-recommender` ranks the Epic files under `planning/epics/` and rewrites `.middle/state.md` instead of the GitHub state issue. The dispatcher's poller picks up state changes on its normal cadence (≈120s), same as GitHub mode.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **The recommender ranks files.** `mm run-recommender` ranks the Epic files under `planning/epics/` and rewrites `.middle/state.md` instead of the GitHub state issue. The dispatcher's poller picks up state changes on its normal cadence (≈120s), same as github mode. | |
| - **The recommender ranks files.** `mm run-recommender` ranks the Epic files under `planning/epics/` and rewrites `.middle/state.md` instead of the GitHub state issue. The dispatcher's poller picks up state changes on its normal cadence (≈120s), same as GitHub mode. |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~122-~122: The official name of this software platform is spelled with a capital “H”.
Context: ... on its normal cadence (≈120s), same as github mode. - PRs are unchanged. The agen...
(GITHUB)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/operator.md` at line 122, Update the documentation text that currently
reads "github mode" to use the correct product capitalization "GitHub mode";
locate the string "github mode" in docs/operator.md (the sentence describing mm
run-recommender rewriting .middle/state.md and dispatcher poller cadence) and
replace it with "GitHub mode" to ensure consistent naming throughout the doc.
…lary-check Add docs/vocabulary.md documenting every middle label (meaning, who applies it, middle's response, when to use) as the single source of truth, and replace the definition-shaped label prose in the three Epic-aware skills with cross-links to it (red-flag entries stay — action-shaped). Add 'mm doctor --vocabulary-check': a docs<->code drift guard that parses the doc and asserts every label the code deterministically keys on (the needs-design / agent-queue:state constants + middle-owned NON_FEATURE_LABELS) is documented, plus the full canonical vocabulary is present. Tested in the CLI suite, booting the real mm binary against the shipped doc (#217).
Add 'mm start --foreground': run the dispatcher in-process with no fork and no pid file, so a service manager (systemd Restart=on-failure / launchd KeepAlive) owns the lifecycle; runDaemon's SIGTERM handler drains and exits 0 cleanly. Add docs/daemon-as-a-service.md with complete systemd user + system units and a launchd plist, plus install / verify / log-tail commands; cross-link it from operator.md's 'Start and stop' section and the command table, and from the README setup steps as the next step after mm doctor. Integration test boots the real mm binary via Bun.spawn against an isolated HOME + config, confirms /health answers, asserts no pid file is written, and that SIGTERM exits cleanly (#218).
…view edges - checkEpicFilesRoundTrip: move the readFileSync inside the try so a directory named *.md (EISDIR) or an unreadable *.md symlink under epics_dir surfaces as an epic-files fail naming the slug, instead of throwing out of the whole mm doctor run (listEpicSlugs matches on name only). - parseDocumentedLabels: skip fenced code blocks so a documentation example that shows a '### `label`' heading can't inflate the documented-label set. - Soften the vocabulary-check docstring: it's a code-coverage + completeness check, not bidirectional equality (extra documented labels are allowed). Tests for both edges; same class as the changes already under review.
…ing fixes - daemon-as-a-service.md: replace literal <you> (invalid XML in the launchd plist example, breaks plutil/launchctl) with a plain YOUR_USERNAME token - operator.md: label the file-layout fenced block as text (MD040) - plan.md: prefix the decision note so it isn't a malformed ATX heading (MD018)
--foreground took an early return that skipped runStart's live/stale pidfile check, so it would launch a second daemon over a running background dispatcher and never clear a stale pidfile. Extract pidFilePreflight and call it from both the background and foreground paths. Tests cover both foreground cases; the integration test now binds an ephemeral free port instead of a fixed 41877 so it can't collide under parallel CI.
64be4ae to
6ac3c38
Compare
Summary
Closes #209
Ships the three operator-facing docs the docs audit flagged, each backed by a real-path integration test so the docs can't silently drift from the code: enabling file mode on an existing repo, the full label vocabulary, and running middle as a system service. One Epic → one branch → one PR; the three open sub-issues were the phases.
What changed
docs/operator.md— new "Enable file mode on an existing repo" how-to (flip viamm init --epic-store=file, directory layout, a round-tripping worked-example Epic file, operator-visible differences);mm start --foreground+ service cross-links in "Start and stop"; command-table updates.docs/vocabulary.md(new) — every middle label (meaning / who applies / middle's response / when to use), the single source of truth.docs/daemon-as-a-service.md(new) — complete systemd (user + system) units and a launchd plist, with install / verify / log-tail commands.README.md— daemon-as-a-service as the next step aftermm doctor; "Going deeper" links for the two new docs.packages/cli/src/commands/doctor.ts— file-mode check extended to three rows (epics_dir, state_file, Epic-file round-trip); newmm doctor --vocabulary-checkdocs↔code drift guard.packages/cli/src/commands/start.ts+index.ts—mm start --foreground(in-process daemon, no pid file, clean SIGTERM).creating-/recommending-/implementing-github-issues) — definition-shaped label prose replaced with a cross-link todocs/vocabulary.md(red-flag entries kept); bootstrap mirror re-synced.packages/cli/test/doctor.test.ts(file-mode + vocabulary),packages/cli/test/start-foreground.test.ts(new).Why these changes
The recommender's classification is LLM-driven, so #217's "the check exits 0 only when docs and code agree" can't be a deterministic replay of the recommender — it's realized as a docs↔code drift guard asserting every label the code keys on is documented. The existing-repo file-mode flip is documented as
mm init --epic-store=file(not a hand TOML edit) becausemm initwrites both the toml and the daemon-db row the dispatcher routes on; a toml-only edit is a silent half-switch. Full reasoning inplanning/issues/209/decisions.md.Acceptance criteria
mm doctorboots the CLI and runs the file-mode checks (epics_dir, state_file, Epic-file round-trip) against the documented config, exiting 0 — integration testdoctor honors the documented file-mode configinpackages/cli/test/doctor.test.tslifts the worked example from the doc and bootsmm doctoron it.mm doctor --vocabulary-checkboots the CLI, parsesdocs/vocabulary.md, and asserts docs↔code label agreement — integration test boots the realmmbinary viaBun.spawninpackages/cli/test/doctor.test.tsand exits 0.mm start --foregroundboots the daemon in-process with no pid file and exits cleanly on SIGTERM — integration test boots the realmmbinary viaBun.spawninpackages/cli/test/start-foreground.test.ts, confirms/health, asserts no pid file, SIGTERM exit 0.Verification
Full suite green:
bun test→ 1381 pass, 0 fail;bun run typecheckclean;bun run lint/formatclean.Fresh-operator walkthrough (every documented command run against the real CLI):
mm start --help/mm doctor --help/mm init --helpshow the documented--foreground,--vocabulary-check,--epic-storeflags. ✅mm init <repo> --epic-store=file --dry-runprints exactly the documented file-mode scaffold:would scaffold .middle/<owner>-<name>.toml ([epic_store] mode=file),planning/epics/ (README.md + .keep),.middle/state.md. ✅[epic_store]TOML the docs show is byte-identical to whatrenderEpicStoreToml()writes (mode = "file",epics_dir = "planning/epics",state_file = ".middle/state.md"). ✅mm doctor --vocabulary-check→✓ docs and code agree — 6 code-keyed label(s) documented, all 13 labels listed. ✅mm start --foregroundboots the real daemon (integration test):/healthanswers, no~/.middle/dispatcher.pidwritten, SIGTERM → exit 0. ✅Per-phase test evidence:
doctor honors the documented file-mode config+malformed Epic file → epic-files fail(doctor.test.ts).Stumbling points
node_modules(nobun installhad run), so@middle/*deep imports failed untilbun install.oxfmtreformats a pre-existing drift inpackages/adapters/copilot/test/adapter.test.tson every run; reverted each time to keep this PR scoped (not mine to fold in).packages/dispatcher/test/workflows/recommender.test.ts, which doesn't exist (the recommender test isrecommender-workflow.test.ts); the drift guard lives inmm doctor, so its test is in the CLI suite — see decisions.md.Suggested CLAUDE.md updates
None — the conventions held. (The
EpicStoreSettings"writes both" docstring incore/config.tswas the key that prevented documenting a misleading toml-only flip; it's already well-placed.)Follow-up issues
None — scope delivered in full.
Out of scope
Windows service templates and Docker/containerized deployment (sibling-noted out of scope in #218); changing recommender/dispatch behavior (docs + a drift guard only).
Summary by CodeRabbit
New Features
Documentation
Tests