feat: Phase 3 — bootstrap commands, skills, and state issue#81
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughImplements Phase 3 bootstrap: adds mm init/uninit CLI flows, byte-accurate canonical skills mirroring with sync/check scripts and pre-commit enforcement, hook config writers/stripers, state-issue creation and dispatcher read/write seams, CLI wiring, operator runbook, and comprehensive tests. ChangesPhase 3 Bootstrap Commands, Skills, and State Issue
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
|
packages/skills/ holds the canonical implementing- and recommending-github-issues skills; packages/cli/src/bootstrap-assets/skills/ mirrors them byte-for-byte (the copy mm init stamps). scripts/sync-skills.ts keeps them in sync; a committed pre-commit hook (core.hooksPath=scripts/hooks, installed by the prepare script) fails the commit on drift, and mm doctor warns on it.
6 tasks
Transactional bootstrap of middle into a target repo (validate → stage skills to .claude/.codex → hook.sh + per-CLI hook config → .middle/config.toml → state label/issue seam → gitignore), idempotent across re-init, with a --dry-run plan. mm uninit reverses each step, stripping only middle's hook entries and preserving foreign ones. Externals are isolated behind an injectable BootstrapDeps; tests exercise fresh/reinit/dry-run/uninit over a scratch repo.
…arsing Locks sub-issue #24: the empty body mm init creates is schema-conforming and byte-identical round-trip stable, and origin-remote slug parsing covers SSH/HTTPS forms with and without .git.
This was referenced May 23, 2026
The dispatcher reads a repo's state issue via gh, parses it, and writes back only In-flight / Rate limits / Slot usage (partial patch supported), re-rendering so recommender-owned sections and the generated metadata survive byte-identically. Optional dispatcher-tick markers are placed before the first section and ignored by the parser. gh access is behind an injectable StateIssueGateway.
5 tasks
Untrack files an earlier `git add -A` swept in; the per-repo .middle/ dir and the machine-local .claude/settings.json are operational, not source. Ignoring .middle/ is mm init's own step 7 applied to middle — the committable slice of the dogfooding self-bootstrap. Skills under .claude/skills/ stay committed.
The live mm init . on middle and mm dispatch . <issue> are an operator handoff (spec dogfooding rule #5), not an in-flight-agent action: dispatch nests agents and the live state issue would orphan from an unmerged branch. Documents the runbook and records the decision; the committable self-bootstrap slice (gitignore .middle/) already landed.
Owner
Author
Phase 3 status — 4/5 done, #26 is an operator handoffSub-issues #22, #23, #24, #25 are implemented, tested, and pushed on PR #81 (all tests + typecheck green):
#26 (dogfooding crossover) needs youThe live crossover can't be safely run from inside this dispatch:
Delivered committable slice: Decision needed: run the live crossover yourself per |
4 tasks
Owner
Author
|
#26 is stale, we have manually done these steps and are indeed running it through middle now, so contraints have been handled and we just need to ensute |
thejustinwalsh
added
the
ready-for-review
Owner
Author
Reviewer's brief — Phase 3 (bootstrap commands, skills, state issue)Scope: sub-issues #22–#25 (all closed). #26 — the live "bootstrap middle into its own repo" crossover — is a post-merge operator action, deliberately deferred. It is not part of this review; don't block on it being open. Rationale in How to run / verify
What to verify (load-bearing)
Extra eyes
Decisions log: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
packages/cli/src/bootstrap/assets.ts (1)
10-13: ⚡ Quick winMake bootstrap skill ordering deterministic.
readdirSyncorder can vary by filesystem, which can cause unstablestageSkillsaction ordering/tests. Sorting the skill names keeps behavior reproducible.♻️ Proposed change
export function listBootstrapSkills(): string[] { if (!existsSync(BOOTSTRAP_SKILLS_DIR)) return []; return readdirSync(BOOTSTRAP_SKILLS_DIR, { withFileTypes: true }) .filter((e) => e.isDirectory()) - .map((e) => e.name); + .map((e) => e.name) + .sort((a, b) => a.localeCompare(b)); }🤖 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/bootstrap/assets.ts` around lines 10 - 13, readdirSync on BOOTSTRAP_SKILLS_DIR can return entries in non-deterministic order; ensure the returned skill list is stable by sorting the directory names before returning: after filtering/map of Dirent names (reference the usage of readdirSync and BOOTSTRAP_SKILLS_DIR in this function), call a stable sort on the array of names (e.g., alphabetical) so stageSkills ordering/tests become deterministic.packages/cli/test/bootstrap-init.test.ts (1)
100-140: ⚡ Quick winAdd a regression test for reinit/migrate when
state_issue.numberis missing/zero.Current tests cover clean reinit, but not the broken-config path that can preserve
stateIssue = 0. A focused test here would lock the expected recovery behavior and prevent regressions.🤖 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/test/bootstrap-init.test.ts` around lines 100 - 140, Add a new unit test in packages/cli/test/bootstrap-init.test.ts that covers the regression where the stored state_issue.number is missing or zero: use makeFakeDeps() and prepare the repo so .middle/config.toml either lacks state_issue or has state_issue = 0, then call initRepo(repo, deps, { dryRun: false }) and assert that initRepo recovers/migrates (result.mode indicates reinit/migrate), result.stateIssue is a non-zero issue id, and no duplicate issue is created (calls.created length remains 1); reference initRepo, makeFakeDeps, stateIssue, .middle/config.toml and calls.created when locating where to add this test.planning/issues/21/plan.md (1)
29-44: 💤 Low valueConsider clarifying phase numbering vs issue numbers.
The section header "Phases (one per sub-issue)" lists phases 1-5, but the issue numbers appear as
#23,#22,#24,#25,#26(out of numerical order). This is intentional—phases are ordered by implementation dependency—but might briefly confuse readers expecting sequential issue numbers.Optional clarification
-## Phases (one per sub-issue) +## Phases (one per sub-issue, ordered by implementation dependency)🤖 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 `@planning/issues/21/plan.md` around lines 29 - 44, Clarify that the "Phases (one per sub-issue)" numbering is implementation order not GitHub issue numerical order by adding a short parenthetical or sentence after the header (referencing the header text "Phases (one per sub-issue)" and the listed items that reference issues `#23`, `#22`, `#24`, `#25`, `#26`) stating that phases 1–5 are ordered by dependency and do not correspond to ascending issue numbers; keep each bullet as-is but add this one-line note so readers aren’t confused.
🤖 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/dogfooding.md`:
- Around line 56-58: Add concrete guidance for creating the manual Epic
referenced by the mm dispatch step: specify required labels (e.g., "epic" plus
any pipeline-specific tags), the expected sub-issue format (each sub-issue must
include title, corresponding assignee and status label, and an optional
"estimate" field in the body), and the Epic description structure (summary,
expected acceptance criteria, and a simple list of sub-issue issue keys or
links). Reference the commands "mm start" and "mm dispatch . <epic-number>" and
mention that the Epic must be reachable by its issue number/id and that
sub-issues should be linked to the Epic (via parent field or issue links) so
dispatch can resolve them.
- Around line 49-50: Update docs/dogfooding.md to clarify two points: confirm
that the git command in step 3 (git add .claude/skills .codex/skills .gitignore)
is correct because mm init stages skills into .claude/skills and .codex/skills
and only .middle/ is intended to be local-only in .gitignore; and enhance step 5
to specify the exact requirements for the “manually created Epic” used with mm
dispatch . <epic-number>: the Epic must be a GitHub issue that contains native
GitHub sub-issues (open sub-issues represent phases and their acceptance
criteria), do not dispatch bare sub-issues, and include a link to the skills
documentation section that explains Epic/sub-issue requirements so operators
know what to put in each sub-issue.
In `@package.json`:
- Line 15: The package.json "prepare" script currently runs automatically and
changes the user's Git core.hooksPath; update this by either removing or making
it opt-in (e.g., require an env flag like INSTALL_HOOKS=true or SKIP_HOOKS check
before running the git config command) and add a short note to README or
CONTRIBUTING describing the behavior and how to opt into or disable it so
contributors aren't surprised by the automatic git configuration change.
In `@packages/cli/src/bootstrap/init.ts`:
- Around line 22-34: The readExistingConfig function currently swallows TOML
parse errors and returns null, causing a malformed .middle/config.toml to be
treated as a fresh install; update readExistingConfig to rethrow or propagate
parsing errors instead of returning null (i.e., remove the empty catch and let
the parseToml/readFileSync errors bubble up or throw a new Error with context),
ensuring callers (initRepo) will fail fast; apply the same change to the other
identical parse block referenced in the diff (the second TOML read/parse code
path) so both code paths consistently surface parse failures.
- Around line 95-106: The code keeps stateIssue (which may be 0) for any mode
!== "fresh", causing repos with no valid state issue to proceed without one;
change the logic so when stateIssue is falsy (e.g., 0) you perform the same
creation steps as the "fresh" branch: call deps.github.ensureStateLabel(info)
and, if not dry, call buildInitialStateIssueBody(deps.now()) and
deps.github.createStateIssue(info, STATE_ISSUE_TITLE, body) to set stateIssue.
Locate the variables/stateIssue, mode, existing, and the calls
deps.github.ensureStateLabel and deps.github.createStateIssue to implement this
fallback creation when stateIssue is missing.
In `@packages/cli/src/bootstrap/skills-sync.ts`:
- Around line 65-77: unionSkillFiles currently builds the file union by
iterating only skills returned from listSkillDirs(canonicalDir), so any skill
directories that exist solely in mirrorDir are omitted and won't be removed by
syncSkills; fix by enumerating skill directories from both trees (e.g., call
listSkillDirs(canonicalDir) and listSkillDirs(mirrorDir) or merge their results
into a single set) and then iterate that combined set when collecting paths with
listFilesRecursive(join(...)). Ensure the returned sorted array includes files
from both canonicalDir and mirrorDir so orphaned mirror directories are detected
and removed during syncSkills.
In `@packages/cli/src/bootstrap/uninit.ts`:
- Around line 58-61: When deciding whether to close a state issue in uninit,
don't skip closure just because local repo metadata `info` is missing; if
`stateIssue > 0` attempt to resolve repository info via the injected `deps`
before skipping. Update the block around `if (stateIssue > 0 && info)` (where
`note`, `dry`, and `deps.github.closeStateIssue` are used) to first try a
fallback lookup like `info = info || await deps.github.resolveRepoInfo()` (or
the appropriate deps method) and only skip closure if that lookup fails; then
call `deps.github.closeStateIssue(info, stateIssue, ...)` when `dry` is false.
In `@packages/cli/src/commands/doctor.ts`:
- Line 111: The current warning summary text still refers to "UX" degradation
but checkSkillsDrift() (and similar checks) produce non-UX warnings; update the
summary strings used around the doctor warnings so they are generic (e.g.,
"Warnings detected" or "Potential issues detected") instead of UX-specific
language. Locate the places that compose the final warning summary (the array
including checkSkillsDrift() and the code that formats/prints the summary around
the entries at the same block that handles warnings at lines near where
checkSkillsDrift() is included) and replace the UX-specific message with a
neutral summary used for all warnings.
---
Nitpick comments:
In `@packages/cli/src/bootstrap/assets.ts`:
- Around line 10-13: readdirSync on BOOTSTRAP_SKILLS_DIR can return entries in
non-deterministic order; ensure the returned skill list is stable by sorting the
directory names before returning: after filtering/map of Dirent names (reference
the usage of readdirSync and BOOTSTRAP_SKILLS_DIR in this function), call a
stable sort on the array of names (e.g., alphabetical) so stageSkills
ordering/tests become deterministic.
In `@packages/cli/test/bootstrap-init.test.ts`:
- Around line 100-140: Add a new unit test in
packages/cli/test/bootstrap-init.test.ts that covers the regression where the
stored state_issue.number is missing or zero: use makeFakeDeps() and prepare the
repo so .middle/config.toml either lacks state_issue or has state_issue = 0,
then call initRepo(repo, deps, { dryRun: false }) and assert that initRepo
recovers/migrates (result.mode indicates reinit/migrate), result.stateIssue is a
non-zero issue id, and no duplicate issue is created (calls.created length
remains 1); reference initRepo, makeFakeDeps, stateIssue, .middle/config.toml
and calls.created when locating where to add this test.
In `@planning/issues/21/plan.md`:
- Around line 29-44: Clarify that the "Phases (one per sub-issue)" numbering is
implementation order not GitHub issue numerical order by adding a short
parenthetical or sentence after the header (referencing the header text "Phases
(one per sub-issue)" and the listed items that reference issues `#23`, `#22`, `#24`,
`#25`, `#26`) stating that phases 1–5 are ordered by dependency and do not
correspond to ascending issue numbers; keep each bullet as-is but add this
one-line note so readers aren’t confused.
🪄 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: fedc0f82-3354-4cf3-b04e-136eb17ed2d3
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (35)
.gitignoredocs/dogfooding.mdpackage.jsonpackages/cli/package.jsonpackages/cli/src/bootstrap-assets/skills/implementing-github-issues/SKILL.mdpackages/cli/src/bootstrap-assets/skills/implementing-github-issues/references/vitexec-integration-suite.mdpackages/cli/src/bootstrap-assets/skills/recommending-github-issues/SKILL.mdpackages/cli/src/bootstrap/assets.tspackages/cli/src/bootstrap/config-template.tspackages/cli/src/bootstrap/deps.tspackages/cli/src/bootstrap/gitignore.tspackages/cli/src/bootstrap/hook-config.tspackages/cli/src/bootstrap/index.tspackages/cli/src/bootstrap/init.tspackages/cli/src/bootstrap/skills-sync.tspackages/cli/src/bootstrap/state-issue-body.tspackages/cli/src/bootstrap/types.tspackages/cli/src/bootstrap/uninit.tspackages/cli/src/commands/doctor.tspackages/cli/src/commands/init.tspackages/cli/src/commands/uninit.tspackages/cli/src/index.tspackages/cli/test/bootstrap-init.test.tspackages/cli/test/doctor.test.tspackages/cli/test/skills-sync.test.tspackages/cli/test/state-issue-body.test.tspackages/dispatcher/src/state-issue.tspackages/dispatcher/test/state-issue.test.tspackages/skills/implementing-github-issues/SKILL.mdpackages/skills/implementing-github-issues/references/vitexec-integration-suite.mdpackages/skills/recommending-github-issues/SKILL.mdplanning/issues/21/decisions.mdplanning/issues/21/plan.mdscripts/hooks/pre-commitscripts/sync-skills.ts
- init: fail fast on a malformed existing .middle/config.toml instead of treating it as fresh (was risking a duplicate state issue + clobbered metadata) - init: mint + persist a state issue on reinit/migrate when the config carries no usable issue number, not only on fresh - uninit: resolve repo identity from deps when [repo] is missing, so the state issue is never left orphaned open - skills-sync: union skill dirs from both trees so an orphaned mirror dir is detected and removed (was silently breaking byte-identity) - doctor: generic warning summary (skills-drift warnings aren't UX-only) - prepare: announce the core.hooksPath change instead of doing it silently - docs/dogfooding: how to create a dispatchable Epic; .codex/skills commit note
This was referenced May 24, 2026
9 tasks
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
1 participant
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.
Summary
Closes #21
🚧 Draft — work in progress. Phase 3 of the build spec: the dogfooding crossover. See the plan comment for full scope.
Plan
mm init/mm uninitbootstrap (and reverse) middle into a target repo — skills, hooks, per-repo config, a schema-conforming state issue, gitignore. The dispatcher gains read/write of its three owned state-issue sections. Finally, middle is bootstrapped into its own repo.Status
packages/skills/+ byte-identicalbootstrap-assets/mirror + sync hook + doctor drift checkmm initandmm uninit(transactional, reversible,--dry-run)mm initVerification evidence
bun test packages/cligreen;bun scripts/sync-skills.ts --checkconfirms byte-identity; the pre-commit hook blocks on injected drift;mm doctorwarns on it.packages/cli/test/bootstrap-init.test.ts(9 tests): fresh install, idempotent re-init,--dry-run, validation rejects, uninit removal, hook-strip preserves foreign entries. Real binary smoke-tested:mm init <scratch> --dry-run.packages/cli/test/state-issue-body.test.ts(10 tests): empty body parses, validates, round-trips byte-identically; config writeback assertsnumber = 142; slug parsing covers SSH/HTTPS.packages/dispatcher/test/state-issue.test.ts(9 tests): dispatcher rewrites only In-flight/Rate limits/Slot usage; recommender-owned sections stay byte-identical (with and without dispatcher-tick markers); ticks don't accumulate.Decisions
See
planning/issues/21/decisions.md(distilled into per-line review comments before final review).Summary by CodeRabbit
New Features
Documentation
Chores
Tests
Bug Fixes