fix(dispatcher): resolve state-issue schema from the package, not per-repo#157
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 (9)
📝 WalkthroughWalkthroughThis PR implements schema path resolution for the state-issue recommender by exporting an absolute filesystem path constant from the ChangesSchema Path Resolution Implementation
Planning and Decisions
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
…-repo The recommender resolved <repoPath>/schemas/state-issue.v1.md, which only exists in middle's own checkout — so a recommender run against any bootstrapped target repo would fail to find the schema once auto-dispatch lands. Add STATE_ISSUE_SCHEMA_PATH to @middle/state-issue, resolved from the package's own location (import.meta.dir) like skills-sync.ts resolves middle-tree assets. Both recommender readers (resolveRecommenderOptions, the daemon's resolveRunSettings) now point at it instead of <repoPath>/schemas. The existsSync guard is reworded: a miss now means a broken middle install, not a repo problem. Drops the dead per-repo schema fixture from run-recommender.test.ts (the thin client never reads it). Closes #107.
| * (its `..` count differs because it sits one directory deeper). This file lives | ||
| * at `packages/state-issue/src/`, so the repo root is three levels up. | ||
| */ | ||
| export const STATE_ISSUE_SCHEMA_PATH = join( |
There was a problem hiding this comment.
Decision: resolve the schema from the package, not per-repo stamping (issue #107 option b, not a).
The issue offered two fixes: have mm init copy the schema into each target repo, or resolve it from the @middle/state-issue package. We chose the package resolver because:
- Single source of truth. The root CLAUDE.md state-issue contract makes
schemas/state-issue.v1.mdauthoritative. Per-repo copies drift from it — the exact failure that rule prevents — and would need a per-repo re-sync gate (skills already carry this "two-copies invariant"; stamping would multiply it by every target repo). - The schema is middle-internal. Skills are stamped because the dispatched coding agent runs in the target repo and collaborators share them. The state-issue schema is dispatch machinery the recommender agent reads; the target repo never needs it in-tree.
- Established precedent.
packages/cli/src/bootstrap/skills-sync.tsalready resolves middle-tree assets fromimport.meta.dir. This mirrors that pattern (different directory depth, same idea).
| return { | ||
| ok: false, | ||
| error: `state-issue schema not found at ${schemaPath} (Phase 7 runs against middle's own repo)`, | ||
| error: `state-issue schema missing from the middle installation at ${schemaPath} — this is a packaging bug, not a repo problem`, |
There was a problem hiding this comment.
Decision: reworded the existsSync guard as a packaging-integrity check. Post-fix the path points into the middle installation, which always ships the schema, so a miss now means a corrupt/partial middle install — a different failure mode than the old per-repo "Phase 7 runs against middle's own repo" condition. The cheap guard stays so the failure surfaces here with a clear cause instead of as an agent-side cat: no such file mid-run.
| // Phase 7 schema lives at the repo root. | ||
| mkdirSync(join(repoPath, "schemas"), { recursive: true }); | ||
| writeFileSync(join(repoPath, "schemas", "state-issue.v1.md"), "# schema\n"); | ||
| // No per-repo schema fixture: runRecommender is a thin daemon client (it never |
There was a problem hiding this comment.
Decision: dropped the dead per-repo schema fixture. runRecommender is a thin daemon client — it never calls resolveRecommenderOptions (validation happens daemon-side), so this fixture was never read even before this change. After option (b), no reader looks in the target repo at all, so keeping it would be doubly misleading.
thejustinwalsh
added
the
ready-for-review
Reviewer's brief — PR #157 (closes #107)What this does. The recommender resolved the state-issue schema at How to run it. bunx tsc --noEmit # typecheck — clean
bun run lint # oxlint --deny-warnings — clean
bun test # 722 pass / 0 fail
bun test packages/state-issue/test/schema-path.test.ts \
packages/dispatcher/test/recommender-run.test.ts # the fix, focusedWhat to verify (and what "correct" looks like).
How to review it. Small, contained diff (+199/−9, 9 files). Per-line review comments on the PR carry the decision rationale. Confirm no other reader still uses Fragile / extra eyes. The only load-bearing assumption is that middle runs from its source tree (Bun runs |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Single-pass new-work-as-base merge of origin/main after rebase kept re-conflicting on the same hunks across multiple commits (CLAUDE.md escape hatch). - packages/dispatcher/src/poller-cron.ts — unified `startPoller(deps, opts)` signature; folded `ReconcilerHooks` into `StartPollerOptions` as `opts.reconcilers` (alongside `opts.checkboxRevert` and `opts.intervalMs`). - packages/dispatcher/src/main.ts — unified daemon-startup: keeps the durable engine + `recoverEngine` + `reconcileOrphanedSignals` from #160, the notification-failsafe watchdog comment from #162, and adds the `reconcileOpenPRsForRepo` block + `reconcilers` config in the `startPoller` call. Dropped the now-unused `Engine` import (main routes through `createDurableEngine`). - packages/core/src/index.ts — kept both export blocks: integration rubric from #163, `selectAdapter` from this PR. - packages/dispatcher/test/recommender-run.test.ts — kept both describe blocks (adapter-enabled gate from this PR, schema-resolution from #157); added `enabled: true` to the schema test's adapter config so it passes the new gate. - packages/dispatcher/test/gates/checkbox-revert-pass.test.ts — added the five new `GitHubGateway` methods to the test stub (`listOpenIssues`, `addLabel`, `listMergedPrsClosingRefs`, `closeIssue`, `createIssue`) main grew during the marathon. Gates re-verified locally: `bun run typecheck` clean, `bun test packages/dispatcher` 620/620 pass, `bun run lint` clean, `bun run format` clean (no changes).
1 participant
Summary
Closes #107
The recommender resolved the state-issue schema at
<repoPath>/schemas/state-issue.v1.md, which exists only in middle's own checkout. Once auto-dispatch (Phase 8) runs the recommender against bootstrapped target repos, the schema isn't there. This resolves the schema from the@middle/state-issuepackage — the issue's option (b) — so no per-repo copy is needed and the single source of truth never drifts.What changed
packages/state-issue/src/schema-path.ts(new) —STATE_ISSUE_SCHEMA_PATH, the absolute path to the canonicalschemas/state-issue.v1.md, resolved from the module's own location (import.meta.dir) so it points into the middle install regardless of cwd or which target repo is in play.packages/state-issue/src/index.ts— export it; module-index frontmatter updated (Public surface:+Where things live:).packages/dispatcher/src/recommender-run.ts—resolveRecommenderOptionsnow uses the resolver; theexistsSyncguard is reworded (a miss = broken middle install, not a repo problem).packages/dispatcher/src/main.ts— the daemon'sresolveRunSettingsuses the resolver.packages/state-issue/test/schema-path.test.ts;recommender-run.test.tsnow asserts the schema resolves independently ofrepoPath; the dead per-repo fixture is removed fromrun-recommender.test.ts.Why these changes
Option (a) —
mm initstamping the schema into each target repo — would create one copy per repo, each able to drift from the source of truth the root CLAUDE.md state-issue contract establishes, and would need a per-repo re-sync gate. The schema is middle-internal dispatch machinery the target repo's collaborators never need in-tree (unlike skills, which the dispatched coding agent reads in-repo). Resolving from the package keeps one authoritative copy and follows the existingskills-sync.tsimport.meta.dirprecedent. The issue's own wording ("so no per-repo copy is needed") leans the same way. Full rationale inplanning/issues/107/decisions.md, distilled into per-line review comments.Verification
bunx tsc --noEmit— clean.bun run lint(--deny-warnings) +bun run format— clean.bun test— 722 pass / 0 fail across 81 files.recommender-run.test.ts→ "resolves schemaPath from the middle install, not from repoPath" builds a target repo with noschemas/dir and assertsresolveRecommenderOptions(...).ok === true,schemaPath === STATE_ISSUE_SCHEMA_PATH, and!schemaPath.startsWith(repoPath).schema-path.test.tsasserts the resolved path is absolute, ends inschemas/state-issue.v1.md, exists on disk, and contains the real schema header.Acceptance evidence
mm initset up (noschemas/dir). — see the testrecommender-run.test.ts"resolves schemaPath from the middle install, not from repoPath", which builds a repo with noschemas/dir and assertsresolveRecommenderOptions(...).ok === true.@middle/state-issuepackage (issue's option b), no per-repo copy. —packages/state-issue/src/schema-path.ts; both readers wired inrecommender-run.tsandmain.ts.schema-path.test.tspins the resolution to the realschemas/state-issue.v1.md.Status
STATE_ISSUE_SCHEMA_PATH, wire both readers, drop dead fixture, testsStumbling points
# Agent Queue State Issue — Schema v1(no literalstate-issuetoken), so the content assertion anchors onState Issue — Schema v1instead.joinbecame unused inrecommender-run.tsafter the swap — caught byoxlint --deny-warnings, removed.Suggested CLAUDE.md updates
None. The state-issue contract section already names
schemas/state-issue.v1.mdas the source of truth — this change makes the code resolve it accordingly.Architectural forks
None. The decision was resolved by the CLAUDE.md contract + the
skills-sync.tsprecedent, not by building competing implementations.Follow-up issues
None surfaced.
Out of scope
mm init's stamping set (option (a) — explicitly rejected).Decisions
See
planning/issues/107/decisions.md(distilled into the per-line review comments on this PR).Summary by CodeRabbit
Release Notes
Refactor
Tests