chore: roadmap planning — 6 spec closures, 3 new roadmap specs, FSM short-circuit#33
Merged
darko-mijic merged 3 commits intomainfrom Mar 12, 2026
Merged
chore: roadmap planning — 6 spec closures, 3 new roadmap specs, FSM short-circuit#33darko-mijic merged 3 commits intomainfrom
darko-mijic merged 3 commits intomainfrom
Conversation
… files The decider previously only bypassed FSM checks for new files with unlock-reason, blocking legitimate retroactive completions of existing specs. This removes the isNewFile constraint, making unlock-reason a universal FSM escape hatch for both new files and existing files. Adds test scenario verifying existing file with unlock-reason bypasses FSM check for roadmap → completed transition.
…hort-circuit Spec closures (retroactive completions): - ArchitectureDiagramAdvanced, ArchitectureDiagramCore → completed - DocsConsolidationStrategy → completed (all 16 deliverables) - StepLintExtendedRules → completed (all 6 deliverables) - KebabCaseSlugs, RichContentHelpersTesting → completed - DataAPIPlatformIntegration → completed (split into dedicated specs) New roadmap specs: - SetupCommand (Phase 45) — interactive project initialization - MCPServerIntegration (Phase 46) — MCP server for Claude Code - MonorepoSupport (Phase 100) — cross-package queries (deferred) Other changes: - TraceabilityGenerator spec rewritten for codec architecture - FSM short-circuit in process-api CLI (~2x faster for static queries) - lint-steps help text updated (8 → 12 rules) - INDEX.md broken link fixed (PUBLISHING.md → MAINTAINERS.md) - Deleted obsolete _claude-md/workflow/session-workflows.md
📝 WalkthroughWalkthroughThe PR marks multiple delivery process specifications as completed with unlock reasons, introduces new BDD specifications for MCP server integration, monorepo support, and setup command features, implements FSM short-circuiting in the CLI for static queries, adjusts process guard validation to bypass FSM checks when unlock reasons are present, and reorganizes documentation references from PUBLISHING to MAINTAINERS. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@delivery-process/specs/monorepo-support.feature`:
- Around line 69-90: Add an explicit edge-case scenario exercising overlapping
package globs: create a new Scenario (e.g., "Package field selects first
matching glob when globs overlap") that uses a multi-package config with two
overlapping globs (for example "packages/*" and "packages/platform-*") where the
config ordering is explicit, provide a source file path that matches both globs
(e.g., "packages/platform-core/src/events.ts"), scan and extract the file, and
assert the resulting ExtractedPattern.package equals the package from the
first-listed glob; name the scenario to complement the existing "Package field
is set from matching glob" and "Package field is undefined without packages
config" scenarios so behavior is unambiguous.
- Around line 41-58: The spec mixes config types by saying "packages without
their own `features` or `stubs` inherit from top-level `sources`;" update the
text and acceptance criteria to be explicit: state that PackageConfig entries
inherit top-level `sources` only for TypeScript/source globs (used to build the
total source set), and separately state whether `features` and `stubs` are
inherited from top-level (e.g., "packages without their own `features` or
`stubs` inherit top-level `features`/`stubs`" or "packages do not inherit
top-level `features`/`stubs` unless explicitly set"); then adjust the Scenario
"Multi-package config is parsed and validated" to assert the exact inheritance
behavior for `sources`, `features`, and `stubs` so the resolver behavior and
PackageConfig schema are unambiguous.
In `@delivery-process/specs/setup-command.feature`:
- Around line 80-81: Define the --yes behavior unambiguously by updating the
feature text and scenarios so that "--yes" explicitly skips all prompts and
defaults to overwriting existing files (i.e., resolves the
overwrite-confirmation conflict); change the sentence to "The --yes flag skips
all prompts and defaults to overwriting existing files" and update the scenarios
referencing overwrite-confirmation (the examples around lines noted in the
comment) to expect silent overwrite when --yes is present, and if you need the
opposite behavior add a separate flag (e.g., --no-overwrite or
--abort-on-conflict) with its own scenario instead of leaving --yes ambiguous.
- Around line 49-50: Update the init command invariant to detect
delivery-process.config.js as well as delivery-process.config.ts so the init
flow will prompt instead of writing a new config; specifically, modify wherever
the spec or safety-checks reference delivery-process.config.ts (the init command
and related checks mentioned in the comment) to include
delivery-process.config.js as an acceptable existing contract and update the
three other occurrences referenced (lines ~111-113 and ~132-136) to perform the
same check for .js as well as .ts.
- Around line 142-145: The spec currently makes the init command set
package.json's "type" to "module" by default; change it to preserve existing
package.json.module type unless the user explicitly opts into an ESM migration:
update the setup-command.feature scenario for the init command to remove the
default mutation of the "type" field and instead add an explicit prompt/flag for
ESM migration, ensure the acceptance criteria no longer require "type": "module"
to be present (remove that check from the scenario), and mention the recommended
flow using delivery-process.config.js + tsx wrappers as the default path.
In `@src/cli/lint-steps.ts`:
- Line 135: Update the CLI help text for the "keyword-in-description" lint rule
in lint-steps.ts so it matches the implemented checker: mention that description
lines starting with Given, When, Then, And, or But are flagged (the behavior is
implemented in src/lint/steps/feature-checks.ts). Locate the help/description
string for the "keyword-in-description" rule in lint-steps.ts and expand it to
list "And" and "But" alongside "Given/When/Then" so the CLI documentation
reflects the actual rule.
In `@src/cli/process-api.ts`:
- Around line 512-599: The short-circuit path (tryFsmShortCircuit) calls
fsmIsValidTransition, fsmGetValidTransitionsFrom and fsmGetProtectionSummary
without verifying the given status strings, which leads to runtime crashes or
invalid returns; fix by checking that the provided status keys exist in the FSM
valid-status map (VALID_TRANSITIONS) before calling those helpers and throw a
QueryApiError('INVALID_ARGUMENT', '<usage>') when unknown. Specifically: in
tryFsmShortCircuit before calling fsmIsValidTransition (case
'isValidTransition') verify VALID_TRANSITIONS[from] and VALID_TRANSITIONS[to];
before returning fsmGetValidTransitionsFrom (case 'getValidTransitionsFrom')
validate VALID_TRANSITIONS[status]; and before calling fsmGetProtectionSummary
(case 'getProtectionInfo') validate VALID_TRANSITIONS[status]; keep the existing
usage strings and reuse fsmValidateTransition for the 'checkTransition' case
which already validates.
In `@src/lint/process-guard/decider.ts`:
- Around line 301-303: The unconditional skip on transition.hasUnlockReason in
decider.ts is too broad; instead of always continuing when
transition.hasUnlockReason === true, gate the bypass to only the
retroactive-completion path and only when the unlock reason has been validated
by derive-state.ts (the non-placeholder / length check done after
detect-changes.ts). Update the logic in the function that evaluates transitions
(referencing transition.hasUnlockReason and checkProtectionLevel()) to: 1)
consult the derived validation flag produced in derive-state.ts (e.g., a field
like transition.unlockReasonValidated or fileState.unlockReasonValid) rather
than the raw presence bit from detect-changes.ts, and 2) only allow the FSM
bypass when the transition target is the retroactive completion route to
completed (not for arbitrary transitions). Leave checkProtectionLevel() behavior
unchanged for terminal exemptions. This ensures only validated unlock reasons on
the retro-complete path skip the FSM check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 784699b8-b6a4-438d-a296-d81621a3f2dc
⛔ Files ignored due to path filters (1)
_claude-md/workflow/session-workflows.mdis excluded by none and included by none
📒 Files selected for processing (21)
delivery-process/specs/architecture-diagram-advanced.featuredelivery-process/specs/architecture-diagram-core.featuredelivery-process/specs/data-api-cli-ergonomics.featuredelivery-process/specs/data-api-platform-integration.featuredelivery-process/specs/docs-consolidation-strategy.featuredelivery-process/specs/mcp-server-integration.featuredelivery-process/specs/monorepo-support.featuredelivery-process/specs/setup-command.featuredelivery-process/specs/step-lint-extended-rules.featuredelivery-process/specs/traceability-enhancements.featuredelivery-process/specs/traceability-generator.featuredocs/INDEX.mdsrc/cli/lint-steps.tssrc/cli/process-api.tssrc/lint/process-guard/decider.tstests/features/behavior/kebab-case-slugs.featuretests/features/behavior/rich-content-helpers.featuretests/features/validation/process-guard.featuretests/steps/behavior/kebab-case-slugs.steps.tstests/steps/behavior/rich-content-helpers.steps.tstests/steps/validation/process-guard.steps.ts
Comment on lines
+49
to
+50
| **Invariant:** The init command reads the target directory for package.json, | ||
| tsconfig.json, delivery-process.config.ts, and monorepo markers before prompting |
There was a problem hiding this comment.
Detect existing delivery-process.config.js before writing a new config.
The safety checks only look for delivery-process.config.ts. In a repo already using delivery-process.config.js, init can miss the existing contract and create a second config instead of prompting.
♻️ Suggested spec adjustment
- tsconfig.json, delivery-process.config.ts, and monorepo markers before prompting
+ tsconfig.json, delivery-process.config.ts or delivery-process.config.js, and
+ monorepo markers before prompting
...
- Given a directory with an existing delivery-process.config.ts
+ Given a directory with an existing delivery-process.config.ts or
+ delivery-process.config.jsAlso applies to: 111-113, 132-136
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@delivery-process/specs/setup-command.feature` around lines 49 - 50, Update
the init command invariant to detect delivery-process.config.js as well as
delivery-process.config.ts so the init flow will prompt instead of writing a new
config; specifically, modify wherever the spec or safety-checks reference
delivery-process.config.ts (the init command and related checks mentioned in the
comment) to include delivery-process.config.js as an acceptable existing
contract and update the three other occurrences referenced (lines ~111-113 and
~132-136) to perform the same check for .js as well as .ts.
… FSM input - Scope unlock-reason FSM bypass to validated reasons targeting completed only (was unconditionally bypassing on raw presence bit from detect-changes) - Add parseProcessStatus() to validate CLI input before FSM short-circuit calls (invalid status values previously caused runtime TypeError crashes) - Expand keyword-in-description help text to include And/But keywords - Clarify monorepo config inheritance wording and add overlapping globs scenario - Detect .config.js alongside .config.ts in setup-command spec - Resolve --yes vs overwrite conflict with --force flag for destructive ops - Make type:module opt-in via --esm flag instead of default mutation
This was referenced Mar 12, 2026
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
roadmapstatusSpec Closures
roadmap→completed(all deliverables terminal)roadmap→completed(all deliverables terminal)roadmap→completed(all 16 deliverables terminal)roadmap→completed(all 6 deliverables terminal)roadmap→completed(all 3 deliverables terminal)roadmap→completed(3 deliverables added + terminal)roadmap→completed(split into dedicated specs)All closures use
@libar-docs-unlock-reason:Retroactive-completion(orSplit-into-dedicated-specs).New Roadmap Specs
npx @libar-dev/delivery-process initsetup flowOther Changes
process-api.ts—tryFsmShortCircuit()dispatches directly to FSM modules forisValidTransition,getValidTransitionsFrom,checkTransition,getProtectionInfowithout building the pipelineisNewFileconstraint from unlock-reason bypass incheckStatusTransitions(), enabling retroactive completions of existing filesPUBLISHING.md→MAINTAINERS.md)_claude-md/workflow/session-workflows.mdTest Plan
pnpm typecheck— no type errorspnpm test— 8,104 tests pass (125 files)pnpm lint— no lint errorspnpm validate:all— 46/46 phases pass, 0 anti-patternsSummary by CodeRabbit
New Features
Improvements