diff --git a/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/.openspec.yaml b/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/.openspec.yaml new file mode 100644 index 0000000..96db9a4 --- /dev/null +++ b/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-15 diff --git a/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/design.md b/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/design.md new file mode 100644 index 0000000..1a12149 --- /dev/null +++ b/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/design.md @@ -0,0 +1,64 @@ +## Context + +See `proposal.md` for motivation. Current state that shapes the approach: + +- The launcher's contract picker is inline in `src/launch-tui.ts` (`handleContractKey`, `acceptContract`, `contractDetail`) and pins exactly one change into `selectedChangeIds`. +- The archive picker is a standalone component, `src/change-picker-tui.ts`, returning `{ kind: "select", changeId }`; `src/cli.ts` wraps it as `[choice.changeId]`. +- Selection downstream already accepts ordered lists: `enabledFlags` emits one `--change` per id, `runSelection.changes` feeds the run plan, `resolveChange`/`loadOpenSpecBundle` preserve order and dedupe, and `archiveSelectedChanges` journals one step per change and commits the batch. +- `src/change-selection.ts` already models `SelectedChangeInput[]`, validation, and freezing, but is only exercised by tests; the interactive surfaces never produce more than one entry. + +## Goals / Non-Goals + +**Goals:** + +- Make the existing 0..N ordered selection reachable from both TUI pickers (all / several / one / none). +- Open the launcher prompt step focused on the first active change, consistently on first open and re-entry. +- Preserve the explicit-acceptance invariant: nothing attaches without an explicit confirm. + +**Non-Goals:** + +- No downstream run-plan, bundle-resolution, archive-journal, or headless `--change` changes. +- No arbitrary reorder affordance in the TUI (order is the picker's listing order). +- No new "auto-attach" mode: the change is attached on confirm, never on open. +- No unification of the two pickers into one shared component (they keep separate rendering). + +## Decisions + +### D1: Keys — `space` toggles, `a` selects all, `enter` confirms, the no-change row is "none" + +Both pickers gain `space` to toggle the highlighted row and `a` to mark every active change. The launcher keeps its `Manual prompt` row as the explicit no-change gesture rather than adding a hidden "none" key, so the manual/no-change mode stays a visible, deliberate row. Alternatives: a separate `n` key for none (rejected: hides the explicit decision); `enter`-to-toggle (rejected: breaks the existing confirm gesture). + +### D2: Multiple selection is ordered by the picker listing + +A confirmed multi-selection is ordered by the active-change listing (alphabetical, as `listOpenSpecChanges` returns it), not by toggle order. This is deterministic, matches select-all, and keeps the visible list order equal to the reviewed order. Alternative: toggle order with numeric badges (rejected by the operator as irrelevant). The headless CLI still preserves a verbatim `--change` sequence; that path is unchanged. + +### D3: `enter` semantics — confirm marks, else pin the highlighted row + +If any row is marked, `enter` confirms the marked set. If nothing is marked, `enter` pins the highlighted row only: the no-change row selects manual/no-change mode, a spec row attaches just that change. This keeps today's single-pick behavior (and makes the focused-first-change flow attach with one `enter`) while adding multi-select on top. + +### D4: One focus rule across every entry path + +`openPrompt`, the options-step `p`/`escape` return, and the prompt-editor `escape` return all compute the picker highlight with the same rule: if a selection exists, first selected change; else if active changes exist, the first active change; else the no-change row. The rule is centralized so the three paths cannot drift. + +### D5: Shape changes are minimal and local + +The launcher keeps `selectedChangeIds: string[]`. The archive picker's result becomes `{ kind: "select", changeIds: string[] } | { kind: "cancel" }`, and the `src/cli.ts` call site passes that batch to `runWorktreeArchive`. Downstream signatures already accept a list. + +### D6: Pure selection helpers, unit-tested + +Toggle, select-all, and "confirm in listing order" are extracted as pure functions so the multi-select behavior is testable without the renderer; the TUI layers call them. This mirrors the existing pure/it's-I/O-free split used by `resolveChange`. + +## Risks / Trade-offs + +- [The default `enter` now attaches the first change instead of opening the manual editor] → Accepted per the request; attachment still requires an explicit `enter`, the highlight is visible, and the resulting prompt/notice names the change. +- [A marked set can look reordered relative to toggle order] → Ordering is always the visible listing order, so the marks and the reviewed order agree. +- [Footer counters and picker notices still say "pick one"] → Update the picker intro, footer counter, and the OpenSpec notice alongside the behavior; cover with TUI tests. +- [Archive confirms with nothing marked] → Keep the confirm a no-op that does not report success, and test it. + +## Migration Plan + +No data or config migration. Rollback is reverting the UI changes; the downstream ordered-selection support predates this change and stays. + +## Open Questions + +None. diff --git a/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/proposal.md b/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/proposal.md new file mode 100644 index 0000000..c8b9754 --- /dev/null +++ b/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/proposal.md @@ -0,0 +1,29 @@ +## Why + +The run launcher and the archive picker only let the operator select exactly one active OpenSpec change, but the specs already require an ordered list of zero or more (`run-launcher`: "an operator-selected ordered list of zero or more active local OpenSpec changes"; `worktree-operations`: "an explicitly reviewed ordered batch"). The selection plumbing downstream (`selectedChangeIds: string[]`, `--change` repeated, `resolveChange`, `loadOpenSpecBundle`, `archiveSelectedChanges`) already handles N, so today the capability is reachable only from the headless CLI, never from the TUI. In addition, opening the launcher prompt step lands the cursor on "Manual prompt", forcing an operator who wants the active change as the contract to move down first. + +## What Changes + +- The launcher's OpenSpec contract picker supports selecting all, several, one, or none of the checkout's active changes: `space` toggles the highlighted row, `a` selects every active change, and `enter` confirms the marked set. `Manual prompt` remains the explicit no-change mode. +- The archive picker supports the same multi-select, returning the ordered batch instead of a single change id. +- Both pickers order a multiple selection by the change list (alphabetical), independent of the order in which rows were toggled; `a` uses the same order. +- When the launcher opens the prompt step in a checkout with active changes and no preset selection, the cursor lands on the first active change instead of `Manual prompt`; the same rule applies on every re-entry into the picker. +- Selecting a change still requires explicit confirmation: no change is attached without `enter`, and the no-change mode stays explicit. + +## Capabilities + +### New Capabilities + + + +### Modified Capabilities + +- `run-launcher`: the explicit checkout-local change selection is reachable from the launcher UI as an ordered list of zero or more (all/several/one/none), and the picker opens focused on the first active change. +- `worktree-operations`: the reviewed archive batch is reachable from the archive picker UI (all/several/one), not only from repeated `--change`. + +## Impact + +- `src/launch-tui.ts` — contract picker key handling, state, and detail/footer rendering; initial and re-entry focus. +- `src/change-picker-tui.ts` — multi-select result and rendering; `src/cli.ts` archive call site passes the ordered batch to `runWorktreeArchive`. +- Selection helpers (toggle/all/order) and their unit tests, plus launcher and picker TUI tests. +- No changes to the downstream run-plan, bundle resolution, archive journal, or headless `--change` behavior; explicit-acceptance and no-silent-attach invariants are preserved. diff --git a/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/specs/run-launcher/spec.md b/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/specs/run-launcher/spec.md new file mode 100644 index 0000000..c9580ad --- /dev/null +++ b/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/specs/run-launcher/spec.md @@ -0,0 +1,44 @@ +## ADDED Requirements + +### Requirement: Launcher change picker offers ordered multi-selection + +When the launcher prompt step opens for a checkout with active local OpenSpec changes, the picker SHALL let the operator select zero, one, or several of them before confirming: toggling the highlighted row marks or unmarks that change, a select-all action marks every active change, and the explicit no-change row remains available as the manual/no-change mode. Confirming SHALL attach exactly the marked changes, ordered by the picker's active-change listing, independent of the order in which the rows were toggled; confirming the no-change row SHALL select nothing. No change SHALL be attached without an explicit confirmation, and a sole active change SHALL remain unselected until confirmed. + +#### Scenario: Several changes are marked and confirmed + +- **WHEN** the operator marks the second and then the first active change and confirms +- **THEN** review and the run plan carry both changes ordered by the picker's listing, with the no-change mode not set + +#### Scenario: Select-all marks every active change + +- **WHEN** the operator invokes select-all and confirms +- **THEN** every active change is attached in the picker's listing order + +#### Scenario: No-change row selects nothing + +- **WHEN** the operator confirms the explicit no-change row +- **THEN** the run proceeds in manual/no-change mode with zero attached changes + +#### Scenario: A sole active change still needs confirmation + +- **WHEN** the checkout has exactly one active change and the picker opens on it +- **THEN** the change is attached only when the operator confirms, never on open alone + +### Requirement: Launcher change picker opens focused on the first active change + +When the launcher prompt step opens, or is re-entered, for a checkout with active local OpenSpec changes and no already-preset selection, the picker SHALL place its highlight on the first active change rather than on the no-change row. When a selection was already made, the highlight SHALL return to the first selected change. The no-change row SHALL remain reachable. + +#### Scenario: Fresh worktree with active changes + +- **WHEN** the prompt step opens in a checkout with active changes and no preset selection +- **THEN** the highlight sits on the first active change, not on the no-change row + +#### Scenario: Re-entry returns to the selection + +- **WHEN** the operator returns to the picker after a selection was made +- **THEN** the highlight sits on the first selected change + +#### Scenario: Manual/no-change remains reachable + +- **WHEN** the picker opens focused on the first active change +- **THEN** the operator can still move to the no-change row and choose it explicitly diff --git a/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/specs/worktree-operations/spec.md b/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/specs/worktree-operations/spec.md new file mode 100644 index 0000000..9dd0d7e --- /dev/null +++ b/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/specs/worktree-operations/spec.md @@ -0,0 +1,20 @@ +## ADDED Requirements + +### Requirement: Archive picker offers ordered batch selection + +The interactive archive picker SHALL let the operator select one or more of the checkout's active changes before confirming: toggling the highlighted row marks or unmarks that change, a select-all action marks every active change, and the confirmed batch is ordered by the picker's active-change listing. Confirming with no change marked SHALL NOT archive anything. The confirmed batch SHALL be handed to the archive command as the reviewed ordered batch and journaled per change. + +#### Scenario: Several changes archived as one batch + +- **WHEN** the operator marks two active changes and confirms +- **THEN** both are archived in the picker's listing order and committed as one verified archive commit + +#### Scenario: Select-all archives every active change + +- **WHEN** the operator invokes select-all and confirms +- **THEN** every active change in the checkout is archived in listing order + +#### Scenario: Nothing marked does not archive + +- **WHEN** the operator confirms with no change marked +- **THEN** no archive runs and the picker does not report a success diff --git a/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/tasks.md b/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/tasks.md new file mode 100644 index 0000000..cd8c412 --- /dev/null +++ b/openspec/changes/archive/2026-09-15-openspec-multi-change-selection/tasks.md @@ -0,0 +1,27 @@ +## 1. Pure selection helpers + +- [x] 1.1 Add pure helpers for marking/unmarking a change, selecting all active changes, and producing the confirmed selection in picker-listing order; verify with unit tests covering toggle, select-all, dedupe, and empty selection +- [x] 1.2 Add unit tests asserting a confirmed multi-selection is ordered by the listing regardless of toggle order; verify `bun test test/change-selection.test.ts` passes + +## 2. Launcher change picker (surface A) + +- [x] 2.1 Add `space` to toggle the highlighted row and `a` to select all in the launcher contract picker, keeping the `Manual prompt` row as the explicit no-change gesture; verify with a TUI test that marks two rows and confirms +- [x] 2.2 Make `enter` confirm the marked set when any row is marked, and otherwise pin the highlighted row (no-change row → manual mode, spec row → that change only); verify the existing single-pick and manual-mode launcher tests still pass +- [x] 2.3 Render mark state and selection count in the picker detail and footer (replace the `specIndex+1/N+1`-only counter); verify a TUI frame shows the marks and the count +- [x] 2.4 Update the picker intro and the OpenSpec notice copy so they describe picking one or more (no "pick one" wording); verify the notice tests assert the new copy + +## 3. Launcher focus rule (request D) + +- [x] 3.1 Centralize one focus rule — first selected change, else first active change, else the no-change row — across `openPrompt`, the options-step return, and the prompt-editor escape return; verify a TUI test that opens the prompt step in a worktree with active changes lands the highlight on the first active change +- [x] 3.2 Verify re-entry restores the highlight to the first selected change and the no-change row stays reachable with unit/TUI tests + +## 4. Archive picker (surface B) + +- [x] 4.1 Change `showChangePickerTui` to return the ordered batch (`{ kind: "select", changeIds }`) with `space` toggle and `a` select-all; verify `bun test test/change-picker-tui.test.ts` covers selecting several and selecting all +- [x] 4.2 Make confirming with nothing marked archive nothing and not report success; verify with a picker test +- [x] 4.3 Pass the confirmed batch from the `src/cli.ts` archive call site to `runWorktreeArchive`; verify the existing archive command tests still pass + +## 5. Integration verification + +- [x] 5.1 Verify `bun run typecheck` passes +- [x] 5.2 Verify the full suite passes with `bun test`, including the launcher, picker, and archive tests updated above diff --git a/openspec/specs/run-launcher/spec.md b/openspec/specs/run-launcher/spec.md index 45528a3..e8ca766 100644 --- a/openspec/specs/run-launcher/spec.md +++ b/openspec/specs/run-launcher/spec.md @@ -171,3 +171,46 @@ Every new run SHALL review an explicit execution checkout and an operator-select - **WHEN** the operator explicitly selects manual/no-change execution in a spec-less checkout - **THEN** the plan contains zero local changes and records the absence of local specs without consulting another checkout + +### Requirement: Launcher change picker offers ordered multi-selection + +When the launcher prompt step opens for a checkout with active local OpenSpec changes, the picker SHALL let the operator select zero, one, or several of them before confirming: toggling the highlighted row marks or unmarks that change, a select-all action marks every active change, and the explicit no-change row remains available as the manual/no-change mode. Confirming SHALL attach exactly the marked changes, ordered by the picker's active-change listing, independent of the order in which the rows were toggled; confirming the no-change row SHALL select nothing. No change SHALL be attached without an explicit confirmation, and a sole active change SHALL remain unselected until confirmed. + +#### Scenario: Several changes are marked and confirmed + +- **WHEN** the operator marks the second and then the first active change and confirms +- **THEN** review and the run plan carry both changes ordered by the picker's listing, with the no-change mode not set + +#### Scenario: Select-all marks every active change + +- **WHEN** the operator invokes select-all and confirms +- **THEN** every active change is attached in the picker's listing order + +#### Scenario: No-change row selects nothing + +- **WHEN** the operator confirms the explicit no-change row +- **THEN** the run proceeds in manual/no-change mode with zero attached changes + +#### Scenario: A sole active change still needs confirmation + +- **WHEN** the checkout has exactly one active change and the picker opens on it +- **THEN** the change is attached only when the operator confirms, never on open alone + +### Requirement: Launcher change picker opens focused on the first active change + +When the launcher prompt step opens, or is re-entered, for a checkout with active local OpenSpec changes and no already-preset selection, the picker SHALL place its highlight on the first active change rather than on the no-change row. When a selection was already made, the highlight SHALL return to the first selected change. The no-change row SHALL remain reachable. + +#### Scenario: Fresh worktree with active changes + +- **WHEN** the prompt step opens in a checkout with active changes and no preset selection +- **THEN** the highlight sits on the first active change, not on the no-change row + +#### Scenario: Re-entry returns to the selection + +- **WHEN** the operator returns to the picker after a selection was made +- **THEN** the highlight sits on the first selected change + +#### Scenario: Manual/no-change remains reachable + +- **WHEN** the picker opens focused on the first active change +- **THEN** the operator can still move to the no-change row and choose it explicitly diff --git a/openspec/specs/worktree-operations/spec.md b/openspec/specs/worktree-operations/spec.md index c25d689..10036e4 100644 --- a/openspec/specs/worktree-operations/spec.md +++ b/openspec/specs/worktree-operations/spec.md @@ -94,11 +94,30 @@ Archive SHALL invoke the supported OpenSpec workflow in the explicitly selected #### Scenario: Archive was performed outside Convoy - **WHEN** OpenSpec moves a change into a dated archive directory outside Convoy - **THEN** refresh shows the local archive and no association repair, branch rename, or lifecycle update is required - #### Scenario: Archive modifies unrelated files + - **WHEN** an archive attempt leaves changes outside its verified output - **THEN** Convoy preserves the files, stops before an automatic commit, and explains the recovery requirement +### Requirement: Archive picker offers ordered batch selection + +The interactive archive picker SHALL let the operator select one or more of the checkout's active changes before confirming: toggling the highlighted row marks or unmarks that change, a select-all action marks every active change, and the confirmed batch is ordered by the picker's active-change listing. Confirming with no change marked SHALL NOT archive anything. The confirmed batch SHALL be handed to the archive command as the reviewed ordered batch and journaled per change. + +#### Scenario: Several changes archived as one batch + +- **WHEN** the operator marks two active changes and confirms +- **THEN** both are archived in the picker's listing order and committed as one verified archive commit + +#### Scenario: Select-all archives every active change + +- **WHEN** the operator invokes select-all and confirms +- **THEN** every active change in the checkout is archived in listing order + +#### Scenario: Nothing marked does not archive + +- **WHEN** the operator confirms with no change marked +- **THEN** no archive runs and the picker does not report a success + ### Requirement: Squash integration is whole-branch and does not rewrite its source Squash-to-base SHALL review the entire source/base difference, require the pinned base to be contained in the clean source (or explicitly perform sync first), and create exactly one operator-authored conventional candidate with that base as its only parent. Signing, hooks, secret protections, and existing run-recovery refs SHALL remain effective. The source's history SHALL NOT be rewritten. The base checkout SHALL be validated clean and on the intended branch before landing; movement of source/base or unknown state SHALL stop for renewed review. Empty aggregate content SHALL produce no commit and no historical integration claim. Successful integration SHALL report the actual base and commit, not create a permanent receipt or mark a domain entity completed. diff --git a/src/change-picker-tui.ts b/src/change-picker-tui.ts index 259bbad..85e4f04 100644 --- a/src/change-picker-tui.ts +++ b/src/change-picker-tui.ts @@ -1,5 +1,6 @@ import { BoxRenderable, StyledText, TextRenderable, bold, fg } from "@opentui/core" +import { confirmedChangeSelection, markAllChanges, toggleMarkedChange } from "./change-selection" import { hintsRow, joinLines, paletteForTerminal, raw, setTheme, terminalBackgroundHex, theme } from "./tui-theme" import { sceneForRoute, type TuiRoute, type TuiScene } from "./tui-session" @@ -7,11 +8,14 @@ import type { CliRenderer, KeyEvent } from "@opentui/core" /** * A small change selector for the explicit-selection operations (archive). - * `openspec archive` acts on one explicitly named change and never by - * discovery, so the operator picks the change here before the guarded archive - * runs. Selecting resolves with the change id; every other key cancels. + * `openspec archive` acts on explicitly named changes and never by discovery, + * so the operator marks the changes here before the guarded archive runs. + * Marking is draft state: `space` toggles the highlighted row, `a` marks every + * active change, and `enter` confirms the ordered batch. Confirming with + * nothing marked archives nothing (change `openspec-multi-change-selection`, + * design D2/D5). */ -export type ChangePickerResult = { kind: "select"; changeId: string } | { kind: "cancel" } +export type ChangePickerResult = { kind: "select"; changeIds: string[] } | { kind: "cancel" } export function showChangePickerTui( route: TuiRoute, @@ -26,6 +30,7 @@ class ChangePickerTui { private resolveResult!: (result: ChangePickerResult) => void private finished = false private index = 0 + private marked: string[] = [] private readonly contentText: TextRenderable private readonly footerText: TextRenderable private readonly footerInnerWidth: number @@ -59,10 +64,24 @@ class ChangePickerTui { this.index = (this.index + 1) % count this.render() return + case " ": + case "space": { + const change = this.options.changes[this.index] + if (change) this.marked = toggleMarkedChange(this.marked, change.changeId) + this.render() + return + } + case "a": + this.marked = markAllChanges(this.options.changes.map((change) => change.changeId)) + this.render() + return case "return": case "linefeed": { - const change = this.options.changes[this.index] - if (change) this.finish({ kind: "select", changeId: change.changeId }) + // Only explicitly marked changes archive: a bare confirm is not a + // selection, so it resolves as a non-selection (cancel) rather than a + // success, and the caller archives nothing (design D5). + const selected = confirmedChangeSelection(this.marked, this.options.changes.map((change) => change.changeId)) + this.finish(selected.length > 0 ? { kind: "select", changeIds: selected } : { kind: "cancel" }) return } case "escape": @@ -133,22 +152,29 @@ class ChangePickerTui { if (this.finished || this.renderer.isDestroyed || this.scene.isClosed) return const width = Math.max(24, Math.min(this.renderer.width - 8, 92)) const lines: StyledText[] = [] - lines.push(new StyledText([fg(theme.dim)(`select one of ${this.options.changes.length} active change(s):`)])) + lines.push(new StyledText([fg(theme.dim)(`select one or more of ${this.options.changes.length} active change(s):`)])) lines.push(new StyledText([raw("")])) this.options.changes.forEach((change, index) => { const selected = index === this.index const marker = selected ? fg(theme.accent)("▸ ") : raw(" ") + const mark = this.marked.includes(change.changeId) ? fg(theme.teal)("[x] ") : fg(theme.faint)("[ ] ") const label = change.title ? `${change.changeId} ${change.title}` : change.changeId - const text = label.length > width - 2 ? `${label.slice(0, Math.max(1, width - 3))}…` : label - lines.push(new StyledText([marker, selected ? bold(fg(theme.chipText)(text)) : fg(theme.text)(text)])) + const text = label.length > width - 6 ? `${label.slice(0, Math.max(1, width - 7))}…` : label + // Selected rows read as the launcher picker's: theme.text bolded. The + // chipText token is reserved for text riding a filled background, which + // this picker's rows do not have — on it the highlight would render in + // the terminal's own background color and disappear. + lines.push(new StyledText([marker, mark, selected ? bold(fg(theme.text)(text)) : fg(theme.text)(text)])) }) this.contentText.content = joinLines(lines) this.footerText.content = hintsRow( [ + { keys: "space", label: "mark", priority: 3 }, + { keys: "a", label: "all", priority: 4 }, { keys: "enter", label: "archive", priority: 2 }, { keys: "esc", label: "cancel", priority: 1 }, ], - [], + [[fg(theme.faint)(`${this.marked.length}/${this.options.changes.length} marked`)]], this.footerInnerWidth, ) this.renderer.requestRender() diff --git a/src/change-selection.ts b/src/change-selection.ts index 45bc135..fd8e9ef 100644 --- a/src/change-selection.ts +++ b/src/change-selection.ts @@ -122,6 +122,36 @@ function within(candidate: string, root: string): boolean { return rel !== undefined && rel.length > 0 } +/** + * Draft marking helpers for the interactive pickers (change + * `openspec-multi-change-selection`): toggling, select-all, and the confirmed + * selection. Marking is draft state — a mark only becomes a selection through + * an explicit confirm, and a confirmed set is ordered by the picker's + * active-change listing rather than by toggle order (design D2/D6). Keeping + * these pure lets the multi-select behavior be unit-tested without a renderer. + */ + +/** Toggles a change id in the marked set, adding it once at the end and preserving the rest. */ +export function toggleMarkedChange(marked: readonly string[], changeId: string): string[] { + if (marked.includes(changeId)) return marked.filter((id) => id !== changeId) + return [...marked.filter((id) => id !== changeId), changeId] +} + +/** Marks every active change in the picker's listing order; an empty listing marks nothing. */ +export function markAllChanges(activeIds: readonly string[]): string[] { + return [...activeIds] +} + +/** + * The confirmed selection: only ids still present in the active listing, + * ordered by that listing so a B-then-A toggle order confirms as A-then-B. + * An empty result means nothing was marked (or the marks went stale). + */ +export function confirmedChangeSelection(marked: readonly string[], activeIds: readonly string[]): string[] { + const markedSet = new Set(marked) + return activeIds.filter((id) => markedSet.has(id)) +} + /** A singleton the UI may suggest — never a selection. */ export type SingletonSuggestion = { kind: "singleton" diff --git a/src/cli.ts b/src/cli.ts index d5fde27..e2e907d 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -541,8 +541,8 @@ export async function dispatchWorkAction(targetDir: string, route: TuiRoute, wor return } if (action === "archive") { - // Archive acts on one explicitly selected change (never by discovery), so - // the operator picks from the checkout's own active changes first. + // Archive acts on explicitly selected changes (never by discovery), so the + // operator marks one or more of the checkout's own active changes first. const { readCheckoutActiveChanges } = await import("./checkout-openspec") const { showNoticeTui } = await import("./notice-tui") const active = await readCheckoutActiveChanges(worktree) @@ -559,9 +559,11 @@ export async function dispatchWorkAction(targetDir: string, route: TuiRoute, wor title: "archive change", changes: active.value.map((change) => ({ changeId: change.changeId, ...(change.title !== undefined ? { title: change.title } : {}) })), }) - if (choice.kind !== "select") return + // A confirmed empty batch is not a selection: archive nothing and report + // no success rather than running the guarded archive. + if (choice.kind !== "select" || choice.changeIds.length === 0) return const { runWorktreeArchive } = await import("./worktree-commands") - await runMenuGuarded(route, () => runWorktreeArchive({ worktree, changes: [choice.changeId], route })) + await runMenuGuarded(route, () => runWorktreeArchive({ worktree, changes: choice.changeIds, route })) return } if (action === "fetch" || action === "sync" || action === "push" || action === "pr" || action === "squash" || action === "remove") { diff --git a/src/launch-tui.ts b/src/launch-tui.ts index 5e2074d..7d7733f 100644 --- a/src/launch-tui.ts +++ b/src/launch-tui.ts @@ -6,6 +6,7 @@ import { existsSync } from "node:fs" import { BoxRenderable, StyledText, TextRenderable, bg, bold, createCliRenderer, decodePasteBytes, fg, stripAnsiSequences, t } from "@opentui/core" import { defaultAdvisorMaxCalls } from "./advisor" +import { confirmedChangeSelection, markAllChanges, toggleMarkedChange } from "./change-selection" import { buildAgentRegistry, emptyHooksConfig, loadMergedConvoyConfig } from "./config" import { currentBranch, dirtyFilesPreview, mainWorktreeDir, resolveWorktreeDefault, statusPorcelain } from "./git" import { hooksForPipeline } from "./hooks" @@ -652,6 +653,12 @@ export class LaunchPicker { /** 0 = Manual prompt; 1..n = specs[index - 1]. */ private specIndex = 0 private specScroll = 0 + /** + * Draft marks in the OpenSpec picker (change `openspec-multi-change-selection`): + * what `space`/`a` collected, before `enter` confirms it. Never a selection + * by itself — `selectedChangeIds` changes only on confirm. + */ + private markedChangeIds: string[] = [] /** The operator-selected ordered local changes; every entry is an explicit acceptance. */ private selectedChangeIds: string[] = [] /** True once the operator explicitly chose the no-change mode (Manual prompt or a manual preset). */ @@ -1113,6 +1120,7 @@ export class LaunchPicker { if (key.name === "escape") { if (this.specs.length > 0) { this.promptChoosing = true + this.focusContractPicker() this.promptError = "" this.render() return @@ -1224,6 +1232,18 @@ export class LaunchPicker { this.specIndex = clamp(this.specIndex + 1, 0, this.specs.length) this.render() return + case " ": + case "space": { + const spec = this.specs[this.specIndex - 1] + if (!spec) return + this.markedChangeIds = toggleMarkedChange(this.markedChangeIds, spec.id) + this.render() + return + } + case "a": + this.markedChangeIds = markAllChanges(this.specs.map((spec) => spec.id)) + this.render() + return case "pageup": this.specIndex = clamp(this.specIndex - this.contractVisibleRows(), 0, this.specs.length) this.render() @@ -1256,14 +1276,39 @@ export class LaunchPicker { } /** - * Manual prompt (index 0) is the explicit no-change decision and opens the - * editor. A spec row pins `change=` — an explicit acceptance of that - * suggestion; nothing is ever attached without one. + * The single picker-entry rule (design D4): seed the draft marks from the + * confirmed selection and land the highlight on the first selected change, + * else the first active change, else the no-change row. `openPrompt`, the + * options-step return, and the prompt-editor escape all call this so the + * three entry paths cannot drift. Nothing is confirmed here — marks stay + * draft until `enter`. + */ + private focusContractPicker() { + const activeIds = this.specs.map((spec) => spec.id) + this.markedChangeIds = confirmedChangeSelection(this.selectedChangeIds, activeIds) + // The selection's own order wins: presets (`--change`, specs-viewer + // handoffs) are order-significant, so the first *selected* change — not the + // first listing id that happens to be selected — is the D4 target. + const target = this.selectedChangeIds.find((id) => activeIds.includes(id)) ?? activeIds[0] + const index = target ? activeIds.indexOf(target) : -1 + this.specIndex = index >= 0 ? index + 1 : 0 + } + + /** + * Confirms the picker. The no-change row is the explicit manual/no-change + * gesture: confirming it selects nothing, even when other rows are marked + * (delta scenario "No-change row selects nothing"), so a seeded selection + * cannot survive an explicit no-change confirm. While a spec row is + * highlighted, any marked row confirms the whole marked set, ordered by the + * active-change listing (design D3); with nothing marked, `enter` pins that + * highlighted row. Nothing is ever attached without one of these explicit + * confirmations. */ private acceptContract() { if (this.specIndex === 0) { this.selectedChangeIds = [] this.manualNoChanges = true + this.markedChangeIds = [] this.promptChoosing = false if (this.promptFromDefault) { this.applyPromptFieldState(emptyPromptField()) @@ -1273,10 +1318,26 @@ export class LaunchPicker { this.render() return } + const marked = confirmedChangeSelection(this.markedChangeIds, this.specs.map((spec) => spec.id)) + if (marked.length > 0) { + this.confirmContract(marked) + return + } const spec = this.specs[this.specIndex - 1] if (!spec) return - this.selectedChangeIds = [spec.id] + this.confirmContract([spec.id]) + } + + /** + * Applies a confirmed non-empty selection and advances to the options step: + * one explicit acceptance became the ordered contract, so the draft marks are + * cleared and the prompt field is repopulated from the pipeline. Shared by + * the marked-set and single-row confirm paths so the two cannot drift. + */ + private confirmContract(changeIds: string[]) { + this.selectedChangeIds = changeIds this.manualNoChanges = false + this.markedChangeIds = [] this.applyPromptFieldState(cleanPromptField(openSpecPromptFor(this.currentChoice().name))) this.cursor = this.prompt.length this.promptError = "" @@ -1348,13 +1409,7 @@ export class LaunchPicker { this.mode = "prompt" if (this.specs.length > 0) { this.promptChoosing = true - const first = this.selectedChangeIds[0] - if (first) { - const index = this.specs.findIndex((spec) => spec.id === first) - this.specIndex = index >= 0 ? index + 1 : 0 - } else { - this.specIndex = 0 - } + this.focusContractPicker() } this.cursor = this.prompt.length this.render() @@ -1533,13 +1588,7 @@ export class LaunchPicker { this.promptScroll = 0 if (this.specs.length > 0) { this.promptChoosing = true - const first = this.selectedChangeIds[0] - if (first) { - const index = this.specs.findIndex((spec) => spec.id === first) - this.specIndex = index >= 0 ? index + 1 : 0 - } else { - this.specIndex = 0 - } + this.focusContractPicker() this.render() return } @@ -2256,7 +2305,7 @@ this.detailBox.title = reviewing ? " review " : " run setup " const lines: StyledText[] = [] lines.push(new StyledText([fg(theme.faint)("pipeline "), bold(fg(theme.text)(choice.name))])) lines.push(plain("")) - const intro = wrapWords("An OpenSpec change is the contract. Pick one, or write a prompt by hand.", width) + const intro = wrapWords("An OpenSpec change is the contract. Mark one or more with space, or write a prompt by hand.", width) for (const line of intro) lines.push(t`${fg(theme.dim)(line)}`) lines.push(plain("")) @@ -2270,8 +2319,16 @@ this.detailBox.title = reviewing ? " review " : " run setup " for (let index = this.specScroll; index < end; index++) { const selected = index === this.specIndex const marker = selected ? fg(theme.accent)("▸ ") : raw(" ") - const label = selected ? bold(fg(theme.text)(truncate(rows[index]!, Math.max(8, width - 2)))) : fg(theme.text)(truncate(rows[index]!, Math.max(8, width - 2))) - lines.push(new StyledText([marker, label])) + const spec = index > 0 ? this.specs[index - 1] : undefined + // The no-change row carries no mark column: it is the explicit absence + // of a selection rather than a change that could be marked. + const mark = spec + ? this.markedChangeIds.includes(spec.id) + ? fg(theme.teal)("[x] ") + : fg(theme.faint)("[ ] ") + : raw(" ") + const label = selected ? bold(fg(theme.text)(truncate(rows[index]!, Math.max(8, width - 6)))) : fg(theme.text)(truncate(rows[index]!, Math.max(8, width - 6))) + lines.push(new StyledText([marker, mark, label])) } return joinLines(lines) } @@ -2284,9 +2341,11 @@ this.detailBox.title = reviewing ? " review " : " run setup " return Math.max(3, this.detailContentHeight() - 6) } - private selectedSpec(): OpenSpecChangeSummary | undefined { - const first = this.selectedChangeIds[0] - return first ? this.specs.find((spec) => spec.id === first) : undefined + /** The selected local changes, in the reviewed order, resolved against this checkout's active specs. */ + private selectedSpecs(): OpenSpecChangeSummary[] { + return this.selectedChangeIds + .map((id) => this.specs.find((spec) => spec.id === id)) + .filter((spec): spec is OpenSpecChangeSummary => spec !== undefined) } private optionsDetail(width: number) { @@ -2294,11 +2353,13 @@ this.detailBox.title = reviewing ? " review " : " run setup " const lines: StyledText[] = [] lines.push(new StyledText([fg(theme.faint)("pipeline "), bold(fg(theme.text)(choice.name))])) lines.push(new StyledText([fg(theme.faint)("prompt "), fg(theme.text)(truncate(this.prompt, Math.max(10, width - 9)))])) - const spec = this.selectedSpec() - if (spec) { + // Every selected change is shown, in review order: a multi-selection must + // read as its whole ordered list, not just its first entry. + this.selectedSpecs().forEach((spec, index) => { const label = spec.title === spec.id ? spec.id : `${spec.id} · ${spec.title}` - lines.push(new StyledText([sectionLabel("openspec "), fg(theme.teal)(truncate(label, Math.max(10, width - 9)))])) - } + const prefix = index === 0 ? sectionLabel("openspec ") : raw(" ") + lines.push(new StyledText([prefix, fg(theme.teal)(truncate(label, Math.max(10, width - 9)))])) + }) // The continue handoff shows what is being reused, so "no new worktree" // is a visible fact of the options step rather than an assumption. if (this.presetFeature) { @@ -2517,10 +2578,11 @@ this.detailBox.title = reviewing ? " review " : " run setup " /** * The OpenSpec counterpart of the history notice: the checkout's active - * local changes, one of which the operator may pick as the contract. Quiet - * when the checkout has no active change, and when a pick exists the picked - * row already says it. Nothing attaches without an explicit pick — the - * notice points at the decision, it never announces a silent attach. + * local changes, one or more of which the operator may mark as the contract. + * Quiet when the checkout has no active change, and when a selection exists + * the picked rows already say it. Nothing attaches without an explicit + * confirm — the notice points at the decision, it never announces a silent + * attach. */ private pushOpenSpecNotice(lines: StyledText[], width: number) { // A preset change handed in that is not active in this checkout is a stale @@ -2534,7 +2596,7 @@ this.detailBox.title = reviewing ? " review " : " run setup " if (this.specs.length === 0 || this.selectedChangeIds.length > 0 || this.manualNoChanges) return const value = Math.max(8, width - 9) lines.push(plain("")) - lines.push(new StyledText([sectionLabel("openspec "), fg(theme.dim)(truncate(`${this.specs.length} active changes · pick one (esc), or Manual prompt for a no-change run`, value))])) + lines.push(new StyledText([sectionLabel("openspec "), fg(theme.dim)(truncate(`${this.specs.length} active changes · pick one or more (esc), or Manual prompt for a no-change run`, value))])) } /** @@ -2594,9 +2656,11 @@ this.detailBox.title = reviewing ? " review " : " run setup " [ { keys: "↑/↓", label: "select", priority: 2, tone: "dim" }, { keys: "enter", label: this.specIndex === 0 ? "write prompt" : "options", priority: 3 }, + { keys: "space", label: "mark", priority: 4 }, + { keys: "a", label: "all", priority: 5 }, { keys: "esc", label: "back", priority: 1 }, ], - [fg(theme.faint)(`${this.specIndex + 1}/${this.specs.length + 1}`)], + [fg(theme.faint)(`${this.markedChangeIds.length}/${this.specs.length} marked`)], ) } return row( diff --git a/test/change-picker-tui.test.ts b/test/change-picker-tui.test.ts index 0a3d45d..adc815b 100644 --- a/test/change-picker-tui.test.ts +++ b/test/change-picker-tui.test.ts @@ -37,19 +37,77 @@ async function openPicker() { return { testRenderer, result, press: (k: string, o?: { ctrl?: boolean }) => testRenderer.renderer.keyInput.emit("keypress", keyEvent(k, o)) } } -test("the picker lists the changes and Enter selects the highlighted one", async () => { +test("the picker lists the changes; space marks and Enter confirms the batch", async () => { const { testRenderer, result, press } = await openPicker() try { await testRenderer.renderOnce() const frame = testRenderer.captureCharFrame() + expect(frame).toContain("select one or more of 2 active change(s):") expect(frame).toContain("add-widget") expect(frame).toContain("Add widget") expect(frame).toContain("fix-login") + expect(frame).toContain("[ ]") + expect(frame).toContain("[space] mark") expect(frame).toContain("[enter] archive") press("down") + press("space") await testRenderer.renderOnce() + expect(testRenderer.captureCharFrame()).toContain("[x]") press("return") - await expect(result).resolves.toEqual({ kind: "select", changeId: "fix-login" }) + await expect(result).resolves.toEqual({ kind: "select", changeIds: ["fix-login"] }) + } finally { + await testRenderer.mockInput.pressKey("c", { ctrl: true }) + } +}) + +test("several marked changes confirm as one batch ordered by the listing", async () => { + const { testRenderer, result, press } = await openPicker() + try { + press("down") // fix-login + press("space") + press("up") // add-widget + press("space") + press("return") + await expect(result).resolves.toEqual({ kind: "select", changeIds: ["add-widget", "fix-login"] }) + } finally { + await testRenderer.mockInput.pressKey("c", { ctrl: true }) + } +}) + +test("select-all marks every active change", async () => { + const { testRenderer, result, press } = await openPicker() + try { + press("a") + await testRenderer.renderOnce() + expect(testRenderer.captureCharFrame()).toContain("2/2 marked") + press("return") + await expect(result).resolves.toEqual({ kind: "select", changeIds: ["add-widget", "fix-login"] }) + } finally { + await testRenderer.mockInput.pressKey("c", { ctrl: true }) + } +}) + +test("confirming with nothing marked archives nothing", async () => { + const { testRenderer, result, press } = await openPicker() + try { + press("return") + await expect(result).resolves.toEqual({ kind: "cancel" }) + } finally { + await testRenderer.mockInput.pressKey("c", { ctrl: true }) + } +}) + +test("space toggles a mark off, and confirming the emptied selection archives nothing", async () => { + const { testRenderer, result, press } = await openPicker() + try { + press("space") // mark the highlighted add-widget + await testRenderer.renderOnce() + expect(testRenderer.captureCharFrame()).toContain("1/2 marked") + press("space") // unmark it + await testRenderer.renderOnce() + expect(testRenderer.captureCharFrame()).toContain("0/2 marked") + press("return") + await expect(result).resolves.toEqual({ kind: "cancel" }) } finally { await testRenderer.mockInput.pressKey("c", { ctrl: true }) } diff --git a/test/change-selection.test.ts b/test/change-selection.test.ts index e270206..42675b3 100644 --- a/test/change-selection.test.ts +++ b/test/change-selection.test.ts @@ -6,9 +6,12 @@ import { join } from "node:path" import { createFixtureRepo, type FixtureRepo } from "./helpers/multi-worktree" import { acceptSingletonSuggestion, + confirmedChangeSelection, expectedSourcePath, freezeSelectedInputs, + markAllChanges, suggestSingleton, + toggleMarkedChange, validateChangeSelection, type ChangeSelectionInput, type SelectedChangeInput, @@ -110,6 +113,32 @@ describe("validateChangeSelection", () => { }) }) +describe("draft marking helpers", () => { + test("toggle marks and unmarks one change, never adding it twice", () => { + expect(toggleMarkedChange([], "change-a")).toEqual(["change-a"]) + expect(toggleMarkedChange(["change-a"], "change-b")).toEqual(["change-a", "change-b"]) + expect(toggleMarkedChange(["change-a", "change-b"], "change-a")).toEqual(["change-b"]) + expect(toggleMarkedChange(["change-a"], "change-a")).toEqual([]) + }) + + test("select-all marks every active change in listing order, and nothing when the listing is empty", () => { + expect(markAllChanges(["change-a", "change-b", "change-c"])).toEqual(["change-a", "change-b", "change-c"]) + expect(markAllChanges([])).toEqual([]) + }) + + test("a confirmed multi-selection is ordered by the listing, independent of toggle order", () => { + // Toggled change-b then change-a; confirm reads change-a then change-b. + const marked = toggleMarkedChange(toggleMarkedChange([], "change-b"), "change-a") + expect(confirmedChangeSelection(marked, ["change-a", "change-b"])).toEqual(["change-a", "change-b"]) + }) + + test("the confirmed selection dedupes and drops stale marks; an empty selection stays empty", () => { + expect(confirmedChangeSelection(["change-a", "change-a", "change-gone"], ["change-a", "change-b"])).toEqual(["change-a"]) + expect(confirmedChangeSelection([], ["change-a", "change-b"])).toEqual([]) + expect(confirmedChangeSelection([], [])).toEqual([]) + }) +}) + describe("singleton suggestions require explicit acceptance", () => { test("one local change may be suggested; zero or several produce no suggestion", async () => { const checkout = tmpdir() diff --git a/test/launch-tui.test.ts b/test/launch-tui.test.ts index 8672e1a..215ef69 100644 --- a/test/launch-tui.test.ts +++ b/test/launch-tui.test.ts @@ -1693,9 +1693,8 @@ describe("launch TUI OpenSpec contract picker", () => { test("enter on a spec pins change and injects the canned prompt without opening the editor", async () => { const launcher = await createLauncher(100, 30, 1, specs) try { - launcher.mockInput.pressEnter() - launcher.mockInput.pressKey("j") - launcher.mockInput.pressEnter() + launcher.mockInput.pressEnter() // pipelines -> prompt (contract list, focused on add-login) + launcher.mockInput.pressEnter() // confirm the highlighted row await launcher.renderOnce() const view = launchView(launcher.picker) expect(view.mode).toBe("options") @@ -1714,16 +1713,192 @@ describe("launch TUI OpenSpec contract picker", () => { test("enter on Manual prompt opens the editor and does not pin a change", async () => { const launcher = await createLauncher(100, 30, 1, specs) + try { + launcher.mockInput.pressEnter() // pipelines -> prompt (contract list, focused on add-login) + launcher.mockInput.pressKey("k") // move up to the Manual prompt row + launcher.mockInput.pressEnter() // Manual prompt -> editor + await launcher.renderOnce() + const view = launchView(launcher.picker) + expect(view.mode).toBe("prompt") + expect(view.promptChoosing).toBe(false) + expect(view.selectedChangeIds).toEqual([]) + expect(view.manualNoChanges).toBe(true) + expect(launcher.captureCharFrame()).toContain("Add onboarding, fix bug") + } finally { + await closeLauncher(launcher) + } + }) +}) + +describe("launch TUI change picker multi-selection", () => { + const specs: OpenSpecChangeSummary[] = [ + { id: "add-login", title: "Add Login" }, + { id: "add-logout", title: "Add Logout" }, + ] + + test("the picker opens focused on the first active change with its mark boxes", async () => { + const launcher = await createLauncher(100, 30, 1, specs) + try { + launcher.mockInput.pressEnter() + await launcher.renderOnce() + const view = launchView(launcher.picker) + expect(view.promptChoosing).toBe(true) + expect(view.specIndex).toBe(1) + const frame = launcher.captureCharFrame() + expect(frame).toContain("space") + expect(frame).toContain("[ ]") + expect(frame).toContain("0/2 marked") + } finally { + await closeLauncher(launcher) + } + }) + + test("space marks several rows and enter confirms them in listing order", async () => { + const launcher = await createLauncher(100, 30, 1, specs) + try { + launcher.mockInput.pressEnter() // open, highlighted add-login + launcher.mockInput.pressKey("j") // add-logout + launcher.mockInput.pressKey(" ") // mark add-logout first + launcher.mockInput.pressKey("k") // add-login + launcher.mockInput.pressKey(" ") // mark add-login second + await launcher.renderOnce() + expect(launcher.captureCharFrame()).toContain("2/2 marked") + launcher.mockInput.pressEnter() // confirm the marked set -> options + await launcher.renderOnce() + const view = launchView(launcher.picker) + expect(view.mode).toBe("options") + expect(view.manualNoChanges).toBe(false) + expect(view.selectedChangeIds).toEqual(["add-login", "add-logout"]) + expect(view.runSelection("pipeline-1").changes).toEqual(["add-login", "add-logout"]) + // The options step lists every selected change, in review order. + const text = optionsText(view, 110) + expect(text).toContain("add-login · Add Login") + expect(text).toContain("add-logout · Add Logout") + } finally { + await closeLauncher(launcher) + } + }) + + test("select-all marks every active change", async () => { + const launcher = await createLauncher(100, 30, 1, specs) + try { + launcher.mockInput.pressEnter() + launcher.mockInput.pressKey("a") + await launcher.renderOnce() + expect(launcher.captureCharFrame()).toContain("2/2 marked") + launcher.mockInput.pressEnter() + await launcher.renderOnce() + expect(launchView(launcher.picker).selectedChangeIds).toEqual(["add-login", "add-logout"]) + } finally { + await closeLauncher(launcher) + } + }) + + test("re-entry returns the highlight to the first selected change", async () => { + const launcher = await createLauncher(100, 30, 1, specs, ["add-logout"]) + try { + launcher.mockInput.pressEnter() + await launcher.renderOnce() + expect(launchView(launcher.picker).specIndex).toBe(2) + launcher.mockInput.pressEnter() // confirm add-logout -> options + await launcher.renderOnce() + expect(launchView(launcher.picker).selectedChangeIds).toEqual(["add-logout"]) + launcher.mockInput.pressKey("p") // options -> picker again + await launcher.renderOnce() + const view = launchView(launcher.picker) + expect(view.promptChoosing).toBe(true) + expect(view.specIndex).toBe(2) + } finally { + await closeLauncher(launcher) + } + }) + + test("a sole active change is attached only when confirmed, never on open", async () => { + const launcher = await createLauncher(100, 30, 1, [{ id: "only-change", title: "Only Change" }]) try { launcher.mockInput.pressEnter() + await launcher.renderOnce() + const view = launchView(launcher.picker) + expect(view.promptChoosing).toBe(true) + expect(view.selectedChangeIds).toEqual([]) + expect(view.manualNoChanges).toBe(false) launcher.mockInput.pressEnter() await launcher.renderOnce() + expect(launchView(launcher.picker).selectedChangeIds).toEqual(["only-change"]) + } finally { + await closeLauncher(launcher) + } + }) + + test("the no-change row selects nothing even after a change was marked", async () => { + const launcher = await createLauncher(100, 30, 1, specs) + try { + launcher.mockInput.pressEnter() // open, focused add-login + launcher.mockInput.pressKey(" ") // mark add-login + await launcher.renderOnce() + expect(launcher.captureCharFrame()).toContain("1/2 marked") + launcher.mockInput.pressKey("k") // move up to the Manual prompt row + launcher.mockInput.pressEnter() // confirm the no-change row + await launcher.renderOnce() const view = launchView(launcher.picker) expect(view.mode).toBe("prompt") expect(view.promptChoosing).toBe(false) expect(view.selectedChangeIds).toEqual([]) expect(view.manualNoChanges).toBe(true) - expect(launcher.captureCharFrame()).toContain("Add onboarding, fix bug") + } finally { + await closeLauncher(launcher) + } + }) + + test("space on the no-change row marks nothing and it still confirms manual mode", async () => { + const launcher = await createLauncher(100, 30, 1, specs) + try { + launcher.mockInput.pressEnter() // open, focused add-login + launcher.mockInput.pressKey("k") // move up to the Manual prompt row + launcher.mockInput.pressKey(" ") // space on the no-change row: not a markable change + await launcher.renderOnce() + expect(launcher.captureCharFrame()).toContain("0/2 marked") + launcher.mockInput.pressEnter() // the no-change row is still the explicit manual gesture + await launcher.renderOnce() + const view = launchView(launcher.picker) + expect(view.manualNoChanges).toBe(true) + expect(view.selectedChangeIds).toEqual([]) + } finally { + await closeLauncher(launcher) + } + }) + + test("the no-change row clears an existing selection on re-entry", async () => { + const launcher = await createLauncher(100, 30, 1, specs, ["add-login"]) + try { + launcher.mockInput.pressEnter() // open: add-login is seeded as a draft mark + await launcher.renderOnce() + expect(launcher.captureCharFrame()).toContain("1/2 marked") + launcher.mockInput.pressKey("k") // move up to the Manual prompt row + launcher.mockInput.pressEnter() // confirm the no-change row + await launcher.renderOnce() + const view = launchView(launcher.picker) + expect(view.selectedChangeIds).toEqual([]) + expect(view.manualNoChanges).toBe(true) + expect(view.runSelection("pipeline-1").changes).toEqual([]) + } finally { + await closeLauncher(launcher) + } + }) + + test("a preset ordered differently from the listing opens on the first selected change", async () => { + const launcher = await createLauncher(100, 30, 1, specs, ["add-logout", "add-login"]) + try { + launcher.mockInput.pressEnter() // open, both preset changes seeded as draft marks + await launcher.renderOnce() + const view = launchView(launcher.picker) + expect(view.promptChoosing).toBe(true) + expect(view.specIndex).toBe(2) // add-logout: the first selected change, not the first listing id + expect(launcher.captureCharFrame()).toContain("2/2 marked") + launcher.mockInput.pressEnter() // confirm the marked set -> options + await launcher.renderOnce() + // Confirming through the picker reorders to the listing order (design D2). + expect(launchView(launcher.picker).selectedChangeIds).toEqual(["add-login", "add-logout"]) } finally { await closeLauncher(launcher) } @@ -1741,7 +1916,7 @@ describe("launch TUI OpenSpec notice", () => { try { await launcher.renderOnce() const frame = launcher.captureCharFrame() - expect(frame).toContain("2 active changes · pick one (esc)") + expect(frame).toContain("2 active changes · pick one or more (esc)") expect(frame).not.toContain("attaches to every step") } finally { await closeLauncher(launcher) @@ -1749,11 +1924,10 @@ describe("launch TUI OpenSpec notice", () => { }) test("the notice stays quiet once a change is explicitly picked", async () => { - const launcher = await createLauncher(100, 40, 1, specs, ["add-login"]) + const launcher = await createLauncher(100, 40, 1, specs) try { - launcher.mockInput.pressEnter() // contract list - launcher.mockInput.pressKey("j") // index 1 (add-login) - launcher.mockInput.pressKey("j") // index 2 (add-logout) + launcher.mockInput.pressEnter() // contract list (focused on add-login) + launcher.mockInput.pressKey("j") // highlight add-logout launcher.mockInput.pressEnter() // pin add-logout -> options await launcher.renderOnce() const view = launchView(launcher.picker) @@ -1761,7 +1935,7 @@ describe("launch TUI OpenSpec notice", () => { expect(view.selectedChangeIds).toEqual(["add-logout"]) const frame = launcher.captureCharFrame() expect(frame).toContain("add-logout · Add Logout") - expect(frame).not.toContain("pick one (esc)") + expect(frame).not.toContain("pick one or more (esc)") } finally { await closeLauncher(launcher) } @@ -1770,7 +1944,8 @@ describe("launch TUI OpenSpec notice", () => { test("the notice stays quiet once the operator explicitly chose the no-change mode", async () => { const launcher = await createLauncher(100, 40, 1, specs, []) try { - launcher.mockInput.pressEnter() // contract list + launcher.mockInput.pressEnter() // contract list (focused on add-login) + launcher.mockInput.pressKey("k") // move up to the Manual prompt row launcher.mockInput.pressEnter() // Manual prompt -> editor for (const ch of "ship it") launcher.mockInput.pressKey(ch) launcher.mockInput.pressEnter() // submit -> options @@ -1779,7 +1954,7 @@ describe("launch TUI OpenSpec notice", () => { expect(view.mode).toBe("options") expect(view.manualNoChanges).toBe(true) const frame = launcher.captureCharFrame() - expect(frame).not.toContain("pick one (esc)") + expect(frame).not.toContain("pick one or more (esc)") } finally { await closeLauncher(launcher) } @@ -1790,7 +1965,7 @@ describe("launch TUI OpenSpec notice", () => { try { await launcher.renderOnce() const frame = launcher.captureCharFrame() - expect(frame).not.toContain("pick one (esc)") + expect(frame).not.toContain("pick one or more (esc)") expect(frame).not.toContain("active changes") } finally { await closeLauncher(launcher) @@ -1811,7 +1986,7 @@ describe("launch TUI preset changes (specs viewer / worktrees run handoff)", () const view = launchView(launcher.picker) expect(view.selectedChangeIds).toEqual(["add-login"]) // Nothing attaches silently, so there is no auto-attach notice to suppress. - expect(launcher.captureCharFrame()).not.toContain("pick one (esc)") + expect(launcher.captureCharFrame()).not.toContain("pick one or more (esc)") // The flags preview already names the pinned contract. const flags = view.optionsDetail(180).chunks.map((chunk) => chunk.text).join("") expect(flags).toContain("--change add-login") @@ -1876,7 +2051,7 @@ describe("launch TUI preset changes (specs viewer / worktrees run handoff)", () const view = launchView(launcher.picker) expect(view.selectedChangeIds).toEqual([]) expect(view.manualNoChanges).toBe(false) - expect(launcher.captureCharFrame()).toContain("2 active changes · pick one (esc)") + expect(launcher.captureCharFrame()).toContain("2 active changes · pick one or more (esc)") } finally { await closeLauncher(launcher) } diff --git a/test/specs-routing.test.ts b/test/specs-routing.test.ts index 2495eab..1349db3 100644 --- a/test/specs-routing.test.ts +++ b/test/specs-routing.test.ts @@ -24,6 +24,8 @@ const actualLaunchTui = await import("../src/launch-tui") const actualOpencode = await import("../src/opencode") const actualNotice = await import("../src/notice-tui") const actualRemovalConfirm = await import("../src/removal-confirm-tui") +const actualChangePicker = await import("../src/change-picker-tui") +const actualWorktreeCommands = await import("../src/worktree-commands") // Snapshot the real functions BEFORE mock.module: bun patches the module // record in place, so the namespace objects above reflect the mock once @@ -34,13 +36,18 @@ const realLaunchRunTui = actualLaunchTui.launchRunTui const realOpenIterateWindow = actualOpencode.openIterateOpencodeWindow const realShowNotice = actualNotice.showNoticeTui const realShowRemovalConfirm = actualRemovalConfirm.showRemovalConfirmTui +const realShowChangePicker = actualChangePicker.showChangePickerTui +const realRunWorktreeArchive = actualWorktreeCommands.runWorktreeArchive let capturing = false let resolutions: SpecsResolution[] = [{ type: "exit" }] let confirmResult: "confirm" | "cancel" = "confirm" +let pickerResult: { kind: "select"; changeIds: string[] } | { kind: "cancel" } = { kind: "cancel" } const launchCalls: Record[] = [] const iterateCalls: Record[] = [] const noticeCalls: Record[] = [] +const pickerCalls: Record[] = [] +const archiveCalls: Record[] = [] mock.module("../src/specs", () => ({ ...actualSpecs, @@ -85,6 +92,28 @@ mock.module("../src/removal-confirm-tui", () => ({ }, })) +// The Home archive action resolves its batch from the interactive picker and +// hands it to the guarded archive command. Both are mocked here so the routing +// test can assert the confirmed ordered batch is passed through verbatim +// (change `openspec-multi-change-selection`, design D5) rather than re-derived. +mock.module("../src/change-picker-tui", () => ({ + ...actualChangePicker, + showChangePickerTui: async (route: unknown, options: Record) => { + if (!capturing) return realShowChangePicker(route as never, options as never) + pickerCalls.push(options) + return pickerResult + }, +})) + +mock.module("../src/worktree-commands", () => ({ + ...actualWorktreeCommands, + runWorktreeArchive: async (input: Record) => { + if (!capturing) return realRunWorktreeArchive(input as never) + archiveCalls.push(input) + return undefined + }, +})) + const { openSpecsBrowser, dispatchWorkAction } = await import("../src/cli") let root: string @@ -94,8 +123,11 @@ beforeEach(async () => { launchCalls.length = 0 iterateCalls.length = 0 noticeCalls.length = 0 + pickerCalls.length = 0 + archiveCalls.length = 0 resolutions = [{ type: "exit" }] confirmResult = "confirm" + pickerResult = { kind: "cancel" } root = await makeChangeRepo() }) @@ -307,6 +339,70 @@ describe("blocked close handoffs (task 7.9)", () => { }) }) +describe("Home archive action hands the picker's ordered batch to the archive command", () => { + /** + * A worktree with two active changes, so the confirmed batch is a real + * ordered multi-selection (change `openspec-multi-change-selection`, + * capability worktree-operations: "the confirmed batch SHALL be handed to + * the archive command as the reviewed ordered batch"). + */ + async function makeTwoChangeWorktree(label: string) { + const repo = await makeWorktreeRepo(label) + const second = join(repo.worktreeDir, "openspec", "changes", "add-login") + await mkdir(second, { recursive: true }) + await writeFile(join(second, "proposal.md"), "# Add login\n") + return repo + } + + test("the confirmed batch is passed through in order, and the picker is offered the checkout's active changes", async () => { + const { main, worktreeDir } = await makeTwoChangeWorktree("routing-archive-batch") + try { + capturing = true + pickerResult = { kind: "select", changeIds: ["add-login", "add-widget"] } + await dispatchWorkAction(main, {} as never, worktreeDir, "archive") + // The picker saw the checkout's own active changes (never by discovery of + // another checkout): both are offered, sorted by the local listing. + expect(pickerCalls).toHaveLength(1) + const offered = (pickerCalls[0] as { changes: { changeId: string }[] }).changes.map((change) => change.changeId) + expect(offered).toEqual(["add-login", "add-widget"]) + // The confirmed ordered batch reaches the guarded archive verbatim, + // target and all — it is not re-derived, re-sorted, or narrowed. + expect(archiveCalls).toHaveLength(1) + expect((archiveCalls[0] as { worktree: string }).worktree).toBe(worktreeDir) + expect((archiveCalls[0] as { changes: string[] }).changes).toEqual(["add-login", "add-widget"]) + } finally { + capturing = false + await rm(main, { recursive: true, force: true }) + } + }) + + test("a confirmed empty batch archives nothing", async () => { + const { main, worktreeDir } = await makeTwoChangeWorktree("routing-archive-empty") + try { + capturing = true + pickerResult = { kind: "select", changeIds: [] } + await dispatchWorkAction(main, {} as never, worktreeDir, "archive") + expect(archiveCalls).toHaveLength(0) + } finally { + capturing = false + await rm(main, { recursive: true, force: true }) + } + }) + + test("a cancelled picker archives nothing", async () => { + const { main, worktreeDir } = await makeTwoChangeWorktree("routing-archive-cancel") + try { + capturing = true + pickerResult = { kind: "cancel" } + await dispatchWorkAction(main, {} as never, worktreeDir, "archive") + expect(archiveCalls).toHaveLength(0) + } finally { + capturing = false + await rm(main, { recursive: true, force: true }) + } + }) +}) + /** A repo whose worktree carries the change (no registry involved). */ async function makeWorktreeRepo( label: string,