feat(missions): per-mission taskPrefix override for triaged task ids#2334
feat(missions): per-mission taskPrefix override for triaged task ids#2334flexi767 wants to merge 12 commits 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>
…-prefix # Conflicts: # packages/core/src/db.ts
…ck prefix Address greptile feedback on PR Runfusion#1930: - Mission edit save sends taskPrefix:null when the field is cleared so JSON retains the key and PATCH clears the stored mission override. - Commit-msg hook quotes "$PREFIX" in case and escapes sed ERE metacharacters on the options.taskPrefix fallback path.
…on taskPrefix Rebase the feat/per-mission-task-prefix work onto main after the SQLite→PostgreSQL cutover. Take main's stubbed db.ts and modular store.ts, then re-port the feature: - missions.task_prefix column (schema + migration 0008 + schema-applier) - AsyncMissionStore CRUD/upsert/triage threads Mission.taskPrefix - createTaskBackend + createTaskWithDistributedReservation honor input.taskPrefix - Keep greptile fixes: null PATCH clear, commit-msg prefix escape/quote - Port mission taskPrefix tests to PG harness; mock dashboard route tests Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
escapeSedEre left `/` raw, so a fallback prefix like TEAM/API produced invalid sed `s/^TEAM/API-//i` and could abort the commit-msg hook. Escape `/` with the other ERE metacharacters; cover with a unit test. Greptile: databaseId 3583850800 on PR Runfusion#1930.
MissionManager state used mission-types.MissionWithSummary while fetchMissions returned api/legacy.MissionWithSummary. taskPrefix was string vs string|null, so the Array.isArray ternary widened to a dual-array union and failed setState (TS2345). Align component Mission.taskPrefix with the API and annotate the normalized list as MissionWithSummary[].
JSON.stringify produced PREFIX="$(id)" which /bin/sh expands via command substitution when the commit-msg hook runs. Emit POSIX single-quoted literals via shellSingleQuote (embedded ' → '\''). Apply the same quoting to TRAILER_NAME and CO_AUTHOR_TRAILER. Cover $(id), ; rm -rf, and O'Brien. Greptile P1 security: databaseIds 3583890011, 3583943640 on PR Runfusion#1930.
📝 WalkthroughWalkthroughChangesMission task prefix overrides
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Dashboard
participant MissionRoutes
participant MissionStore
participant TaskStore
participant TaskAllocator
Dashboard->>MissionRoutes: submit mission taskPrefix
MissionRoutes->>MissionStore: validate and persist taskPrefix
MissionStore-->>Dashboard: return mission
Dashboard->>TaskStore: triage mission feature
TaskStore->>TaskAllocator: create task with taskPrefix
TaskAllocator-->>TaskStore: return prefixed task ID
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Greptile SummaryThis PR adds an optional per-mission
Confidence Score: 5/5Safe to merge. All write paths consistently map cleared prefixes to NULL, the migration is additive and idempotent, and the commit-hook quoting changes eliminate the previously identified command-injection surface. The change is well-scoped and well-tested: 86 PostgreSQL store/schema tests, 17 worktree-hook unit tests covering empty, special-char, injection, and single-quote edge cases, 5 dashboard task-prefix tests, and route-level regression tests all pass. The null=clear vs omit=no-change contract is enforced end-to-end. The POSIX single-quoting and ERE-escaping logic is verifiably correct for the full space of inputs the validator allows, and is defense-in-depth for the fallback path. No files require special attention. The security-sensitive shell-quoting logic in packages/engine/src/worktree-hooks.ts has dedicated test coverage for injection, backtick, slash, and embedded-single-quote cases. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant UI as MissionManager (UI)
participant RT as mission-routes (API)
participant MS as AsyncMissionStore
participant DB as PostgreSQL
participant TA as Task Allocator
participant WH as worktree-hooks
Note over UI,DB: Mission create / edit
UI->>RT: "POST /missions { taskPrefix: ERR }"
RT->>RT: validateTaskPrefix → ERR
RT->>MS: "createMission({ taskPrefix: ERR })"
MS->>DB: "INSERT missions (task_prefix = ERR)"
UI->>RT: "PATCH /missions/:id { taskPrefix: null }"
RT->>RT: validateTaskPrefix(null) → undefined
RT->>MS: "updateMission(id, { taskPrefix: undefined })"
MS->>DB: "UPDATE missions SET task_prefix = NULL"
Note over MS,WH: Feature triage
MS->>MS: triageFeature(featureId)
MS->>MS: lookup mission.taskPrefix → ERR
MS->>TA: "createTask({ taskPrefix: ERR })"
TA->>TA: "prefix = input.taskPrefix ?? settings.taskPrefix ?? FN"
TA-->>MS: "Task { id: ERR-42 }"
Note over WH: Commit-msg hook generation
MS->>WH: "installCommitMsgHook(taskId=ERR-42, taskPrefix=FN)"
WH->>WH: "derivedPrefix = ERR from ERR-42"
WH->>WH: "taskPrefix = derivedPrefix || options.taskPrefix || FN"
WH->>WH: "PREFIX=shellSingleQuote(ERR)"
WH->>WH: "sed = shellSingleQuote(s/^ERR-//i)"
WH-->>MS: "hook script with PREFIX='ERR'"
%%{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"}}}%%
sequenceDiagram
participant UI as MissionManager (UI)
participant RT as mission-routes (API)
participant MS as AsyncMissionStore
participant DB as PostgreSQL
participant TA as Task Allocator
participant WH as worktree-hooks
Note over UI,DB: Mission create / edit
UI->>RT: "POST /missions { taskPrefix: ERR }"
RT->>RT: validateTaskPrefix → ERR
RT->>MS: "createMission({ taskPrefix: ERR })"
MS->>DB: "INSERT missions (task_prefix = ERR)"
UI->>RT: "PATCH /missions/:id { taskPrefix: null }"
RT->>RT: validateTaskPrefix(null) → undefined
RT->>MS: "updateMission(id, { taskPrefix: undefined })"
MS->>DB: "UPDATE missions SET task_prefix = NULL"
Note over MS,WH: Feature triage
MS->>MS: triageFeature(featureId)
MS->>MS: lookup mission.taskPrefix → ERR
MS->>TA: "createTask({ taskPrefix: ERR })"
TA->>TA: "prefix = input.taskPrefix ?? settings.taskPrefix ?? FN"
TA-->>MS: "Task { id: ERR-42 }"
Note over WH: Commit-msg hook generation
MS->>WH: "installCommitMsgHook(taskId=ERR-42, taskPrefix=FN)"
WH->>WH: "derivedPrefix = ERR from ERR-42"
WH->>WH: "taskPrefix = derivedPrefix || options.taskPrefix || FN"
WH->>WH: "PREFIX=shellSingleQuote(ERR)"
WH->>WH: "sed = shellSingleQuote(s/^ERR-//i)"
WH-->>MS: "hook script with PREFIX='ERR'"
Reviews (4): Last reviewed commit: "fix(dashboard): validate mission prefix ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
This is ready for another review. The concrete use case is one Fusion project for one repository/deployment pipeline, with durable mission namespaces for separate workstreams ( I refreshed the branch onto current |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/engine/src/__tests__/worktree-hooks.test.ts (2)
96-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse JSDoc-style comments for
FNXCannotations.As per coding guidelines, it is preferred to use concise JSDoc-style comments (
/** ... */) instead of single-line comments forFNXCproduct area annotations.♻️ Proposed refactor
For example, update line 96 as follows:
-// FNXC:WorktreeHooks 2026-07-14-12:00: fallback options.taskPrefix is quoted in case and escaped for sed -E so metacharacters cannot break the hook or strip the wrong value (greptile P2 on PR `#1930`). +/** FNXC:WorktreeHooks 2026-07-14-12:00: fallback options.taskPrefix is quoted in case and escaped for sed -E so metacharacters cannot break the hook or strip the wrong value (greptile P2 on PR `#1930`). */Also applies to: 105-105, 113-113
🤖 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/__tests__/worktree-hooks.test.ts` at line 96, Convert the FNXC annotations near the worktree hook tests from single-line comments to concise JSDoc-style comments using /** ... */. Apply this consistently to the annotations at the referenced locations while preserving their existing text and surrounding test logic.Source: Coding guidelines
102-102: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
.toContainover.toMatchfor exact string assertions.For exact substring negations, using
.toContainis more readable and consistent with other assertions in this test suite (such as line 110). It avoids the need to heavily escape regex metacharacters ("leaning toothpick syndrome").♻️ Proposed refactor
- expect(hook).not.toMatch(/s\/\^A\.B\+C-\/\/i/); + expect(hook).not.toContain("s/^A.B+C-//i");- expect(cmdSub).not.toMatch(/PREFIX="\$\(id\)"/); + expect(cmdSub).not.toContain('PREFIX="$(id)"');Also applies to: 117-117
🤖 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/__tests__/worktree-hooks.test.ts` at line 102, Update the exact substring negations in the worktree hook tests to use the appropriate `.not.toContain` assertions instead of `.not.toMatch`, including the assertion near line 102 and the corresponding one near line 117; preserve the existing expected string contents without regex escaping.packages/core/src/mission-store.ts (1)
649-693: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing rationale comment for
taskPrefixclearing semantics.
createMission/updateMissionhere implement the same "write NULL to re-inherit the project prefix" contract asasync-mission-store-queries.ts, but only the async file documents it via an FNXC comment (citing PR#1930/ greptile P1). Without the same note here, a future edit toupdated.taskPrefix ?? null(Line 1293) ormission.taskPrefix ?? null(Line 681) could silently break clearing.As per coding guidelines: "Add
FNXC:Area-of-productcomments inyyyy-MM-dd-hh:mmformat describing important user-facing requirements, implementation rationale, or requirement changes... ensure important user requirements are encoded in code comments."📝 Suggested comment addition
branchStrategy: mission.branchStrategy ? JSON.stringify(mission.branchStrategy) : null, + // FNXC:MissionTaskPrefix 2026-07-19-12:53: write NULL when cleared so the mission re-inherits the project prefix (mirrors async-mission-store-queries.ts, PR `#1930`). taskPrefix: mission.taskPrefix ?? null,Also applies to: 1256-1306
🤖 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 649 - 693, Add an FNXC rationale comment in the synchronous mission creation and update paths, adjacent to the taskPrefix persistence expressions in createMission and the corresponding update logic, documenting that writing NULL clears the mission override so it re-inherits the project prefix and referencing the existing contract rationale. Use the required yyyy-MM-dd-hh:mm format and preserve the current NULL behavior.Source: Coding guidelines
🤖 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/dashboard/src/mission-routes.ts`:
- Around line 191-202: Update validateTaskPrefix to throw badRequest(...) for
both invalid type and invalid format cases, ensuring taskPrefix validation
failures are recognized as client errors and return HTTP 400 through
catchHandler.
In `@packages/engine/src/worktree-hooks.ts`:
- Line 269: Update the generated sed expression in the fallback task-ID branch
to use the existing shellSingleQuote helper, producing a POSIX single-quoted
shell literal after applying escapeSedEre. Preserve the current prefix-removal
behavior while ensuring taskPrefix characters, including backticks, cannot
trigger shell command substitution when the hook runs.
---
Nitpick comments:
In `@packages/core/src/mission-store.ts`:
- Around line 649-693: Add an FNXC rationale comment in the synchronous mission
creation and update paths, adjacent to the taskPrefix persistence expressions in
createMission and the corresponding update logic, documenting that writing NULL
clears the mission override so it re-inherits the project prefix and referencing
the existing contract rationale. Use the required yyyy-MM-dd-hh:mm format and
preserve the current NULL behavior.
In `@packages/engine/src/__tests__/worktree-hooks.test.ts`:
- Line 96: Convert the FNXC annotations near the worktree hook tests from
single-line comments to concise JSDoc-style comments using /** ... */. Apply
this consistently to the annotations at the referenced locations while
preserving their existing text and surrounding test logic.
- Line 102: Update the exact substring negations in the worktree hook tests to
use the appropriate `.not.toContain` assertions instead of `.not.toMatch`,
including the assertion near line 102 and the corresponding one near line 117;
preserve the existing expected string contents without regex escaping.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 598ac597-3d4c-44c3-91f2-a5194934b078
📒 Files selected for processing (22)
.changeset/mission-task-prefix.mdpackages/core/src/__tests__/postgres/mission-store.pg.test.tspackages/core/src/__tests__/postgres/schema-applier.test.tspackages/core/src/async-mission-store-queries.tspackages/core/src/async-mission-store.tspackages/core/src/mission-store.tspackages/core/src/mission-types.tspackages/core/src/postgres/index.tspackages/core/src/postgres/migrations/0026_mission_task_prefix.sqlpackages/core/src/postgres/schema-applier.tspackages/core/src/postgres/schema/project.tspackages/core/src/task-store/remaining-ops-4.tspackages/core/src/task-store/task-creation.tspackages/core/src/types.tspackages/dashboard/app/api/legacy.tspackages/dashboard/app/components/MissionManager.tsxpackages/dashboard/app/components/__tests__/MissionManager.task-prefix.test.tsxpackages/dashboard/app/components/mission-types.tspackages/dashboard/src/__tests__/mission-task-prefix-routes.test.tspackages/dashboard/src/mission-routes.tspackages/engine/src/__tests__/worktree-hooks.test.tspackages/engine/src/worktree-hooks.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/dashboard/app/components/__tests__/MissionManager.task-prefix.test.tsx (1)
164-165: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrevent test flakiness by using asynchronous DOM queries after state updates.
waitForresolves immediately once the mock is called, which happens before React finishes its re-render with the fetched data. The subsequent synchronousgetAllByRolequery is susceptible to race conditions and may throw if the button hasn't appeared in the DOM yet.Use an asynchronous query like
findAllByRoleto safely wait for the UI to update.🛠️ Proposed fix to ensure stable DOM querying
await waitFor(() => expect(mockFetchMission).toHaveBeenCalledWith("M-001", projectId)); - fireEvent.click(screen.getAllByRole("button", { name: "Edit mission" })[0]); + const editButtons = await screen.findAllByRole("button", { name: "Edit mission" }); + fireEvent.click(editButtons[0]);🤖 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/__tests__/MissionManager.task-prefix.test.tsx` around lines 164 - 165, Update the test after the mockFetchMission assertion to await screen.findAllByRole("button", { name: "Edit mission" }) instead of using the synchronous getAllByRole query, ensuring the button is present after React finishes rendering before firing the click.
🤖 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.
Nitpick comments:
In
`@packages/dashboard/app/components/__tests__/MissionManager.task-prefix.test.tsx`:
- Around line 164-165: Update the test after the mockFetchMission assertion to
await screen.findAllByRole("button", { name: "Edit mission" }) instead of using
the synchronous getAllByRole query, ensuring the button is present after React
finishes rendering before firing the click.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 76d35c95-7570-48f6-a67d-efd65ca1b47f
📒 Files selected for processing (2)
packages/dashboard/app/components/MissionManager.tsxpackages/dashboard/app/components/__tests__/MissionManager.task-prefix.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/dashboard/app/components/MissionManager.tsx
|
Thanks for explanation this makes sense |
Merge conflicts resolved (maintainer follow-up)This PR is CONFLICTING with current Fork push to → #2347 ( Resolution notes:
Prefer merging #2347 (or cherry-picking that tip onto this fork branch if you can push). |
Why
This supersedes #1930, which GitHub will not allow the author to reopen.
A Fusion project can represent one repository and deployment pipeline while durable missions represent distinct workstreams. In our project, general work uses
FN-*, ImageEditor usesIMG-*, native/mobile usesMOB-*, and production incidents useERR-*.Those IDs leave the board and become durable references in commit trailers, branch names, implementation notes, migration filenames, logs, and alerts. Labels do not provide that context outside the board, and splitting one repository into multiple Fusion projects would duplicate repository, agent, merge, and deployment configuration.
Temporarily changing the project-wide prefix is unsafe because autopilot can triage other missions concurrently and allocate their tasks under the temporary namespace. An optional mission override makes allocation deterministic while preserving the project prefix as the default.
What changed
Mission.taskPrefix; unset missions inherit the project prefix.nullmeans inherit; omission means no change).0026, the next free version on currentmain.Review history
All actionable review threads on #1930 were resolved. Greptile's final review reported 5/5 confidence and no blocking issues. The branch has now been refreshed onto current
main; the previous migration0008was moved to0026because upstream assigned0008to session-advisor state.Validation
html2canvasruntime import.Supersedes #1930.
Summary by CodeRabbit
New Features
taskPrefix, including clear-to-inherit behavior and automatic uppercase normalization.Bug Fixes
Tests & Migration