feat(missions): per-mission taskPrefix override for triaged task ids#1930
feat(missions): per-mission taskPrefix override for triaged task ids#1930flexi767 wants to merge 1 commit into
Conversation
Add an optional `taskPrefix` field on Mission so a single mission's tickets can use a distinct id prefix (e.g. ERR-) while the rest of the board keeps the project-wide prefix (default FN-). Previously taskPrefix was project-scoped only; flipping it changed every new ticket board-wide including autopilot. The distributed id allocator was already multi-prefix, so this only threads a per-mission prefix through triage: - Mission.taskPrefix (nullable) persisted via a new additive migration (v140). - MissionStore.triageFeature passes it into TaskCreateInput.taskPrefix (a transient minting hint, not persisted on the task). - createTaskWithDistributedReservation prefers input.taskPrefix over settings. - buildCommitMsgTrailerHook derives the strip PREFIX from the task id itself rather than the project-wide option, so ERR-5 strips correctly while the project prefix is still FN (fixes all engine call sites at once). - Mission create + edit UI (MissionManager) exposes a Task prefix input; mission-routes POST/PATCH validate it (letter-led alphanumeric, uppercased). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Greptile SummaryThis PR adds per-mission task prefixes for triaged task IDs. The main changes are:
Confidence Score: 4/5The mission edit flow needs a fix before merging.
packages/dashboard/app/components/MissionManager.tsx; packages/engine/src/worktree-hooks.ts Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
UI[Mission create/edit form] --> API[Mission routes validation]
API --> Store[MissionStore persists nullable taskPrefix]
Store --> Triage[triageFeature reads mission taskPrefix]
Triage --> TaskCreate[TaskCreateInput transient taskPrefix]
TaskCreate --> Allocator[Distributed task ID reservation]
Allocator --> Hook[Commit hook derives strip prefix from task ID]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
UI[Mission create/edit form] --> API[Mission routes validation]
API --> Store[MissionStore persists nullable taskPrefix]
Store --> Triage[triageFeature reads mission taskPrefix]
Triage --> TaskCreate[TaskCreateInput transient taskPrefix]
TaskCreate --> Allocator[Distributed task ID reservation]
Allocator --> Hook[Commit hook derives strip prefix from task ID]
Reviews (1): Last reviewed commit: "feat(missions): per-mission taskPrefix o..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| autopilotEnabled: missionForm.autopilotEnabled, | ||
| baseBranch: missionForm.baseBranch.trim() || "", | ||
| branchStrategy, | ||
| taskPrefix: missionForm.taskPrefix.trim() || undefined, |
There was a problem hiding this comment.
When a mission already has taskPrefix set and the edit form is cleared, this line converts the empty input to undefined. JSON.stringify drops that key, so the PATCH route skips taskPrefix and the old prefix remains stored; future triaged tasks keep using the stale mission prefix instead of inheriting the project prefix.
| const derivedPrefix = /^[A-Za-z][A-Za-z0-9]*-\d+$/.test(trimmedTaskId) | ||
| ? trimmedTaskId.replace(/-\d+$/, "").toUpperCase() | ||
| : ""; | ||
| const taskPrefix = (derivedPrefix || options.taskPrefix || "FN").trim() || "FN"; |
There was a problem hiding this comment.
When taskId does not match the new numeric-id regex, this falls back to options.taskPrefix and later interpolates it directly into the generated case pattern and sed regex. A configured prefix containing regex or shell metacharacters can make the hook strip the wrong value or expand shell variables, producing an incorrect Fusion-Task-Id trailer for valid commits.
📝 WalkthroughWalkthroughAdds a per-mission ChangesPer-mission task prefix
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant MissionManager
participant LegacyAPI
participant MissionRoutes
participant MissionStore
participant TaskStore
MissionManager->>LegacyAPI: createMission/updateMission({ taskPrefix })
LegacyAPI->>MissionRoutes: POST/PATCH /api/missions
MissionRoutes->>MissionRoutes: validateTaskPrefix(taskPrefix)
MissionRoutes->>MissionStore: createMission/updateMission(input)
MissionStore-->>MissionRoutes: mission
Note over MissionStore,TaskStore: later, during triage
MissionStore->>TaskStore: createTask({ taskPrefix: mission.taskPrefix })
TaskStore->>TaskStore: reserve id using input.taskPrefix override
TaskStore-->>MissionStore: minted taskId
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/dashboard/app/components/MissionManager.tsx (1)
1657-1668: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEditing a mission cannot clear an existing
taskPrefix.The update payload sends
taskPrefix: missionForm.taskPrefix.trim() || undefined.JSON.stringifydrops keys withundefinedvalues, so clearing the field in the UI and saving omitstaskPrefixfrom the PATCH body entirely. The server'sif (taskPrefix !== undefined)guard inmission-routes.tsthen never fires, so the previously-set prefix is never reverted to "inherit project prefix" — contradicting the intended empty-means-inherit behavior for edits.
baseBranchalready avoids this by sending""explicitly instead ofundefined(line 1665);taskPrefixshould do the same.🐛 Proposed fix
const updates: Record<string, unknown> = { title: missionForm.title.trim(), description: missionForm.description.trim() || undefined, status: missionForm.status, autopilotEnabled: missionForm.autopilotEnabled, baseBranch: missionForm.baseBranch.trim() || "", branchStrategy, - taskPrefix: missionForm.taskPrefix.trim() || undefined, + taskPrefix: missionForm.taskPrefix.trim() || "", };🤖 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/dashboard/app/components/MissionManager.tsx` around lines 1657 - 1668, The edit flow in MissionManager’s update payload is dropping an intentionally cleared taskPrefix because it sends undefined, so the PATCH body omits the field and the server never resets it to inherit the project prefix. Update the MissionManager save logic where the edits are built so taskPrefix is sent explicitly as an empty string when cleared, matching the existing baseBranch handling, and keep the mission-routes.ts update guard compatible with that payload shape.
🧹 Nitpick comments (2)
packages/engine/src/worktree-hooks.ts (1)
210-219: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLGTM — prefix derivation logic is sound.
Regex-based derivation correctly handles per-mission prefixes, and the fallback to
options.taskPrefix/"FN"for malformed task ids is a sensible safety net. Consider adding a unit test for the fallback path (e.g., a taskId that doesn't match<PREFIX>-<digits>) to lock in that behavior, but it's not blocking.🤖 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/engine/src/worktree-hooks.ts` around lines 210 - 219, Add a unit test for the task prefix fallback behavior in worktree-hooks: verify that the logic in the taskId-derived prefix path falls back to options.taskPrefix, and then to "FN", when taskId does not match the "<PREFIX>-<digits>" pattern. Use the prefix derivation block around trimmedTaskId, derivedPrefix, and taskPrefix to locate the code, and cover at least one malformed taskId case so the fallback behavior stays locked in.packages/core/src/mission-store.ts (1)
654-698: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueNo format validation of
taskPrefixat the store layer.
createMission/updateMissionpersistinput.taskPrefix/updates.taskPrefixverbatim with no shape check (letter-led alphanumeric, uppercase). Per the PR description, validation lives in the dashboard mission-routes handlers, so any direct caller ofMissionStore(scripts, plugins, future internal callers, orapplyMissionHierarchySnapshot) can persist a malformed prefix that later flows into task-id minting. This matches the existing convention forbranchStrategy/baseBranch(also unvalidated here), so it's not a regression, but worth a defense-in-depth note.Also applies to: 1261-1311
🤖 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/core/src/mission-store.ts` around lines 654 - 698, `MissionStore.createMission` (and the matching update path) currently persists `taskPrefix` without validating its shape, so direct callers can store malformed prefixes that later break task-id generation. Add defense-in-depth validation in `MissionStore` itself, reusing the same letter-led uppercase alphanumeric rules enforced by the dashboard mission-routes handlers before writing `input.taskPrefix`/`updates.taskPrefix` to the database. Make sure the same check also covers internal entry points like `applyMissionHierarchySnapshot` so all store-level writes stay 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.
Outside diff comments:
In `@packages/dashboard/app/components/MissionManager.tsx`:
- Around line 1657-1668: The edit flow in MissionManager’s update payload is
dropping an intentionally cleared taskPrefix because it sends undefined, so the
PATCH body omits the field and the server never resets it to inherit the project
prefix. Update the MissionManager save logic where the edits are built so
taskPrefix is sent explicitly as an empty string when cleared, matching the
existing baseBranch handling, and keep the mission-routes.ts update guard
compatible with that payload shape.
---
Nitpick comments:
In `@packages/core/src/mission-store.ts`:
- Around line 654-698: `MissionStore.createMission` (and the matching update
path) currently persists `taskPrefix` without validating its shape, so direct
callers can store malformed prefixes that later break task-id generation. Add
defense-in-depth validation in `MissionStore` itself, reusing the same
letter-led uppercase alphanumeric rules enforced by the dashboard mission-routes
handlers before writing `input.taskPrefix`/`updates.taskPrefix` to the database.
Make sure the same check also covers internal entry points like
`applyMissionHierarchySnapshot` so all store-level writes stay consistent.
In `@packages/engine/src/worktree-hooks.ts`:
- Around line 210-219: Add a unit test for the task prefix fallback behavior in
worktree-hooks: verify that the logic in the taskId-derived prefix path falls
back to options.taskPrefix, and then to "FN", when taskId does not match the
"<PREFIX>-<digits>" pattern. Use the prefix derivation block around
trimmedTaskId, derivedPrefix, and taskPrefix to locate the code, and cover at
least one malformed taskId case so the fallback behavior stays locked in.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ee38844e-e6c9-4514-8ed5-c763bf6404b2
📒 Files selected for processing (12)
packages/core/src/__tests__/mission-store.test.tspackages/core/src/db.tspackages/core/src/mission-store.tspackages/core/src/mission-types.tspackages/core/src/store.tspackages/core/src/types.tspackages/dashboard/app/api/legacy.tspackages/dashboard/app/components/MissionManager.tsxpackages/dashboard/app/components/mission-types.tspackages/dashboard/src/mission-routes.tspackages/engine/src/__tests__/worktree-hooks.test.tspackages/engine/src/worktree-hooks.ts
Problem
taskPrefixis a project-wide setting (defaultFN). There's no way to make a single mission's tickets use a different prefix (e.g.ERR-) while everything else keepsFN-. Flipping the project setting changes the prefix for every new ticket board-wide — including whatever autopilot is minting on other missions concurrently.Solution
Add an optional per-mission
taskPrefix. The distributed id allocator is already multi-prefix (reserveDistributedTaskId({ prefix }), per-prefix sequences), so this change only threads a per-mission prefix through triage — no allocator changes.Mission.taskPrefix(nullable, optional) — persisted via a new additive migration (schema v140,addColumnIfMissing, no backfill). Mirrors the existingbranchStrategypersistence shape.MissionStore.triageFeaturepasses the mission's prefix intoTaskCreateInput.taskPrefix— a transient minting hint, not persisted on the task.createTaskWithDistributedReservationprefersinput.taskPrefixover the projectsettings.taskPrefixfor the id reservation.buildCommitMsgTrailerHooknow derives the stripPREFIXfrom the task id itself (ERR-5→ERR) rather than the project-wideoptions.taskPrefix, so per-mission prefixes strip correctly while the project prefix staysFN. This fixes all five engine call sites at once, backward-compatibly.MissionManager) exposes a "Task prefix" input;mission-routesPOST/PATCH validate it (letter-led alphanumeric, uppercased; empty ⇒ inherit project prefix).Tests
mission-store.test.ts: create/update round-trip, legacy-NULL read, and triage mints with the mission's prefix (/^ERR-\d+$/).worktree-hooks.test.ts: commit-msg hook derives the strip prefix from the task id, ignoring a mismatchedoptions.taskPrefix.All touched suites green;
tsc --noEmitclean across core / engine / dashboard.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores