feat(epic-store): file-mode completeness — review-resume, recommender, browse cache#204
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 (6)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughIntroduces file-mode epic store support: adds EpicStore config and EpicStoreSettings, migrates epics to canonical string refs, widens state in-flight refs to strings, routes epic/state access per-repo mode, updates auto-dispatch/recommender/poller/gateways, and updates dashboard/UI and tests. ChangesFile-mode Epic Store Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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. Comment |
Resolve a file-mode Epic's PR from the Epic file's durable meta.pr stamp so review-changes/CHANGES_REQUESTED (and the merged/closed reconcile) resume work in file mode — previously the file poll gateway returned null for any slug. - Widen PollGateway with by-PR-number prSnapshot/prLifecycle; ghPollGateway's Closes-#N finder now reuses the extracted fetchPrSnapshot builder. - file-poll-gateway resolves a slug via meta.pr then fetches by number; numeric refs still delegate to gh's finders. A stale/absent meta.pr degrades to null. - Route the two methods through makeRoutingPollGateway. - Integration test drives the real runPoller path end to end for a file Epic.
Route the auto-dispatch readState and the recommender state I/O through a new makeRoutingStateGateway (file <-> github per repo), so a file-mode repo's ranked plan in its state_file drives dispatch and the recommender reads/writes it. - state-issue: InFlightItem.issue widened number -> string (numeric Epic number or file-mode slug); parser reads #([\w-]+); stays schema v1 (additive, round -trip holds; fuzz + samples seeded with slugs). - auto-dispatch: enqueue by epicRef (string); parseEpicRef extracts #<ref>; a file Epic dispatches by slug. main.ts runs auto-dispatch for file-mode repos (sentinel state issue 0; the file gateway ignores it). - recommender: in-flight rows source epicRef (added to ActiveImplementationWorkflow via the migration-009 epic_ref column); state I/O routed; resolveRecommenderOptions accepts file mode; the prompt reframes for the file store. surface() skips the gh comment for sentinel 0. Live ranking quality is operator-smoke (per #190). - core: parse [epic_store] into MiddleConfig.epicStore. - Integration test drives the real file-mode auto-dispatch readState path.
Re-key the epics browse cache from (repo, number) to (repo, ref) [migration 010] so a file-mode Epic — a slug with no GitHub number — is cached and surfaces in the dashboard. refreshEpics routes through the routing Epic gateway (per-mode listing); readEpics orders github Epics newest-first, file Epics after. - epics-cache: ref-keyed upsert/close, EpicRow gains ref + nullable number, the file-Epic skip is removed. - dashboard: EpicCard carries ref + nullable number; the card renders via the existing <EpicRef> (#N label or file:// slug link); workflowForEpic keys on epic_ref (both modes); the ready-row join matches by ref. In-dashboard force -dispatch is disabled for file Epics (numeric route only) with a title pointing at 'mm dispatch <slug>' — browsable, not falsely dispatchable. - Tests: file-mode cache (cache/close/order), dashboard listEpics by ref, the Epics component file:// render. Schema-version assertions bumped 9 -> 10.
Adversarial self-review pass before marking ready — resolve the ref-shape class: - recommender-run: forward opts.epicStore into RecommenderInput (was dropped on the standalone-helper path; the daemon path already forwarded it). Test captures the on-disk prompt of a file-mode run. - file-poll-gateway: discriminate file vs github by epicFileExists (the check listIssueComments already uses), not a ^\d+$ heuristic — a numeric-named file Epic (42.md) resolves via meta.pr, not github issue #42. - ref regexes: parseEpicRef #(\S+), IN_FLIGHT_RE #([^*\s]+) — match the real ref grammar (space / ** delimited), so a non-kebab file stem (v1.2-rollout) doesn't truncate and break the byte-identical round-trip invariant. Each fix carries a failing-first test.
thejustinwalsh
commented
Jun 3, 2026
thejustinwalsh
left a comment
thejustinwalsh
left a comment
Owner
Author
There was a problem hiding this comment.
Decision-log highlights distilled inline (full rationale in planning/issues/200/decisions.md). These mark the load-bearing design choices a reviewer should sanity-check.
| // github number → gh's `Closes #<n>` finder. Keying on the file (not a | ||
| // `^\d+$` shape) means a numeric-named file Epic (`123.md`) still resolves | ||
| // via meta.pr rather than being mistaken for github issue #123. | ||
| if (!epicFileExists(epicsDir, epicRef)) return gh.findPrForEpic(repo, epicRef); |
Owner
Author
There was a problem hiding this comment.
Why epicFileExists, not a numeric check. A file Epic's PR is resolved from the file's meta.pr stamp — the same key file-epic-gateway.findEpicPr already uses. The discriminator is the Epic file on disk (the same check listIssueComments uses), not a ^\d+$ shape, so a numeric-named file Epic (42.md) still resolves via meta.pr instead of being mistaken for github issue #42. prSnapshot/prLifecycle (new by-PR-number PollGateway methods) do the fetch; a stale meta.pr degrades to null, not a thrown pass.
| if (!m) fail(`malformed "In-flight" item: "${line}"`); | ||
| return { | ||
| issue: Number(m[1]), | ||
| issue: m[1]!, |
Owner
Author
There was a problem hiding this comment.
InFlightItem.issue is now a string (numeric ref or slug), still schema v1. Numeric refs are a subset of string refs and render identically (**#200**), so the byte-identical round-trip invariant holds — no v1→v2 fork. The regex is #([^*\s]+) (not [\w-]) because a file stem isn't constrained to kebab; matching up to the ** delimiter keeps the round-trip exact for any slug the file store produced (a dotted v1.2-rollout would otherwise truncate). Scoped to In-flight per the issue; the other sections stay numeric.
| * auto-dispatch `readState` always read the GitHub state issue, so a file-mode | ||
| * repo's ranked plan (in its `state_file`) is never read or rewritten (#200). | ||
| */ | ||
| export function makeRoutingStateGateway(deps: { |
Owner
Author
There was a problem hiding this comment.
Routing state gateway + sentinel-0. The StateGateway interface is (repo, issueNumber)-keyed. Rather than thread a number | fileTarget union through the recommender's ~8 call sites, a file-mode repo passes a sentinel 0 that the file gateway ignores (it reads state_file); github repos pass the real number. This router resolves file-vs-github per repo, mirroring makeRoutingEpicGateway/makeRoutingPollGateway. Every path that could hit gh on issue #0 is guarded (the recommender surface, the auto-dispatch parse-surfacer).
| PRIMARY KEY (repo, ref) | ||
| ); | ||
|
|
||
| INSERT INTO epics_new |
Owner
Author
There was a problem hiding this comment.
Re-key (repo, number) → (repo, ref). A file Epic has no GitHub number, so the numeric PK couldn't represent it — it was explicitly skipped, invisible in the dashboard. ref is canonical (numeric string | slug), backfilled CAST(number AS TEXT), exactly mirroring migration 009's workflows.epic_ref. SQLite can't change a PK in place; the runner disables FK enforcement around the rebuild loop for precisely this. readEpics orders number DESC, ref ASC so github Epics come first (newest), file Epics (NULL number → last under DESC) after.
| // routed state gateway reads/writes it and ignores the sentinel `0`. Github mode | ||
| // still requires a configured issue number. | ||
| const fileMode = config.epicStore?.mode === "file"; | ||
| const stateIssue = fileMode ? 0 : config.stateIssue?.number; |
Owner
Author
There was a problem hiding this comment.
Recommender run reachable for file mode. [epic_store] is parsed into MiddleConfig.epicStore; a file-mode repo uses sentinel stateIssue = 0 instead of erroring on the missing [state_issue] number, and the prompt reframes for the file store (rank Epic files, rewrite state_file, refs are slugs). The wiring is unit/integration-tested; the live ranking quality over file Epics is operator-smoke (per #190's file-mode precedent), since it's a live agent run that can't be CI-gated.
thejustinwalsh
added
the
ready-for-review
Owner
Author
Reviewer's brief — #200 file-mode completeness (PR #204)What this is. Closes the three Phase-1/2 file-mode gaps from Epic #190 so a file-backed Epic reaches parity with a GitHub-backed one: PR-review resume, recommender + auto-dispatch, and browse-cache/dashboard visibility. One branch, one PR; How to run itbun install
bun run typecheck # clean
bun run lint # clean
bun test # 1286 pass, 0 fail
# The three real-path integration tests, one per gap:
bun test packages/dispatcher/test/epic-store/file-review-resume-integration.test.ts
bun test packages/dispatcher/test/epic-store/file-auto-dispatch-integration.test.ts
bun test packages/dispatcher/test/epics-cache.test.ts packages/dashboard/test/epics-deps.test.tsWhat to verify (and what "correct" looks like)
How to review
Fragile / extra eyes
|
3 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
schemas/state-issue.v1.md (1)
79-79:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winScope validation rule
#4so it doesn’t imply in-flight#<ref>must be numeric
schemas/state-issue.v1.mdsays “Validation rule#4: All#Nreferences match /#\d+/”, butpackages/state-issue/src/validate.tsapplies the numeric check only toreadyToDispatchepics andblockedissue blockers—state.inFlightrefs are not constrained there.- To keep the schema doc authoritative, update rule
#4wording/scope to exclude in-flight#<ref>(the in-flight ref may be a file-mode slug).- Also align the in-flight slug description: the doc says “kebab-case”, but the parser allows dotted stems (e.g.
v1.2-rollout).🤖 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 `@schemas/state-issue.v1.md` at line 79, Update the schema doc rule `#4` to match actual validator behavior by scoping the numeric-only constraint to readyToDispatch epics and blocked issue refs (as enforced in packages/state-issue/src/validate.ts by the readyToDispatch and blocked checks) and explicitly exclude state.inFlight references from the /#\d+/ requirement; also change the in-flight slug description from “kebab-case” to allow dotted stems (e.g., note that state.inFlight may be a file-mode slug like v1.2-rollout) so the markdown aligns with the parser/validator behavior.
🧹 Nitpick comments (1)
schemas/state-issue.v1.md (1)
47-51: 💤 Low valueDoc says "kebab-case" but the parser accepts broader slugs.
The slug is documented as kebab-case here, yet
parser.tsintentionally captures any non-space/non-*ref (e.g. dottedv1.2-rollout) to keep the round-trip exact. As this doc is authoritative for parser conformance, consider widening the wording (e.g. "a file-stem slug, e.g.rollout-epic-store") so the spec matches the implemented capture surface.🤖 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 `@schemas/state-issue.v1.md` around lines 47 - 51, The spec incorrectly restricts `<ref>` to "kebab-case" while parser.ts accepts any non-space/non-`*` ref (e.g. dotted or mixed tokens like `v1.2-rollout`); update the documentation in state-issue.v1.md to describe `<ref>` as a file-stem slug (or "file-mode Epic slug") that may include dots, hyphens and other non-space/non-`*` characters, give an example such as `v1.2-rollout` alongside `rollout-epic-store`, and note that this wording matches the parser.ts capture behavior so the spec and implementation remain consistent.
🤖 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 `@packages/dispatcher/src/poller-gateway.ts`:
- Around line 107-112: The call that assigns reviewsOut using gh([...
`repos/${repo}/pulls/${prNumber}/reviews`]) is not protected by try/catch, so
transient failures can throw and break the poller pass; update the code to
mirror the viewOut handling by wrapping the reviews fetch in a try/catch block
around the gh call (referencing reviewsOut, gh, repo, prNumber) and return null
(or otherwise handle the error consistently) on failure to preserve per-workflow
failure isolation as implemented in poller.ts.
---
Outside diff comments:
In `@schemas/state-issue.v1.md`:
- Line 79: Update the schema doc rule `#4` to match actual validator behavior by
scoping the numeric-only constraint to readyToDispatch epics and blocked issue
refs (as enforced in packages/state-issue/src/validate.ts by the readyToDispatch
and blocked checks) and explicitly exclude state.inFlight references from the
/#\d+/ requirement; also change the in-flight slug description from “kebab-case”
to allow dotted stems (e.g., note that state.inFlight may be a file-mode slug
like v1.2-rollout) so the markdown aligns with the parser/validator behavior.
---
Nitpick comments:
In `@schemas/state-issue.v1.md`:
- Around line 47-51: The spec incorrectly restricts `<ref>` to "kebab-case"
while parser.ts accepts any non-space/non-`*` ref (e.g. dotted or mixed tokens
like `v1.2-rollout`); update the documentation in state-issue.v1.md to describe
`<ref>` as a file-stem slug (or "file-mode Epic slug") that may include dots,
hyphens and other non-space/non-`*` characters, give an example such as
`v1.2-rollout` alongside `rollout-epic-store`, and note that this wording
matches the parser.ts capture behavior so the spec and implementation remain
consistent.
🪄 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: 03d50858-88bc-431f-9bda-6fce98b92a30
📒 Files selected for processing (41)
packages/cli/test/db-scripts.test.tspackages/core/src/config.tspackages/core/src/index.tspackages/dashboard/src/app/components/Epics.tsxpackages/dashboard/src/db-deps.tspackages/dashboard/src/wire.tspackages/dashboard/test/app.test.tsxpackages/dashboard/test/epics-deps.test.tspackages/dashboard/test/epics.test.tsxpackages/dispatcher/src/auto-dispatch.tspackages/dispatcher/src/db/migrations/010_epics_ref_key.sqlpackages/dispatcher/src/epic-store/file-poll-gateway.tspackages/dispatcher/src/epic-store/index.tspackages/dispatcher/src/epics-cache.tspackages/dispatcher/src/main.tspackages/dispatcher/src/poller-gateway.tspackages/dispatcher/src/poller.tspackages/dispatcher/src/recommender-run.tspackages/dispatcher/src/workflow-record.tspackages/dispatcher/src/workflows/recommender.tspackages/dispatcher/test/auto-dispatch.test.tspackages/dispatcher/test/db.test.tspackages/dispatcher/test/epic-store/file-auto-dispatch-integration.test.tspackages/dispatcher/test/epic-store/file-poll-gateway.test.tspackages/dispatcher/test/epic-store/file-review-resume-integration.test.tspackages/dispatcher/test/epic-store/file-watcher-integration.test.tspackages/dispatcher/test/epic-store/selector.test.tspackages/dispatcher/test/epics-cache.test.tspackages/dispatcher/test/poller.test.tspackages/dispatcher/test/recommender-run.test.tspackages/dispatcher/test/recommender-workflow.test.tspackages/dispatcher/test/state-issue.test.tspackages/state-issue/src/parser.tspackages/state-issue/src/schema.v1.tspackages/state-issue/test/fuzz.test.tspackages/state-issue/test/parser.test.tspackages/state-issue/test/sample-states.tspackages/state-issue/test/validate.test.tsplanning/issues/200/decisions.mdplanning/issues/200/plan.mdschemas/state-issue.v1.md
A transient failure of the per-PR reviews API call (rate limit, network) threw out of fetchPrSnapshot, unlike the sibling `pr view` fetch which already degrades to null. Wrap it to match — a stale/unreachable PR snapshot degrades to "no PR" instead of propagating. Pass `env` to the `gh` helper so argv[0] resolves against the current PATH (behavior-identical in prod), which lets a fake gh on PATH drive both failure branches under test.
…slug wording The schema doc claimed in-flight `<ref>` is kebab-case and rule 4 numeric-checks all #N refs, but validate.ts only constrains Ready epics and Blocked blockers and parser.ts captures any non-space/non-\* stem for in-flight refs (file-mode Epic slugs, incl. dotted `v1.2-rollout`). Correct the schema doc and the authoritative build spec to match. Pin the behavior: a validate test for a slug in-flight ref, and a dotted-slug arm in the round-trip fuzz generator.
Owner
Author
|
Addressed the two
Pinned the behavior: a |
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 #200
Completes the three documented Phase-1/2 file-mode gaps from Epic #190 (PR #198) so a file-backed Epic reaches parity with a GitHub-backed one: PR-review resume, recommender + auto-dispatch, and browse-cache/dashboard visibility. Each gap routes an already-mode-aware seam through to a path that was still GitHub-hardcoded, reusing the existing
makeRouting*Gateway/readEpicStoreConfigrouting pattern.What changed
packages/dispatcher/src/epic-store/file-poll-gateway.ts— a file Epic's PR resolves from the Epic file's durablemeta.prstamp (via new by-PR-numberprSnapshot/prLifecycle), so review-resume + merged/closed reconcile work in file mode. File-vs-github is discriminated byepicFileExists, not a numeric heuristic.packages/state-issue/—InFlightItem.issuewidenednumber → string(numeric Epic number or file-mode slug); parser reads#([^*\s]+); stays schema v1 (additive, byte-identical round-trip holds).packages/dispatcher/src/epic-store/index.ts—makeRoutingStateGateway(mirrors the epic/poll routers).packages/dispatcher/src/auto-dispatch.ts+main.ts— auto-dispatch reads state via the routing gateway and enqueues file Epics by slug (parseEpicRef); file-mode repos run with a sentinel state-issue0the file gateway ignores.packages/dispatcher/src/workflows/recommender.ts+recommender-run.ts+packages/core/src/config.ts— the recommender's state I/O is routed; its run is reachable for file mode ([epic_store]parsed into config;resolveRecommenderOptionssentinel-0 gate; the prompt reframes for the file store).packages/dispatcher/src/db/migrations/010_epics_ref_key.sql+epics-cache.ts— the Epic browse cache is re-keyed(repo, number) → (repo, ref)so file Epics are cached;refreshEpicsroutes per mode.packages/dashboard/—EpicCardcarriesref+ nullablenumber; the card renders via the existing<EpicRef>(#Nlabel orfile://slug link); the workflow/decision joins key onepic_ref.Why these changes
PRs/reviews are GitHub-native in both modes — file mode just can't resolve the PR via
Closes #<n>, so it resolves via the Epic file'smeta.pr(the same keyfindEpicPralready uses). The recommender/auto-dispatch loop and the browse cache were GitHub-hardcoded; routing them through the existing per-repo gateway resolvers (and re-keying the cache on the canonicalref, mirroring migration 009'sworkflows.epic_ref) is what surfaces file Epics end to end. TheInFlightItemwidening is additive — numeric refs render identically, so the state-issue round-trip invariant is preserved. Full rationale inplanning/issues/200/decisions.md.Acceptance criteria
CHANGES_REQUESTEDreview (and reconciles on merge/close), resolving its PR frommeta.pr. Evidenced by the integration testpackages/dispatcher/test/epic-store/file-review-resume-integration.test.ts(drives the realrunPollerpath) +file-poll-gateway.test.ts.state_fileand dispatches file Epics by slug; the recommender run is reachable and frames its prompt for the file store. Evidenced by the integration testpackages/dispatcher/test/epic-store/file-auto-dispatch-integration.test.ts(drives the realreadState → autoDispatchpath) +auto-dispatch.test.ts,recommender-run.test.ts,recommender-workflow.test.ts. (Live recommender ranking quality over file Epics is operator-smoke, per feat(epic-store): file-backed Epic store (opt-in hybrid) #190's file-mode precedent — see Out of scope.)file://slug links. Evidenced bypackages/dispatcher/test/epics-cache.test.ts,packages/dashboard/test/epics-deps.test.ts, andpackages/dashboard/test/epics.test.tsx.epics-deps.test.ts/file-auto-dispatch-integration.test.ts.Verification
bun test— 1286 pass, 0 fail (122 files).bun run typecheckclean.bun run lint+bun run formatclean.bun test packages/dispatcher/test/epic-store/file-review-resume-integration.test.ts— real poller resume of a file Epic.bun test packages/dispatcher/test/epic-store/file-auto-dispatch-integration.test.ts— realstate_file→ auto-dispatch by slug.bun test packages/dispatcher/test/epics-cache.test.ts packages/dashboard/test/epics-deps.test.ts— file Epics cached + surfaced by ref.packages/dispatcher/test/db.test.ts).Stumbling points
origin/mainbefore starting.stateIssue(prompt, run-options, surfacer). Rather than thread anumber | fileTargetunion through ~8 sites, file mode uses a sentinel0the routing state gateway ignores; the prompt branches on the explicitepicStoremode. The standalonedispatchRecommenderhelper was droppingepicStore(the daemon path forwarded it) — caught in self-review and fixed with a prompt-capture test.#<ref>regexes were widened from[\w-]to the real delimiters (space /**) to keep the round-trip invariant intact for a dotted slug.Suggested CLAUDE.md updates
state-issueCLAUDE.mdcould note that#<ref>in the In-flight section may now be a file-mode slug (not just a number), and that the round-trip invariant depends on the ref containing no space/*.Out of scope
reconcilers/pr-divergence.ts) is still numeric-only; making it file-mode-aware is tracked separately (it's the reconciler defect area, fix(dispatcher): open-PR divergence reconciler reset an approved branch to main and closed the PR (data-loss) #201).mm dispatch <slug>(which already works); a slug-aware dashboard dispatch route is a follow-up.Summary by CodeRabbit
New Features
Improvements
Database Changes