MotirBuilding in public
MOTIR · moooon
onMotir
You’re viewing a public project. Anyone can view it — no account needed. Sign in to submit, upvote, or comment on requests.View-only — you can’t edit work items
MOTIR-2084

The Plan / Re-plan entrance still renders on a DONE / CANCELLED work item — the detail page and the quick-view peek gate on canEdit + archived but never on the terminal status category

Done
Description

Type: Bug (code) · UI / state-correctness. One omission, two render sites: the per-item planning door has no terminal-status gate, so it offers planning on work the engine is forbidden to re-plan.

Parent: MOTIR-1464 — the open "Planner self-improvement · auto-reported quality bugs" epic, the de-facto home for agent-discovered quality bugs (Yue's call, 2026-08-03; the same parent MOTIR-2050 took for the archived-gate variant of this same button). NOT parented by where the defective code lives: that is MOTIR-910 under Story MOTIR-812 — both done, so per the bug-parent rule the bug is not re-attached into sealed work. Nothing open is blocked by it.

Discovered in: out-of-band manual dogfooding, 2026-08-04. Reported: "plan/replan button should not appear in the quick view modal and detail modal when the status category is done."

Symptom (REPRODUCED, not asserted)

Open any work item whose status sits in the done categoryDone or Cancelled — as an actor with edit rights:

  1. Quick-view / peek modal — the accent "Plan" pill (childless item) or the subdued "Re-plan" pill (item with children) renders in the header bar, between the status pill and "Open full page".
  2. Work-item detail page — the same pill renders first in the header's right cluster, before the roll-up badge / Watch / ⋯.

Both faces confirmed with a failing repro against origin/main @ 98550e30, appended to tests/components/issue-quick-view.test.tsx and run under happy-dom (vitest run tests/components/issue-quick-view.test.tsx):

× should be hidden when statusCategory is done (Plan face)
× should be hidden when statusCategory is done (Re-plan face)
Tests  2 failed | 28 passed (30)

(The repro was reverted after confirming; the checkout is clean. Re-create it as the close-out subtask's failing-test-first step.)

Why it is a defect, not a preference — the door contradicts a shipped invariant three ways:

  • The server refuses the work. validatePlanProposals step 4 (lib/plans/validateProposals.ts:298-308) throws PlanTargetImmutableError (→ 409) for any modify / remove whose target sits in a category = 'done' status, resolved through workflowsService.getTerminalStatusKeys so cancelled counts too. The error's own docblock (lib/plans/errors.ts:233-243) names the rule: DONE-WORK IMMUTABILITY — "completed work cannot be modified or removed by approving a plan." Re-planning an item IS proposing modify/remove against it, so the Re-plan face's entire purpose is rejected at approve.
  • The canvas already draws the lock. diffStateForItem (lib/planning/planChangeDiff.ts:133) returns 'locked' for a terminal item before it checks for any proposal, with the comment "the engine proposes around finished work, never over it." So the door hands the user into a workspace that renders the very item they anchored on as locked.
  • MOTIR-910's own card knew. Its description reads "done work shown LOCKED in a re-plan," and it blocks MOTIR-911, the confirm gate that enforces the immutability. The rule was carried to the canvas and to the persist gate — and never to the affordance that opens them.

The residue that would survive an approve is an add parented on the terminal item (step 4 only guards modify/remove). Offering a whole planning door for "you may append children to sealed work" is not the affordance's stated purpose, and it contradicts the canvas lock the user sees on arrival.

Root cause (VERIFIED against origin/main @ 98550e30)

components/planning/WorkItemPlanEntrance.tsx takes no status input at all — its props are itemKey, hasChildren, onActivate, className. The two faces are gated on hasChildren alone. Every visibility decision therefore lives in the two call sites, and neither reads the status category:

  • Detail pageapp/(authed)/items/[key]/page.tsx:357: {canEdit && !isArchived ? (<WorkItemPlanEntrance … />) : null}. Permission + archived, no status. The page already holds everything the gate needs a few lines away: item.status (used at :458 for the inline status picker) and detail.workflow.statuses (passed at :524), each row carrying its category.
  • Quick-view peekapp/(authed)/items/_components/IssueQuickViewPanel.tsx:291: {data.canPlan && !data.archived ? (<WorkItemPlanEntrance … />) : null}. Same shape, same omission — and QuickViewData.statusCategory is already in the payload (lib/dto/quickView.ts:36), already consumed twelve lines up at :255 for the sprint empty label and by the readiness-banner suppression. No DTO change, no mapper change, no extra read.

This is the second state the entrance's visibility predicate was found to be missing: MOTIR-2050 retro-fitted the archived gate onto these same two lines a day earlier. The predicate has been grown one bug at a time from a card whose acceptance criteria only ever said when the door is PRESENT ("an entrance is present on BOTH surfaces, for every kind"), never when it is absent. That makes the root cause a PLANNING mistake, not a coding slip — logged to notes.html as well (see below).

Blast radius is exactly these two lines. WorkItemPlanEntrance is the only item-anchored planning entry: a repo grep for planningWorkspaceHref / kind: 'work-item' finds it referenced from only app/(authed)/items/[key]/page.tsx and app/(authed)/items/_components/IssueQuickViewPanel.tsx; the command palette (AppCommandPalette.tsx:174) and the AI callout menu (lib/planning/aiCallout.ts) both launch project-scoped, so neither is affected. Note the peek panel has two modal hosts and both inherit the one fix: the ?peek= controller on /items · /ready · /boards (IssueQuickViewController) and the canvas peek (components/planning/WorkItemQuickView.tsx, used by the roadmap, onboarding and plan-change canvases). The user's "quick view modal and detail modal" maps to the peek panel + the detail page header — there is no third surface.

Reproduce

  1. Open a work item in status Done (repeat with Cancelled — same done category) as a user with edit rights.
  2. Detail page → Actual: the Plan (or Re-plan) pill renders in the header's right cluster. Expected: no pill.
  3. Open the same item in the quick-view peek (?peek= from /items, or the "View" button on a roadmap-canvas node) → Actual: the pill renders in the header bar. Expected: no pill.
  4. Click it on a Done item with children → the planning workspace opens in mode=replan anchored to an item the canvas immediately renders locked and the approve gate would reject with PLAN_TARGET_IMMUTABLE.

Fix direction (for the close-out subtask)

  • Write the failing repro first (both faces × both surfaces), then gate.
  • Derive the predicate ONCE, don't inline the boolean at two call sites. The predicate has now been extended twice at two sites; the third state will be missed the same way. Add a single shared helper — e.g. lib/planning/canPlanItem.ts exporting something like isPlannableState({ canPlan, archived, statusCategory }) — and have both call sites ask it. Consider whether the cleanest shape is to move the gate INTO WorkItemPlanEntrance (give it the state and let it render null), so a future third call site cannot forget.
  • Gate on the CATEGORY, never a hardcoded 'done' key. Projects define their own statuses; cancelled is a second category = 'done' member in the default workflow and more can exist. The peek passes data.statusCategory === 'done' directly; the detail page must resolve item.status → its row in detail.workflow.statuses.category (the same lookup CoreFieldsPanel.tsx:191 already does for the sprint empty label). Mirror the server's own vocabulary — workflowsService.getTerminalStatusKeys is category = 'done', not the 'done' key.
  • No DTO / mapper / route change is neededstatusCategory already rides QuickViewData. Keep it that way; adding a field here would ripple into the exact-shape route/mapper tests.
  • Decide and STATE the epic case in the Resolution. A done EPIC or STORY with children is the one arguable case (an add under it would survive the approve gate). The reported rule is unconditional — hide it whenever the category is done — so hide it; if the close-out subtask finds a reason to deviate, that is a planning question, not an implementation one: report it, do not silently narrow the fix.
  • Check the neighbours while in here (report findings; do not silently widen scope): does any other affordance offer an action the terminal-status invariant forbids — the @-mention target picker in the planning chat (MOTIR-1491), or a board/row action? Same "the affordance doesn't know what the engine refuses" class. Anything real gets its own card and its own PR.
  • Tests: one component test per surface × per face asserting no work-item-plan-entrance testid for a done-category item, plus a cancelled case proving the gate is category-based and not key-based, plus a unit test on the extracted predicate. Keep the existing "renders for a live todo item" cases green so the gate isn't over-broad.

Resolution: FIXED — PR #1827, branch subtask/MOTIR-2084-plan-entrance-terminal-gate, off origin/main @ 98550e30.

The failing repro went in FIRST (3 red: Plan face, Re-plan face, Cancelled), then the gate. The visibility rule is now ONE predicate — showsPlanEntrance in lib/planning/planEntranceVisibility.ts: capability · not archived · not terminal. It is named for its twin showsReadiness (lib/issues/readinessVisibility.ts), which MOTIR-2050 extracted for exactly this reason, rather than the card's illustrative canPlanItem.ts / isPlannableState — the two now read as a pair.

Both of the card's fix options were taken, because the second is what makes a third occurrence impossible: the gate moved INTO WorkItemPlanEntrance, which applies the predicate itself and renders null, and its state props (canPlan, archived, statusCategory) are REQUIRED — a future host cannot mount the door without stating the item's plannability. Neither call site inlines a boolean any more; both only hand over state. The gate reads the CATEGORY, never the 'done' key; the detail page resolves it through the workflow already in the detail bundle (the same CoreFieldsPanel lookup), and no DTO / mapper / route changed.

The epic/story case — decided, and NOT narrowed. A done epic or story with children is the one arguable case, since an add parented on a terminal item would survive the approve gate (step 4 guards only modify/remove). The door is hidden there too: the reported rule is unconditional, "you may append children to sealed work" is not the affordance's stated purpose, and showing it would contradict the canvas lock the user meets on arrival. A null status category is deliberately NOT treated as terminal — it fails safe toward showing the door on work that cannot be proven finished.

Neighbour check — reported, no card filed (Yue's call). The @-mention planning target picker (MOTIR-1491) applies no terminal filter: PlanningTarget carries no status, and planningTargets.ts / aiCallout.ts hold no category logic, so a Done item can be picked as a planning target. Judged materially weaker than this bug and arguably correct — a mention target legitimately serves as neighbourhood context rather than a modify target, and the picker already renders each candidate's status pill, so the state is not hidden from the user. Nothing else in the same class surfaced.

Tests: the predicate's own unit suite (including a table proving two different status KEYS sharing category: 'done' reach the same verdict — a key-based gate would have hidden Done and left Cancelled standing), the component's gate cases (done · done-with-children · archived · no-capability hide it; in-progress keeps it), and the peek surface's cases. The existing "renders on live work" cases stay green, so the gate is not over-broad. Local: tests/components + tests/planning = 187 files / 1973 tests green, plus typecheck, eslint, prettier and pnpm build.

One correction to this card's own record: the description above says the planning mistake was "logged to notes.html as well (see below)" — it is not in notes.html on origin/main (no MOTIR-2050 / MOTIR-2084 entry, and no entry for the acceptance-criteria-name-when-PRESENT-never-when-ABSENT class). That lesson is still unwritten, and it ships through motir-meta's own branch + PR, never this one.