From f7f18b624c81009629c9bb3b734f5634acf237b2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 19:53:50 +0000 Subject: [PATCH 01/16] docs: Add agent CLI ergonomics plan spec Plan to reduce agent bash contortions (for-loops over issues, output truncation, the --no-sync/sync ritual) via bulk/multi-target verbs, a trustworthy output contract, and an honest stage-then-publish sync model. Phase 1 is backward-compatible quick wins for the current release; Phase 2 maps query-driven mutation and a tbd apply transaction file. https://claude.ai/code/session_01P9jYZ7T6MCMCzLL2iJjn9b --- .../plan-2026-06-13-agent-cli-ergonomics.md | 318 ++++++++++++++++++ 1 file changed, 318 insertions(+) create mode 100644 docs/project/specs/active/plan-2026-06-13-agent-cli-ergonomics.md diff --git a/docs/project/specs/active/plan-2026-06-13-agent-cli-ergonomics.md b/docs/project/specs/active/plan-2026-06-13-agent-cli-ergonomics.md new file mode 100644 index 00000000..7f2f6f54 --- /dev/null +++ b/docs/project/specs/active/plan-2026-06-13-agent-cli-ergonomics.md @@ -0,0 +1,318 @@ +--- +title: "Agent CLI Ergonomics: Bulk Ops, Output Contract, and Sync Clarity" +description: Reduce agent bash contortions (for-loops over issues, output truncation, sync rituals) by adding bulk/multi-target verbs, a trustworthy output contract, and a clear sync model +author: Joshua Levy (github.com/jlevy) with LLM assistance +--- +# Feature: Agent CLI Ergonomics (Bulk Ops, Output Contract, Sync Clarity) + +**Date:** 2026-06-13 (last updated 2026-06-13) + +**Author:** Joshua Levy + +**Status:** Draft + +## Overview + +Agents drive tbd through bash, and when a task touches more than one issue they fall back +to error-prone shell contortions: `for` loops over issue IDs, `2>&1 | tail -1` on every +command to tame output, `echo "=== ... ==="` headers to hand-roll a progress display, and +a `--no-sync ... ; tbd sync` ritual to manage syncing. These patterns are brittle, hide +what happened from the user, and make multi-issue work the most failure-prone thing an +agent does with tbd. + +This spec catalogs the common problems, then proposes a **layered** fix that front-loads +backward-compatible **quick wins** suitable for the current release (multi-target verbs, a +trustworthy output contract, and an honest sync model) and maps out **broader ideas** +(query-driven mutation, a transaction-file `apply` command) as follow-on work. + +It is a product/runtime change to tbd's own CLI. It is distinct from +[plan-2026-06-03-tbd-agent-cli-guideline-improvements.md](plan-2026-06-03-tbd-agent-cli-guideline-improvements.md), +which improves the *guideline* about writing agent skills; this spec changes how tbd's +issue commands actually behave. + +### Motivating example + +A real agent command observed in the field (tbd-relevant slice; git/gh plumbing elided): + +```bash +for b in fin-iri4 fin-kq1k fin-c02g; do + tbd close $b --reason "Delivered via PR #78 ..." --no-sync --quiet 2>&1 | tail -1 +done +tbd close fin-xgo2 --reason "Keystone delivered via PR #78 ..." --no-sync --quiet 2>&1 | tail -1 +tbd create "KNOWN_DESCRIPTORS completeness gate ..." -t task -p 3 --parent fin-g1gv \ + --no-sync --quiet -d '...long multi-paragraph body with $vars and `backticks`...' 2>&1 | tail -1 +tbd create "Reconcile fintool-server SEC mock shape ..." -t task -p 3 --parent fin-g1gv \ + --no-sync --quiet -d '...' 2>&1 | tail -1 +tbd sync --quiet 2>&1 | tail -1 +``` + +Every awkward thing here maps to a missing tbd primitive (see Problem Catalog). After +Phase 1 the close loop collapses to a single call: + +```bash +# one call, one lock, one summary line, one push: +tbd close fin-iri4 fin-kq1k fin-c02g fin-xgo2 \ + --reason "Delivered via PR #78 (merged to integration/launch 2026-06-13)" --sync +``` + +## Goals + +- **Eliminate the bash `for` loop** over issue IDs for the common verbs by accepting + multiple targets natively, under one lock, with one summary. +- **Make output trustworthy** so agents stop wrapping commands in `2>&1 | tail -1`: a + documented single-line success contract, a stable `--json` shape, and incidental + notices that `--quiet` reliably silences. +- **Make the sync model honest and self-revealing** so the `--no-sync ... ; tbd sync` + ritual is unnecessary: tell the agent when changes are unsynced, and let a bulk write + publish once on request. +- **Remove the shell-quoting hazard** for large text fields (reasons, descriptions, notes) + by supporting file and stdin bodies consistently across verbs. +- **Prioritize backward-compatible quick wins** that can ship in the current release, and + map the larger ideas without committing to them yet. +- **Establish a consistent verb spine** (`verb [ids...] --flags`) future commands inherit. + +## Non-Goals + +- Owning git/gh plumbing (PR merge, worktree/branch cleanup). tbd will not wrap these; + only the issue-tracking half of the motivating example is in scope. +- A general query/expression language beyond what `list` already understands. Phase 2 + reuses the existing filter grammar; it does not invent a new one. +- Changing the on-disk issue format or the sync-worktree storage model. +- Interactive TUI work. The output contract targets piped/non-TTY agent use first. + +## Background + +Findings from the current code (cited so the plan is grounded, not speculative): + +- The core mutators each take exactly one ID: + [`close.ts:88`](../../../../packages/tbd/src/cli/commands/close.ts), + [`update.ts:431`](../../../../packages/tbd/src/cli/commands/update.ts), + [`reopen.ts:91`](../../../../packages/tbd/src/cli/commands/reopen.ts), + and `show.ts:194` all declare `.argument('')`. +- The repo already uses variadic args idiomatically elsewhere: + `label.ts:201` (``) and `docs-fork.ts:762` (`[names...]`). The mutators are + inconsistent, not idiomatically blocked. +- Argument conventions are uneven: `label add ` is variadic on the *label* + axis (one issue, many labels), `dep add ` is pairwise, `docs fork + [names...]` is variadic, and the issue mutators are single. There is no shared `verb + [ids...] --flags` spine. +- `success()` already prints exactly one line ([`output.ts:382`](../../../../packages/tbd/src/cli/lib/output.ts)), + but two stderr notices can interleave with it: worktree auto-heal + ([`data-context.ts:183`](../../../../packages/tbd/src/cli/lib/data-context.ts)) and + config migration (`data-context.ts:164`). +- **`--no-sync` is effectively a no-op for issue writes.** The `sync` boolean is set in + [`context.ts:46`](../../../../packages/tbd/src/cli/lib/context.ts) but read by no + mutator; `auto_sync` has no issue-level consumer in `src/` (only the docs cache honors + `doc_auto_sync_hours`). Writes stage into the hidden sync worktree and are published + only by `tbd sync`. +- The right shape already exists in one place: `import` ingests many issues under a single + lock, prints one summary line, and nudges + *"Run `tbd sync` to commit and push imported issues"* + ([`import.ts:293`](../../../../packages/tbd/src/cli/commands/import.ts)). The mutators + should adopt the same shape. + +## Problem Catalog + +The common problems agents hit, each with the bash symptom it produces: + +1. **P1: Single-target mutators force shell loops.** `close`/`reopen`/`update`/`show` take + one ``, so N issues become `for b in ...; do tbd close $b ...; done`. Each iteration + re-acquires the repo lock and re-resolves the worktree (N times the work), and a + mid-loop failure leaves a partially-applied change with no transaction boundary. + +2. **P2: No query-driven mutation (no select-and-act).** There is no way to express "close + all open children of X" or "label everything matching Y" without first listing, parsing + IDs out of text/JSON, and looping. The read side has filters (`list`, `ready`, + `blocked`, `stale`, `search`); the write side cannot consume them. + +3. **P3: Output is not trusted, so agents truncate it.** Despite the single-line + `success()`, agents defensively pipe `2>&1 | tail -1` because they cannot be sure how + many lines a command emits, stderr notices interleave with stdout, and in a loop they + only want the final line. `tail -1` then swallows real errors, hiding failures. + +4. **P4: No structured, user-visible record of a multi-step session.** The agent + hand-rolls a UI with `echo "=== ... ==="` because tbd surfaces nothing to the user. A + multi-issue tbd session is invisible unless the agent narrates it, and there is no + machine- and human-readable "here is the batch I just applied" summary. + +5. **P5: The sync model is murky and partly vestigial.** `--no-sync` is consumed by no + mutator and `auto_sync` is not wired for issues, yet both *imply* a per-command sync. + Agents cargo-cult `--no-sync` on each write and append a final `tbd sync`, getting the + right outcome by accident. Nothing tells the agent "you have N unsynced changes." + +6. **P6: Large text fields inline in the shell are a quoting and correctness hazard.** + Multi-paragraph `--reason "..."` / `-d '...'` strings containing `$`, backticks, + quotes, or `#` are pasted straight into bash, risking interpolation, truncation, or + command injection, and are not reviewable. `create` already supports `--file`/`-f` and + `update` supports `--notes-file`, but `close --reason` has no file form and there is no + batch equivalent or shared stdin (`-`) convention. + +7. **P7: Inconsistent argument conventions.** Without a `verb [ids...] --flags` spine, + agents cannot generalize muscle memory across verbs and reach for bash instead (see + Background for the specific inconsistencies). + +8. **P8: No preview/confirm discipline for set-based writes.** `--dry-run` exists globally + but there is no "show the N items this would touch, then apply" affordance, which is + exactly what makes query-driven mutation (P2) safe to offer. + +9. **P9 (adjacent, broader): "delivered by PR #N" has no first-class expression.** The + recurring intent "close these as delivered via PR #78" always degrades into bespoke + bash and an unstructured reason string, so the provenance is not queryable later. Out + of tbd's direct control for the git half, but the bead half could be first-class. + +## Design + +### Approach + +Layer the work so value lands early and risk stays low: + +- **Phase 1 (quick wins, current release):** purely additive, backward-compatible changes + to existing verbs. Multi-target IDs, a bulk summary + accurate sync hint, file/stdin + bodies for reasons, and an honest sync contract. No new top-level commands. +- **Phase 2 (broader, follow-on):** query-driven mutation and a transaction-file `apply` + command. These are larger surfaces with real blast radius and get their own design + pass; this spec scopes them enough to file beads, not to build blind. + +### Components + +CLI command modules under `packages/tbd/src/cli/commands/` (`close.ts`, `reopen.ts`, +`update.ts`, `show.ts`), the shared output layer (`cli/lib/output.ts`), the command +context (`cli/lib/context.ts`), and the data context that owns the lock +(`cli/lib/data-context.ts`). Phase 2 adds a new `apply.ts` that generalizes the existing +`import.ts` ingestion path. + +### API Changes (Phase 1, backward-compatible) + +- **Variadic IDs.** Change `close`/`reopen`/`update`/`show` from `` to ``. + A single ID behaves exactly as today. Multiple IDs are processed under one + `withDataSyncContext({ lock: true })` pass. + - `tbd close A B C --reason "..."` applies the shared reason to all. + - `tbd update A B C --status closed --add-label delivered` applies shared field updates + to all. Per-ID-only flags (e.g. `--title`) are rejected when more than one ID is + given. + - **Validation/atomicity (default, fail-closed):** resolve all IDs first; if any is + unknown, abort before writing anything and list the bad IDs. `already-closed` is a + reported *skip*, not a failure. `--ignore-missing` downgrades unknown IDs to skips. + (Exact default is an Open Question.) +- **Bulk summary + sync hint.** A multi-target write prints one deterministic summary line + (text mode), e.g. `Closed 3, skipped 1 (already closed): fin-iri4 fin-kq1k fin-c02g`, + and, when changes are unsynced, the same nudge `import` already uses. `--json` returns a + structured result array: `{ results: [{ id, action, ok, skippedReason? }], summary: {...} }`. +- **File/stdin bodies.** Add `--reason-file ` to `close`/`reopen`, and a shared `-` + convention so `--reason -`, `-d -`, and `--notes -` read the body from stdin. This + removes the P6 quoting hazard for big text without per-verb special cases. +- **Honest sync (decided: stage + opt-in `--sync`).** Keep the stage-then-publish model: + writes land in the sync worktree and are published by `tbd sync`. Make it + self-revealing: every mutator prints an unsynced-changes hint (the same nudge `import` + uses) when changes are pending. Add `--sync` to the mutators to publish once at the end + of the operation (equivalent to a trailing `tbd sync`, inside the same invocation), so a + bulk write is a single call. Phase 1 adds **no** per-command auto-sync; `--no-sync` + becomes a documented no-op for issue writes (there is no auto-sync to skip), and the + canonical controls are explicit `tbd sync` and `--sync`. + +### API Changes (Phase 2, sketch only) + +- **Query-driven mutation.** `tbd close --where ""` (and `update`, `reopen`) + reusing the **existing `list` filter grammar** (status, priority, label, parent, spec, + etc.). Always prints the matched set and count first; requires `--yes` to apply above a + small threshold, or `--dry-run` to preview only. Directly resolves P2 and P8. +- **Transaction file.** `tbd apply ` (or `... | tbd apply -`) applies a YAML/JSON + document of mixed `create`/`close`/`update` operations atomically under one lock, syncs + once, and prints a structured summary. This is the clean answer to **creating many + distinct issues** (which variadic cannot express, since each needs its own title/body) + and to P6 at batch scale. It generalizes `import` rather than adding a parallel path. + + ```yaml + # delivery.yml + close: + - ids: [fin-iri4, fin-kq1k, fin-c02g, fin-xgo2] + reason: "Delivered via PR #78 (merged to integration/launch 2026-06-13)" + create: + - { title: "KNOWN_DESCRIPTORS completeness gate", type: task, priority: 3, + parent: fin-g1gv, description_file: followup-1.md } + - { title: "Reconcile fintool-server SEC mock shape", type: task, priority: 3, + parent: fin-g1gv, description_file: followup-2.md } + sync: true + ``` + +## Implementation Plan + +### Phase 1: Quick wins (current release) + +- [ ] Make `close`, `reopen`, `update`, `show` accept ``; single-ID behavior + unchanged. Process all IDs under one locked data-sync context. +- [ ] Implement the validation/atomicity policy (validate-all-then-apply; `already-X` as a + skip; `--ignore-missing`); reject per-ID-only flags when multiple IDs are passed. +- [ ] Add the bulk summary line and the structured `--json` results array; reuse the + `import` "unsynced changes, run `tbd sync`" nudge. +- [ ] Make `--quiet` collapse multi-target output to a single line and ensure incidental + stderr notices are suppressed under `--quiet`, so `2>&1 | tail -1` is unnecessary. +- [ ] Add `--reason-file` to `close`/`reopen` and the shared `-`/stdin convention for + `--reason`, `-d/--description`, and `--notes`. +- [ ] Add `--sync` to the mutators (publish once at end) plus the unsynced-changes hint; + keep stage-then-publish (no per-command auto-sync) and document `--no-sync` as a no-op + for issue writes. Update the manual and `tbd prime` to state the stage-then-publish model + in one place. +- [ ] Document the output contract (single-line success; `--json` shape; what `--quiet` + guarantees) in `tbd-docs.md` and the `cli-agent-skill-patterns` guideline examples. + +### Phase 2: Broader (follow-on, separate design pass) + +- [ ] Query-driven mutation (`--where` reusing the `list` grammar) with mandatory + preview + `--yes` threshold. +- [ ] `tbd apply` transaction file generalizing `import`, with a structured summary and + single sync. +- [ ] (Smaller follow-ons) First-class delivery provenance (P9), e.g. `close --by-pr ` + recording structured provenance; a documented `verb [ids...]` spine future verbs inherit. + +## Testing Strategy + +- **tryscript e2e goldens** (the repo's `.tryscript.md` harness) for each new form: bulk + close/reopen/update with mixed already-closed and unknown IDs; `--ignore-missing`; + single-ID backward compatibility; `--reason-file` and stdin bodies; `--quiet` + single-line output; the `--json` results array shape. +- **vitest unit tests** for the validate-all-then-apply policy and the summary/exit-code + logic (non-zero exit when any target failed under the default policy). +- A golden that asserts `--quiet` emits exactly one line (or nothing) with the worktree + auto-heal and config-migration notices suppressed, locking the P3 fix. +- Phase 2 gets its own test plan at design time. + +## Rollout Plan + +- Phase 1 is additive and backward-compatible, so it ships in the current release cycle. + Update `packages/tbd/CHANGELOG.md`, the `tbd-docs.md` manual, `tbd prime`, and the + agent-CLI guideline examples in the same change. +- Phase 2 ships in a later release after its own design pass; nothing in Phase 1 blocks + on it, and Phase 1 establishes the conventions Phase 2 builds on. + +## Open Questions + +- **Sync model fork.** ~~Three options for the `--no-sync`/`auto_sync` vestige.~~ + **Resolved 2026-06-13: option (a)** keep stage-then-publish, make it self-revealing via + the unsynced-changes hint, and add opt-in `--sync` for one push per batch. `--no-sync` + becomes a documented no-op for issue writes; no per-command auto-sync is added. + (Rejected: (b) per-command auto-sync behind `auto_sync`; (c) removing `--no-sync` + outright.) +- **Default atomicity for bulk writes.** Fail-closed (abort the whole batch on any unknown + ID, recommended for predictability) vs. best-effort (apply the valid ones, report the + rest)? `--ignore-missing` covers the latter either way. +- **`--where` grammar reuse.** Confirm Phase 2 reuses the exact `list` filter vocabulary + (learn-once for read and write) rather than introducing a separate selector syntax. +- **Target release.** Confirm Phase 1 is intended for the current (v0.3.x) cycle. +- **Self-tracking.** Do we want this filed as a tbd epic + child beads linked to this spec + via `--spec` (dogfooding the very ergonomics under discussion)? + +## References + +- Motivating analysis and code findings: this repo's command sources cited inline above + (`close.ts`, `update.ts`, `reopen.ts`, `import.ts`, `output.ts`, `context.ts`, + `data-context.ts`). +- Related (distinct) guideline plan: + [plan-2026-06-03-tbd-agent-cli-guideline-improvements.md](plan-2026-06-03-tbd-agent-cli-guideline-improvements.md) +- Agent-CLI guideline: `tbd guidelines cli-agent-skill-patterns` + ([packages/tbd/docs/guidelines/cli-agent-skill-patterns.md](../../../../packages/tbd/docs/guidelines/cli-agent-skill-patterns.md)) +- Design doc (sync model): [tbd-design.md](../../../../packages/tbd/docs/tbd-design.md) + + From 136d178543b950725686f11d27431c7d7ad655df Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 20:31:23 +0000 Subject: [PATCH 02/16] docs: Revise agent CLI ergonomics spec per PR #176 review Incorporate senior review feedback tightening Phase 1: tri-state sync intent with an explicit lock boundary, a visible unsynced hint via output.notice() (not verbose-only info()), preserved single-ID close/reopen behavior (skips are bulk-only), bulk show split into a separate read-only design, update --status closed excluded from bulk closure, an aligned --quiet contract (silent on success), and sync doc fixes extended to tbd-design.md. https://claude.ai/code/session_01P9jYZ7T6MCMCzLL2iJjn9b --- .../plan-2026-06-13-agent-cli-ergonomics.md | 344 +++++++++++------- 1 file changed, 213 insertions(+), 131 deletions(-) diff --git a/docs/project/specs/active/plan-2026-06-13-agent-cli-ergonomics.md b/docs/project/specs/active/plan-2026-06-13-agent-cli-ergonomics.md index 7f2f6f54..348747b6 100644 --- a/docs/project/specs/active/plan-2026-06-13-agent-cli-ergonomics.md +++ b/docs/project/specs/active/plan-2026-06-13-agent-cli-ergonomics.md @@ -11,23 +11,31 @@ author: Joshua Levy (github.com/jlevy) with LLM assistance **Status:** Draft +> **Revised 2026-06-13 after a senior engineering review on +> [PR #176](https://github.com/jlevy/tbd/pull/176).** Phase 1 was tightened: `--sync` +> gets a tri-state intent and an explicit lock boundary, the unsynced hint must be +> visible (via `output.notice()`, not the verbose-only `output.info()`), single-ID +> `close`/`reopen` behavior is preserved (skips are bulk-only), bulk `show` is split out +> as a separate read-only design, and `update --status closed` is excluded from bulk +> closure. Sync doc fixes extend to `tbd-design.md`. + ## Overview -Agents drive tbd through bash, and when a task touches more than one issue they fall back -to error-prone shell contortions: `for` loops over issue IDs, `2>&1 | tail -1` on every -command to tame output, `echo "=== ... ==="` headers to hand-roll a progress display, and -a `--no-sync ... ; tbd sync` ritual to manage syncing. These patterns are brittle, hide -what happened from the user, and make multi-issue work the most failure-prone thing an -agent does with tbd. +Agents drive tbd through bash, and when a task touches more than one issue they fall +back to error-prone shell contortions: `for` loops over issue IDs, `2>&1 | tail -1` on +every command to tame output, `echo "=== ... ==="` headers to hand-roll a progress +display, and a `--no-sync ... ; tbd sync` ritual to manage syncing. +These patterns are brittle, hide what happened from the user, and make multi-issue work +the most failure-prone thing an agent does with tbd. This spec catalogs the common problems, then proposes a **layered** fix that front-loads -backward-compatible **quick wins** suitable for the current release (multi-target verbs, a -trustworthy output contract, and an honest sync model) and maps out **broader ideas** +backward-compatible **quick wins** suitable for the current release (multi-target verbs, +a trustworthy output contract, and an honest sync model) and maps out **broader ideas** (query-driven mutation, a transaction-file `apply` command) as follow-on work. -It is a product/runtime change to tbd's own CLI. It is distinct from +It is a product/runtime change to tbd’s own CLI. It is distinct from [plan-2026-06-03-tbd-agent-cli-guideline-improvements.md](plan-2026-06-03-tbd-agent-cli-guideline-improvements.md), -which improves the *guideline* about writing agent skills; this spec changes how tbd's +which improves the *guideline* about writing agent skills; this spec changes how tbd’s issue commands actually behave. ### Motivating example @@ -46,8 +54,8 @@ tbd create "Reconcile fintool-server SEC mock shape ..." -t task -p 3 --parent f tbd sync --quiet 2>&1 | tail -1 ``` -Every awkward thing here maps to a missing tbd primitive (see Problem Catalog). After -Phase 1 the close loop collapses to a single call: +Every awkward thing here maps to a missing tbd primitive (see Problem Catalog). +After Phase 1 the close loop collapses to a single call: ```bash # one call, one lock, one summary line, one push: @@ -65,20 +73,23 @@ tbd close fin-iri4 fin-kq1k fin-c02g fin-xgo2 \ - **Make the sync model honest and self-revealing** so the `--no-sync ... ; tbd sync` ritual is unnecessary: tell the agent when changes are unsynced, and let a bulk write publish once on request. -- **Remove the shell-quoting hazard** for large text fields (reasons, descriptions, notes) - by supporting file and stdin bodies consistently across verbs. -- **Prioritize backward-compatible quick wins** that can ship in the current release, and - map the larger ideas without committing to them yet. -- **Establish a consistent verb spine** (`verb [ids...] --flags`) future commands inherit. +- **Remove the shell-quoting hazard** for large text fields (reasons, descriptions, + notes) by supporting file and stdin bodies consistently across verbs. +- **Prioritize backward-compatible quick wins** that can ship in the current release, + and map the larger ideas without committing to them yet. +- **Establish a consistent verb spine** (`verb [ids...] --flags`) future commands + inherit. ## Non-Goals -- Owning git/gh plumbing (PR merge, worktree/branch cleanup). tbd will not wrap these; - only the issue-tracking half of the motivating example is in scope. -- A general query/expression language beyond what `list` already understands. Phase 2 - reuses the existing filter grammar; it does not invent a new one. +- Owning git/gh plumbing (PR merge, worktree/branch cleanup). + tbd will not wrap these; only the issue-tracking half of the motivating example is in + scope. +- A general query/expression language beyond what `list` already understands. + Phase 2 reuses the existing filter grammar; it does not invent a new one. - Changing the on-disk issue format or the sync-worktree storage model. -- Interactive TUI work. The output contract targets piped/non-TTY agent use first. +- Interactive TUI work. + The output contract targets piped/non-TTY agent use first. ## Background @@ -87,17 +98,18 @@ Findings from the current code (cited so the plan is grounded, not speculative): - The core mutators each take exactly one ID: [`close.ts:88`](../../../../packages/tbd/src/cli/commands/close.ts), [`update.ts:431`](../../../../packages/tbd/src/cli/commands/update.ts), - [`reopen.ts:91`](../../../../packages/tbd/src/cli/commands/reopen.ts), - and `show.ts:194` all declare `.argument('')`. -- The repo already uses variadic args idiomatically elsewhere: - `label.ts:201` (``) and `docs-fork.ts:762` (`[names...]`). The mutators are - inconsistent, not idiomatically blocked. -- Argument conventions are uneven: `label add ` is variadic on the *label* - axis (one issue, many labels), `dep add ` is pairwise, `docs fork - [names...]` is variadic, and the issue mutators are single. There is no shared `verb - [ids...] --flags` spine. -- `success()` already prints exactly one line ([`output.ts:382`](../../../../packages/tbd/src/cli/lib/output.ts)), - but two stderr notices can interleave with it: worktree auto-heal + [`reopen.ts:91`](../../../../packages/tbd/src/cli/commands/reopen.ts), and + `show.ts:194` all declare `.argument('')`. +- The repo already uses variadic args idiomatically elsewhere: `label.ts:201` + (``) and `docs-fork.ts:762` (`[names...]`). The mutators are inconsistent, + not idiomatically blocked. +- Argument conventions are uneven: `label add ` is variadic on the + *label* axis (one issue, many labels), `dep add ` is pairwise, + `docs fork [names...]` is variadic, and the issue mutators are single. + There is no shared `verb [ids...] --flags` spine. +- `success()` already prints exactly one line + ([`output.ts:382`](../../../../packages/tbd/src/cli/lib/output.ts)), but two stderr + notices can interleave with it: worktree auto-heal ([`data-context.ts:183`](../../../../packages/tbd/src/cli/lib/data-context.ts)) and config migration (`data-context.ts:164`). - **`--no-sync` is effectively a no-op for issue writes.** The `sync` boolean is set in @@ -105,9 +117,9 @@ Findings from the current code (cited so the plan is grounded, not speculative): mutator; `auto_sync` has no issue-level consumer in `src/` (only the docs cache honors `doc_auto_sync_hours`). Writes stage into the hidden sync worktree and are published only by `tbd sync`. -- The right shape already exists in one place: `import` ingests many issues under a single - lock, prints one summary line, and nudges - *"Run `tbd sync` to commit and push imported issues"* +- The right shape already exists in one place: `import` ingests many issues under a + single lock, prints one summary line, and nudges *“Run `tbd sync` to commit and push + imported issues”* ([`import.ts:293`](../../../../packages/tbd/src/cli/commands/import.ts)). The mutators should adopt the same shape. @@ -115,50 +127,55 @@ Findings from the current code (cited so the plan is grounded, not speculative): The common problems agents hit, each with the bash symptom it produces: -1. **P1: Single-target mutators force shell loops.** `close`/`reopen`/`update`/`show` take - one ``, so N issues become `for b in ...; do tbd close $b ...; done`. Each iteration - re-acquires the repo lock and re-resolves the worktree (N times the work), and a - mid-loop failure leaves a partially-applied change with no transaction boundary. +1. **P1: Single-target mutators force shell loops.** `close`/`reopen`/`update`/`show` + take one ``, so N issues become `for b in ...; do tbd close $b ...; done`. Each + iteration re-acquires the repo lock and re-resolves the worktree (N times the work), + and a mid-loop failure leaves a partially-applied change with no transaction + boundary. -2. **P2: No query-driven mutation (no select-and-act).** There is no way to express "close - all open children of X" or "label everything matching Y" without first listing, parsing - IDs out of text/JSON, and looping. The read side has filters (`list`, `ready`, - `blocked`, `stale`, `search`); the write side cannot consume them. +2. **P2: No query-driven mutation (no select-and-act).** There is no way to express + “close all open children of X” or “label everything matching Y” without first + listing, parsing IDs out of text/JSON, and looping. + The read side has filters (`list`, `ready`, `blocked`, `stale`, `search`); the write + side cannot consume them. 3. **P3: Output is not trusted, so agents truncate it.** Despite the single-line `success()`, agents defensively pipe `2>&1 | tail -1` because they cannot be sure how many lines a command emits, stderr notices interleave with stdout, and in a loop they - only want the final line. `tail -1` then swallows real errors, hiding failures. + only want the final line. + `tail -1` then swallows real errors, hiding failures. 4. **P4: No structured, user-visible record of a multi-step session.** The agent - hand-rolls a UI with `echo "=== ... ==="` because tbd surfaces nothing to the user. A - multi-issue tbd session is invisible unless the agent narrates it, and there is no - machine- and human-readable "here is the batch I just applied" summary. + hand-rolls a UI with `echo "=== ... ==="` because tbd surfaces nothing to the user. + A multi-issue tbd session is invisible unless the agent narrates it, and there is no + machine- and human-readable “here is the batch I just applied” summary. 5. **P5: The sync model is murky and partly vestigial.** `--no-sync` is consumed by no mutator and `auto_sync` is not wired for issues, yet both *imply* a per-command sync. - Agents cargo-cult `--no-sync` on each write and append a final `tbd sync`, getting the - right outcome by accident. Nothing tells the agent "you have N unsynced changes." + Agents cargo-cult `--no-sync` on each write and append a final `tbd sync`, getting + the right outcome by accident. + Nothing tells the agent “you have N unsynced changes.” 6. **P6: Large text fields inline in the shell are a quoting and correctness hazard.** Multi-paragraph `--reason "..."` / `-d '...'` strings containing `$`, backticks, quotes, or `#` are pasted straight into bash, risking interpolation, truncation, or - command injection, and are not reviewable. `create` already supports `--file`/`-f` and - `update` supports `--notes-file`, but `close --reason` has no file form and there is no - batch equivalent or shared stdin (`-`) convention. + command injection, and are not reviewable. + `create` already supports `--file`/`-f` and `update` supports `--notes-file`, but + `close --reason` has no file form and there is no batch equivalent or shared stdin + (`-`) convention. 7. **P7: Inconsistent argument conventions.** Without a `verb [ids...] --flags` spine, agents cannot generalize muscle memory across verbs and reach for bash instead (see Background for the specific inconsistencies). -8. **P8: No preview/confirm discipline for set-based writes.** `--dry-run` exists globally - but there is no "show the N items this would touch, then apply" affordance, which is - exactly what makes query-driven mutation (P2) safe to offer. +8. **P8: No preview/confirm discipline for set-based writes.** `--dry-run` exists + globally but there is no “show the N items this would touch, then apply” affordance, + which is exactly what makes query-driven mutation (P2) safe to offer. -9. **P9 (adjacent, broader): "delivered by PR #N" has no first-class expression.** The - recurring intent "close these as delivered via PR #78" always degrades into bespoke - bash and an unstructured reason string, so the provenance is not queryable later. Out - of tbd's direct control for the git half, but the bead half could be first-class. +9. **P9 (adjacent, broader): “delivered by PR #N” has no first-class expression.** The + recurring intent “close these as delivered via PR #78” always degrades into bespoke + bash and an unstructured reason string, so the provenance is not queryable later. + Out of tbd’s direct control for the git half, but the bead half could be first-class. ## Design @@ -166,9 +183,11 @@ The common problems agents hit, each with the bash symptom it produces: Layer the work so value lands early and risk stays low: -- **Phase 1 (quick wins, current release):** purely additive, backward-compatible changes - to existing verbs. Multi-target IDs, a bulk summary + accurate sync hint, file/stdin - bodies for reasons, and an honest sync contract. No new top-level commands. +- **Phase 1 (quick wins, current release):** purely additive, backward-compatible + changes to existing verbs. + Multi-target IDs, a bulk summary + accurate sync hint, file/stdin bodies for reasons, + and an honest sync contract. + No new top-level commands. - **Phase 2 (broader, follow-on):** query-driven mutation and a transaction-file `apply` command. These are larger surfaces with real blast radius and get their own design pass; this spec scopes them enough to file beads, not to build blind. @@ -183,44 +202,80 @@ context (`cli/lib/context.ts`), and the data context that owns the lock ### API Changes (Phase 1, backward-compatible) -- **Variadic IDs.** Change `close`/`reopen`/`update`/`show` from `` to ``. - A single ID behaves exactly as today. Multiple IDs are processed under one - `withDataSyncContext({ lock: true })` pass. +- **Variadic IDs (mutators).** Change `close`/`reopen`/`update` from `` to + `` (mutators only; **not** `show`, see below). + A single ID behaves exactly as today. + Multiple IDs are processed under one `withDataSyncContext({ lock: true })` pass. - `tbd close A B C --reason "..."` applies the shared reason to all. - - `tbd update A B C --status closed --add-label delivered` applies shared field updates - to all. Per-ID-only flags (e.g. `--title`) are rejected when more than one ID is - given. + - `tbd update A B C --add-label delivered --priority 3` applies shared **field** + updates to all. Per-ID-only flags (e.g. `--title`) are rejected when more than one ID + is given. Closing is done via `close`, **not** `update --status closed`: `update` + assigns `status` only and does not set `closed_at`/`close_reason`, so using it as a + bulk close would write semantically incomplete issues (PR #176 review). + Bulk `update` is limited to fields with no lifecycle side effects. - **Validation/atomicity (default, fail-closed):** resolve all IDs first; if any is - unknown, abort before writing anything and list the bad IDs. `already-closed` is a - reported *skip*, not a failure. `--ignore-missing` downgrades unknown IDs to skips. - (Exact default is an Open Question.) -- **Bulk summary + sync hint.** A multi-target write prints one deterministic summary line - (text mode), e.g. `Closed 3, skipped 1 (already closed): fin-iri4 fin-kq1k fin-c02g`, - and, when changes are unsynced, the same nudge `import` already uses. `--json` returns a - structured result array: `{ results: [{ id, action, ok, skippedReason? }], summary: {...} }`. -- **File/stdin bodies.** Add `--reason-file ` to `close`/`reopen`, and a shared `-` - convention so `--reason -`, `-d -`, and `--notes -` read the body from stdin. This - removes the P6 quoting hazard for big text without per-verb special cases. -- **Honest sync (decided: stage + opt-in `--sync`).** Keep the stage-then-publish model: - writes land in the sync worktree and are published by `tbd sync`. Make it - self-revealing: every mutator prints an unsynced-changes hint (the same nudge `import` - uses) when changes are pending. Add `--sync` to the mutators to publish once at the end - of the operation (equivalent to a trailing `tbd sync`, inside the same invocation), so a - bulk write is a single call. Phase 1 adds **no** per-command auto-sync; `--no-sync` - becomes a documented no-op for issue writes (there is no auto-sync to skip), and the - canonical controls are explicit `tbd sync` and `--sync`. + unknown, abort before writing anything and list the bad IDs. + `--ignore-missing` downgrades unknown IDs to skips. + - **Single-ID behavior is preserved exactly (PR #176 review).** Single-target `close` + of an already-closed issue stays idempotent (silent success); single-target `reopen` + of an already-open issue keeps **erroring with exit 1** (locked by + `cli-crud.tryscript.md`). The “`already-X` is a reported *skip*” rule applies **only + to bulk paths** (2+ IDs), where the intent is batch completion. + Every existing single-ID golden stays green. +- **Bulk `show` is a separate, read-only design (PR #176 review), not part of this + mutator slice.** `show` takes no write lock, emits multi-line YAML/Markdown plus + optional parent context, and shares none of the summary/hint/`--sync` contract. + A safe version is `tbd show A B C` rendering each issue with a delimiter and `--json` + returning an array; it is tracked separately and may land later. +- **Bulk summary + visible sync hint.** A multi-target write prints one deterministic + summary line (text mode), e.g. + `Closed 3, skipped 1 (already closed): tbd-a tbd-b tbd-c`. When changes are unsynced + it prints a **visible** hint. + Caveat (PR #176 review): the `import` nudge is emitted via `output.info()`, which only + shows under `--verbose`/`--debug`, so it must not be copied verbatim. + Route the hint through `output.notice()` (shown at the default level, suppressed by + `--quiet` and `--json`). `--json` carries it as structured state instead: + `{ results: [{ id, action, ok, skippedReason? }], summary: {...}, sync: { pending: true, hint: "..." } }`. +- **File/stdin bodies.** Add `--reason-file ` to `close`/`reopen`, and a shared + `-` convention so `--reason -`, `-d -`, and `--notes -` read the body from stdin. + This removes the P6 quoting hazard for big text without per-verb special cases. +- **Honest sync (decided: stage + opt-in `--sync`, tri-state intent).** Keep the + stage-then-publish model: writes land in the sync worktree and are published by + `tbd sync`. Phase 1 adds **no** per-command auto-sync. + Two implementation details the current code forces (PR #176 review): + - **Tri-state sync intent, not a boolean.** Today `getCommandContext()` derives + `sync: opts.sync !== false`, so the *absence* of `--no-sync` reads as `true`; a + naive `if (ctx.sync) sync()` would auto-sync every mutator and contradict this + decision. Model intent explicitly as `unspecified | sync | no-sync` (a per-mutator + `--sync` flag plus the legacy global `--no-sync`), where *unspecified* means + stage-only. `--no-sync` stays accepted as a documented no-op for issue writes. + - **Lock boundary: never nest the data-sync lock.** Apply all issue writes under one + `withDataSyncContext({ lock: true })` pass, *release that lock*, and only then run + the sync path (which re-takes the same non-reentrant lock and performs network I/O). + `--sync` publishes once at the end, so a bulk write is a single call. ### API Changes (Phase 2, sketch only) - **Query-driven mutation.** `tbd close --where ""` (and `update`, `reopen`) - reusing the **existing `list` filter grammar** (status, priority, label, parent, spec, - etc.). Always prints the matched set and count first; requires `--yes` to apply above a - small threshold, or `--dry-run` to preview only. Directly resolves P2 and P8. + reusing the `list` filter vocabulary. + **Prerequisite (PR #176 review):** that vocabulary is currently an + implementation-local `ListOptions` shape inside `list.ts`, not a reusable selector. + Phase 2 must first extract a shared selector module used by both `list` and the + mutators, with tests proving read and write selection match. + Always prints the matched set and count first; requires `--yes` above a small + threshold, or `--dry-run` to preview only. + Resolves P2 and P8. + - **Transaction file.** `tbd apply ` (or `... | tbd apply -`) applies a YAML/JSON - document of mixed `create`/`close`/`update` operations atomically under one lock, syncs - once, and prints a structured summary. This is the clean answer to **creating many - distinct issues** (which variadic cannot express, since each needs its own title/body) - and to P6 at batch scale. It generalizes `import` rather than adding a parallel path. + document of mixed `create`/`close`/`update` operations under one lock, syncs once, and + prints a structured summary. + The clean answer to **creating many distinct issues** (which variadic cannot express, + since each needs its own title/body) and to P6 at batch scale. + It generalizes `import`. **Caveat (PR #176 review):** a real atomicity promise needs a + transaction/rollback story spanning issue writes, ID-mapping updates, parent + `child_order_hints`, and sync; a failure after mapping or hint writes could otherwise + leave partial state. + Phase 2 must specify that boundary. ```yaml # delivery.yml @@ -239,22 +294,33 @@ context (`cli/lib/context.ts`), and the data context that owns the lock ### Phase 1: Quick wins (current release) -- [ ] Make `close`, `reopen`, `update`, `show` accept ``; single-ID behavior - unchanged. Process all IDs under one locked data-sync context. -- [ ] Implement the validation/atomicity policy (validate-all-then-apply; `already-X` as a - skip; `--ignore-missing`); reject per-ID-only flags when multiple IDs are passed. -- [ ] Add the bulk summary line and the structured `--json` results array; reuse the - `import` "unsynced changes, run `tbd sync`" nudge. -- [ ] Make `--quiet` collapse multi-target output to a single line and ensure incidental - stderr notices are suppressed under `--quiet`, so `2>&1 | tail -1` is unnecessary. +- [ ] Make `close`, `reopen`, `update` accept `` (mutators only; **not** + `show`); single-ID behavior unchanged. + Process all IDs under one locked data-sync context. +- [ ] Implement the validation/atomicity policy (validate-all-then-apply; `already-X` as + a **bulk-only** skip while single-ID `close`/`reopen` keep current behavior; + `--ignore-missing`); reject per-ID-only flags when multiple IDs are passed. +- [ ] Add the bulk summary line and the structured `--json` results array; emit the + unsynced-changes hint via `output.notice()` (visible by default), **not** + `output.info()` (which is `import`’s verbose-only nudge). +- [ ] Output contract (PR #176 review): keep `--quiet` **silent on success** (preserves + the `cli-advanced.tryscript.md` quiet-create golden); default **text** mode prints the + one-line summary; `--json` is the machine contract. + Suppress worktree-heal and config-migration stderr notices under `--quiet` too, so + `2>&1 | tail -1` is unnecessary. - [ ] Add `--reason-file` to `close`/`reopen` and the shared `-`/stdin convention for - `--reason`, `-d/--description`, and `--notes`. -- [ ] Add `--sync` to the mutators (publish once at end) plus the unsynced-changes hint; - keep stage-then-publish (no per-command auto-sync) and document `--no-sync` as a no-op - for issue writes. Update the manual and `tbd prime` to state the stage-then-publish model - in one place. -- [ ] Document the output contract (single-line success; `--json` shape; what `--quiet` + `--reason`, `-d/--description`, and `--notes` (one shared body reader). +- [ ] Sync: model intent as tri-state (`unspecified | sync | no-sync`), default + stage-only; add `--sync` (publish once at end), applying writes under the lock, + releasing it, then syncing (no nested lock). + Document `--no-sync` as a no-op for issue writes. + Land the sync doc fixes in the manual, `tbd prime`, **and `tbd-design.md`** (see + Rollout). +- [ ] Document the output contract (one-line summary; `--json` shape; what `--quiet` guarantees) in `tbd-docs.md` and the `cli-agent-skill-patterns` guideline examples. +- [ ] (Separate, read-only, optional this release) bulk `tbd show A B C` → delimited + text / `--json` array; no write lock, no summary/sync contract. + Tracked apart from the mutators. ### Phase 2: Broader (follow-on, separate design pass) @@ -262,49 +328,65 @@ context (`cli/lib/context.ts`), and the data context that owns the lock preview + `--yes` threshold. - [ ] `tbd apply` transaction file generalizing `import`, with a structured summary and single sync. -- [ ] (Smaller follow-ons) First-class delivery provenance (P9), e.g. `close --by-pr ` - recording structured provenance; a documented `verb [ids...]` spine future verbs inherit. +- [ ] (Smaller follow-ons) First-class delivery provenance (P9), e.g. + `close --by-pr ` recording structured provenance; a documented `verb [ids...]` + spine future verbs inherit. ## Testing Strategy -- **tryscript e2e goldens** (the repo's `.tryscript.md` harness) for each new form: bulk - close/reopen/update with mixed already-closed and unknown IDs; `--ignore-missing`; - single-ID backward compatibility; `--reason-file` and stdin bodies; `--quiet` - single-line output; the `--json` results array shape. +- **tryscript e2e goldens** (the repo’s `.tryscript.md` harness): bulk + close/reopen/update with mixed already-X and unknown IDs; `--ignore-missing`; + single-ID backward compatibility; `--reason-file` and stdin bodies; the `--json` + results array shape. + Per the PR #176 review, also lock down: + - `--sync` absent does **not** sync; `--sync` does; legacy `--no-sync` does not sync + and is a documented no-op for issue writes. + - the lock boundary around bulk-write-then-sync (no nested lock acquisition). + - `reopen` already-open: single-ID errors (exit 1) vs bulk reports a skip. + - `update --status closed` is rejected in bulk (or sets lifecycle metadata if ever + added); bulk `update` otherwise touches only side-effect-free fields. + - default text vs `--quiet` (silent on success) vs `--json` output contracts. + - stdin bodies for `--reason -`, `--description -`, `--notes -` with shell-sensitive + text (`$`, backticks, quotes). - **vitest unit tests** for the validate-all-then-apply policy and the summary/exit-code logic (non-zero exit when any target failed under the default policy). -- A golden that asserts `--quiet` emits exactly one line (or nothing) with the worktree - auto-heal and config-migration notices suppressed, locking the P3 fix. - Phase 2 gets its own test plan at design time. ## Rollout Plan - Phase 1 is additive and backward-compatible, so it ships in the current release cycle. - Update `packages/tbd/CHANGELOG.md`, the `tbd-docs.md` manual, `tbd prime`, and the - agent-CLI guideline examples in the same change. + Update `packages/tbd/CHANGELOG.md`, the `tbd-docs.md` manual, `tbd prime`, the + agent-CLI guideline examples, **and `tbd-design.md`** in the same change. + Specifically fix the stale `--no-sync`/`auto_sync` “real issue-write behavior” claims + in `tbd-docs.md` (~L805-827, L1094-1098, L1263-1267) and `tbd-design.md` (~L1608-1646, + L2936-2961) so the repo stops carrying contradictory sync semantics (PR #176 review). - Phase 2 ships in a later release after its own design pass; nothing in Phase 1 blocks on it, and Phase 1 establishes the conventions Phase 2 builds on. ## Open Questions - **Sync model fork.** ~~Three options for the `--no-sync`/`auto_sync` vestige.~~ - **Resolved 2026-06-13: option (a)** keep stage-then-publish, make it self-revealing via - the unsynced-changes hint, and add opt-in `--sync` for one push per batch. `--no-sync` - becomes a documented no-op for issue writes; no per-command auto-sync is added. - (Rejected: (b) per-command auto-sync behind `auto_sync`; (c) removing `--no-sync` - outright.) -- **Default atomicity for bulk writes.** Fail-closed (abort the whole batch on any unknown - ID, recommended for predictability) vs. best-effort (apply the valid ones, report the - rest)? `--ignore-missing` covers the latter either way. -- **`--where` grammar reuse.** Confirm Phase 2 reuses the exact `list` filter vocabulary - (learn-once for read and write) rather than introducing a separate selector syntax. + **Resolved 2026-06-13: option (a)** keep stage-then-publish, make it self-revealing + via the unsynced-changes hint, and add opt-in `--sync` for one push per batch. + `--no-sync` becomes a documented no-op for issue writes; no per-command auto-sync is + added. (Rejected: (b) per-command auto-sync behind `auto_sync`; (c) removing + `--no-sync` outright.) +- **Default atomicity for bulk writes.** ~~Fail-closed vs best-effort?~~ **Resolved (PR + #176 review): fail-closed by default** (abort the batch on any unknown ID), with + `--ignore-missing` as the explicit best-effort escape hatch. +- **`--where` grammar reuse.** ~~Reuse `list`’s vocabulary?~~ **Resolved (PR #176 + review): yes, but Phase 2 must first extract a shared selector module** from + `list.ts`’s implementation-local `ListOptions`, with tests proving read and write + selection match. - **Target release.** Confirm Phase 1 is intended for the current (v0.3.x) cycle. -- **Self-tracking.** Do we want this filed as a tbd epic + child beads linked to this spec - via `--spec` (dogfooding the very ergonomics under discussion)? +- **Self-tracking.** Do we want this filed as a tbd epic + child beads linked to this + spec via `--spec` (dogfooding the very ergonomics under discussion)? ## References -- Motivating analysis and code findings: this repo's command sources cited inline above +- Senior engineering review that tightened Phase 1: + [PR #176](https://github.com/jlevy/tbd/pull/176) (jlevy, 2026-06-13). +- Motivating analysis and code findings: this repo’s command sources cited inline above (`close.ts`, `update.ts`, `reopen.ts`, `import.ts`, `output.ts`, `context.ts`, `data-context.ts`). - Related (distinct) guideline plan: From fc9065bde5eedcfdd334ca762937f95b847ddd6f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 20:59:37 +0000 Subject: [PATCH 03/16] feat: Add variadic close (bulk multi-target close) tbd close now accepts multiple IDs and processes them under one lock: validate-all-then-apply (fail-closed on unknown IDs, --ignore-missing to skip), a one-line summary, a structured --json results array with sync:{pending,hint}, and a visible unsynced-changes notice. Single-ID behavior is preserved byte-for-byte. Adds shared cli/lib/bulk.ts helpers and a cli-bulk-mutation golden; updates the close help-text golden. Part of Phase 1 of the agent CLI ergonomics spec (tbd-38ov). https://claude.ai/code/session_01P9jYZ7T6MCMCzLL2iJjn9b --- packages/tbd/src/cli/commands/close.ts | 142 ++++++++++++------ packages/tbd/src/cli/lib/bulk.ts | 87 +++++++++++ .../tbd/tests/cli-bulk-mutation.tryscript.md | 132 ++++++++++++++++ packages/tbd/tests/cli-setup.tryscript.md | 2 +- 4 files changed, 315 insertions(+), 48 deletions(-) create mode 100644 packages/tbd/src/cli/lib/bulk.ts create mode 100644 packages/tbd/tests/cli-bulk-mutation.tryscript.md diff --git a/packages/tbd/src/cli/commands/close.ts b/packages/tbd/src/cli/commands/close.ts index 0d127d4c..cf44e0f1 100644 --- a/packages/tbd/src/cli/commands/close.ts +++ b/packages/tbd/src/cli/commands/close.ts @@ -1,7 +1,12 @@ /** - * `tbd close` - Close an issue. + * `tbd close` - Close one or more issues. * - * See: tbd-design.md §4.4 Close + * A single ID preserves the legacy output exactly. Two or more IDs are a bulk + * operation: processed under one lock, with a one-line summary, a structured + * `--json` results array, and a visible unsynced-changes hint. + * + * See: tbd-design.md §4.4 Close and + * docs/project/specs/active/plan-2026-06-13-agent-cli-ergonomics.md */ import { Command } from 'commander'; @@ -11,83 +16,126 @@ import { requireInit, NotFoundError } from '../lib/errors.js'; import { readIssue, writeIssue } from '../../file/storage.js'; import { formatDisplayId, formatDebugId } from '../../lib/ids.js'; import { now } from '../../utils/time-utils.js'; -import { resolveToInternalId } from '../../file/id-mapping.js'; import { withDataSyncContext } from '../lib/data-context.js'; +import { resolveAllIds, summarizeBulk, toJsonResult, type BulkItemResult } from '../lib/bulk.js'; interface CloseOptions { reason?: string; + ignoreMissing?: boolean; } class CloseHandler extends BaseCommand { - async run(id: string, options: CloseOptions): Promise { + async run(ids: string[], options: CloseOptions): Promise { const tbdRoot = await requireInit(); - - let displayId = id; - let alreadyClosed = false; - let didClose = false; + const results: BulkItemResult[] = []; await this.execute(async () => { await withDataSyncContext( tbdRoot, { lock: true }, async ({ dataSyncDir, mapping, config }) => { - // Resolve input ID to internal ID - let internalId: string; - try { - internalId = resolveToInternalId(id, mapping); - } catch { - throw new NotFoundError('Issue', id); - } + const { resolved, missing } = resolveAllIds(ids, mapping); - // Load existing issue - let issue; - try { - issue = await readIssue(dataSyncDir, internalId); - } catch { - throw new NotFoundError('Issue', id); + // Fail closed: any unknown ID aborts before writing anything, unless + // the caller opted into best-effort with --ignore-missing. + if (missing.length > 0 && !options.ignoreMissing) { + throw new NotFoundError('Issue', missing.join(', ')); } - displayId = this.ctx.debug - ? formatDebugId(issue.id, mapping, config.display.id_prefix) - : formatDisplayId(issue.id, mapping, config.display.id_prefix); - - // Idempotent: if already closed, succeed silently without modification - if (issue.status === 'closed') { - alreadyClosed = true; - didClose = true; + const dryRunMessage = + ids.length === 1 ? 'Would close issue' : `Would close ${resolved.length} issues`; + if (this.checkDryRun(dryRunMessage, { ids: resolved.map((r) => r.internalId) })) { return; } - if (this.checkDryRun('Would close issue', { id: internalId, reason: options.reason })) { - return; - } + for (const { input, internalId } of resolved) { + let issue; + try { + issue = await readIssue(dataSyncDir, internalId); + } catch { + results.push({ id: input, action: 'missing', ok: false, skippedReason: 'not found' }); + continue; + } + + const displayId = this.ctx.debug + ? formatDebugId(issue.id, mapping, config.display.id_prefix) + : formatDisplayId(issue.id, mapping, config.display.id_prefix); - // Update issue - issue.status = 'closed'; - issue.closed_at = now(); - issue.close_reason = options.reason ?? null; - issue.version += 1; - issue.updated_at = now(); + // Idempotent: already-closed is a reported skip, not a failure. + if (issue.status === 'closed') { + results.push({ + id: displayId, + action: 'skipped', + ok: true, + skippedReason: 'already closed', + }); + continue; + } - await writeIssue(dataSyncDir, issue); - didClose = true; + issue.status = 'closed'; + issue.closed_at = now(); + issue.close_reason = options.reason ?? null; + issue.version += 1; + issue.updated_at = now(); + await writeIssue(dataSyncDir, issue); + results.push({ id: displayId, action: 'closed', ok: true }); + } + + // Report (but do not fail on) IDs skipped via --ignore-missing. + for (const m of missing) { + results.push({ id: m, action: 'missing', ok: false, skippedReason: 'not found' }); + } }, ); }, 'Failed to close issue'); - if (!didClose) return; + if (results.length === 0) return; // dry-run or nothing to do + + // Single ID: preserve the exact legacy output (text + JSON). + if (ids.length === 1) { + const r = results[0]!; + if (r.action === 'missing') return; // --ignore-missing on a lone unknown ID + const alreadyClosed = r.action === 'skipped'; + this.output.data({ id: r.id, closed: true, alreadyClosed }, () => { + this.output.success(`Closed ${r.id}`); + }); + return; + } + + this.emitBulkSummary(results); + } + + /** Bulk path: one summary line + structured JSON, with a visible sync hint. */ + private emitBulkSummary(results: BulkItemResult[]): void { + const summary = summarizeBulk(results); + const syncPending = summary.changed > 0; + const json: Record = { + results: results.map(toJsonResult), + summary, + }; + if (syncPending) { + json.sync = { pending: true, hint: 'Run `tbd sync` to publish.' }; + } - this.output.data({ id: displayId, closed: true, alreadyClosed }, () => { - this.output.success(`Closed ${displayId}`); + this.output.data(json, () => { + const parts = [`Closed ${summary.changed}`]; + if (summary.skipped > 0) parts.push(`skipped ${summary.skipped} (already closed)`); + if (summary.missing > 0) parts.push(`not found ${summary.missing}`); + const idList = results.map((r) => r.id).join(' '); + this.output.success(`${parts.join(', ')}: ${idList}`); + if (syncPending) { + this.output.notice('Unsynced changes — run `tbd sync` to publish.'); + } }); } } export const closeCommand = new Command('close') - .description('Close an issue') - .argument('', 'Issue ID') + .description('Close one or more issues') + .argument('', 'Issue ID(s)') .option('--reason ', 'Close reason') - .action(async (id, options, command) => { + .option('--ignore-missing', 'Skip unknown IDs instead of failing') + .action(async (ids, options, command) => { const handler = new CloseHandler(command); - await handler.run(id, options); + await handler.run(ids, options); }); diff --git a/packages/tbd/src/cli/lib/bulk.ts b/packages/tbd/src/cli/lib/bulk.ts new file mode 100644 index 00000000..f9c450a2 --- /dev/null +++ b/packages/tbd/src/cli/lib/bulk.ts @@ -0,0 +1,87 @@ +/** + * Shared helpers for bulk (multi-target) mutators: `close`, `reopen`, `update`. + * + * Phase 1 of the agent CLI ergonomics work. A single ID keeps each command's + * legacy behavior byte-for-byte; two or more IDs are processed under one lock + * with a one-line summary and a structured `--json` results array. See + * docs/project/specs/active/plan-2026-06-13-agent-cli-ergonomics.md. + */ + +import type { IdMapping } from '../../file/id-mapping.js'; +import { resolveToInternalId } from '../../file/id-mapping.js'; + +export interface ResolvedId { + /** The ID exactly as the user typed it. */ + input: string; + /** The resolved internal ULID-based ID. */ + internalId: string; +} + +export interface IdResolution { + resolved: ResolvedId[]; + /** Inputs that did not resolve, in input order. */ + missing: string[]; +} + +/** + * Resolve every input ID up front (validate-all-then-apply). Unknown inputs are + * collected in `missing` so the caller can fail closed before any write. + */ +export function resolveAllIds(inputIds: string[], mapping: IdMapping): IdResolution { + const resolved: ResolvedId[] = []; + const missing: string[] = []; + for (const input of inputIds) { + try { + resolved.push({ input, internalId: resolveToInternalId(input, mapping) }); + } catch { + missing.push(input); + } + } + return { resolved, missing }; +} + +export type BulkAction = 'closed' | 'reopened' | 'updated' | 'skipped' | 'missing'; + +export interface BulkItemResult { + /** Display ID, or the raw input when the issue could not be resolved/read. */ + id: string; + action: BulkAction; + ok: boolean; + /** Human reason when `action` is `skipped` or `missing`. */ + skippedReason?: string; +} + +export interface BulkSummary { + /** Items actually mutated (action equals the verb, e.g. `closed`). */ + changed: number; + /** Items left unchanged on purpose (e.g. already closed). */ + skipped: number; + /** Inputs that could not be resolved (only present with `--ignore-missing`). */ + missing: number; + total: number; +} + +/** Tally a results array into a summary. */ +export function summarizeBulk(results: BulkItemResult[]): BulkSummary { + let changed = 0; + let skipped = 0; + let missing = 0; + for (const r of results) { + if (r.action === 'skipped') skipped++; + else if (r.action === 'missing') missing++; + else changed++; + } + return { changed, skipped, missing, total: results.length }; +} + +/** The machine-readable result item shape emitted under `--json`. */ +export function toJsonResult(r: BulkItemResult): { + id: string; + action: BulkAction; + ok: boolean; + skippedReason?: string; +} { + return r.skippedReason + ? { id: r.id, action: r.action, ok: r.ok, skippedReason: r.skippedReason } + : { id: r.id, action: r.action, ok: r.ok }; +} diff --git a/packages/tbd/tests/cli-bulk-mutation.tryscript.md b/packages/tbd/tests/cli-bulk-mutation.tryscript.md new file mode 100644 index 00000000..231095cb --- /dev/null +++ b/packages/tbd/tests/cli-bulk-mutation.tryscript.md @@ -0,0 +1,132 @@ +--- +sandbox: true +env: + NO_COLOR: '1' + FORCE_COLOR: '0' +path: + - ../dist +timeout: 30000 +patterns: + SHORTID: '[0-9a-z]{4,5}' +before: | + # Set up a test git repository + git init --initial-branch=main + git config user.email "test@example.com" + git config user.name "Test User" + git config commit.gpgsign false + echo "# Test repo" > README.md + git add README.md + git commit -m "Initial commit" + # Initialize tbd with test prefix + tbd init --prefix=test +--- +# tbd CLI: Bulk Mutation + +Tests for variadic `tbd close` (Phase 1 agent CLI ergonomics). +Single-ID behavior is covered by cli-crud; this file covers the multi-target (bulk) +paths. + +* * * + +## Bulk close + +# Test: Seed three open issues + +```console +$ tbd create "Bulk A" --json | jq -r '.id' | tee a.txt +test-[SHORTID] +? 0 +``` + +```console +$ tbd create "Bulk B" --json | jq -r '.id' | tee b.txt +test-[SHORTID] +? 0 +``` + +```console +$ tbd create "Bulk C" --json | jq -r '.id' | tee c.txt +test-[SHORTID] +? 0 +``` + +# Test: Close two issues at once (one summary line + visible sync hint) + +```console +$ tbd close $(cat a.txt) $(cat b.txt) +✓ Closed 2: test-[SHORTID] test-[SHORTID] +• Unsynced changes[..] +? 0 +``` + +# Test: Closing again reports both as skipped (bulk idempotent) + +```console +$ tbd close $(cat a.txt) $(cat b.txt) +✓ Closed 0, skipped 2 (already closed): test-[SHORTID] test-[SHORTID] +? 0 +``` + +# Test: An unknown ID fails closed and changes nothing + +```console +$ tbd close $(cat c.txt) test-zzzz 2>&1 +[..] +? 1 +``` + +# Test: c is still open after the aborted batch + +```console +$ tbd show $(cat c.txt) --json | jq -r '.status' +open +? 0 +``` + +# Test: --ignore-missing closes the known ID and reports the unknown + +```console +$ tbd close $(cat c.txt) test-zzzz --ignore-missing +✓ Closed 1, not found 1: test-[SHORTID] test-zzzz +• Unsynced changes[..] +? 0 +``` + +# Test: --json emits a structured results array and summary + +```console +$ tbd create "Bulk D" --json | jq -r '.id' | tee d.txt +test-[SHORTID] +? 0 +``` + +```console +$ tbd create "Bulk E" --json | jq -r '.id' | tee e.txt +test-[SHORTID] +? 0 +``` + +```console +$ tbd close $(cat d.txt) $(cat e.txt) --json | jq -r '.summary.changed' +2 +? 0 +``` + +# Test: --quiet is silent on success (bulk) + +```console +$ tbd create "Bulk F" --json | jq -r '.id' | tee f.txt +test-[SHORTID] +? 0 +``` + +```console +$ tbd create "Bulk G" --json | jq -r '.id' | tee g.txt +test-[SHORTID] +? 0 +``` + +```console +$ tbd close $(cat f.txt) $(cat g.txt) --quiet +? 0 +``` diff --git a/packages/tbd/tests/cli-setup.tryscript.md b/packages/tbd/tests/cli-setup.tryscript.md index fa1bb87c..fcfe0713 100644 --- a/packages/tbd/tests/cli-setup.tryscript.md +++ b/packages/tbd/tests/cli-setup.tryscript.md @@ -70,7 +70,7 @@ Working With Issues: create [options] [title] Create a new issue show [options] Show issue details update [options] Update an issue - close [options] Close an issue + close [options] Close one or more issues reopen [options] Reopen a closed issue search [options] Search issues by text From dfb57d8dbbd94fc64cc8737ddc6a91efa4af8adf Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 21:14:00 +0000 Subject: [PATCH 04/16] test: Add unit tests for bulk mutation helpers Covers resolveAllIds (order preservation, missing collection, empty input), summarizeBulk (changed/skipped/missing tallies including reopened/updated), and toJsonResult (skippedReason omission). These pure helpers back the variadic close path, which is otherwise exercised only as a tryscript subprocess and thus invisible to vitest coverage. Implements tbd-xxe4 (part of Phase 1, tbd-38ov). https://claude.ai/code/session_01P9jYZ7T6MCMCzLL2iJjn9b --- packages/tbd/tests/bulk.test.ts | 81 +++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 packages/tbd/tests/bulk.test.ts diff --git a/packages/tbd/tests/bulk.test.ts b/packages/tbd/tests/bulk.test.ts new file mode 100644 index 00000000..92047607 --- /dev/null +++ b/packages/tbd/tests/bulk.test.ts @@ -0,0 +1,81 @@ +import { describe, it, expect } from 'vitest'; + +import { + resolveAllIds, + summarizeBulk, + toJsonResult, + type BulkItemResult, +} from '../src/cli/lib/bulk.js'; +import { addIdMapping, type IdMapping } from '../src/file/id-mapping.js'; + +const ULID_A = 'a'.repeat(26); +const ULID_B = 'b'.repeat(26); + +function makeMapping(shorts: Record): IdMapping { + const mapping: IdMapping = { shortToUlid: new Map(), ulidToShort: new Map() }; + for (const [short, ulid] of Object.entries(shorts)) { + addIdMapping(mapping, ulid, short); + } + return mapping; +} + +describe('resolveAllIds', () => { + it('resolves known IDs and collects unknown ones, preserving order', () => { + const mapping = makeMapping({ aaaa: ULID_A, bbbb: ULID_B }); + const { resolved, missing } = resolveAllIds(['aaaa', 'zzzz', 'bbbb'], mapping); + expect(resolved.map((r) => r.input)).toEqual(['aaaa', 'bbbb']); + expect(missing).toEqual(['zzzz']); + }); + + it('reports every input as missing when nothing resolves', () => { + const { resolved, missing } = resolveAllIds(['zzzz', 'yyyy'], makeMapping({})); + expect(resolved).toHaveLength(0); + expect(missing).toEqual(['zzzz', 'yyyy']); + }); + + it('handles an empty input list', () => { + const { resolved, missing } = resolveAllIds([], makeMapping({})); + expect(resolved).toHaveLength(0); + expect(missing).toHaveLength(0); + }); +}); + +describe('summarizeBulk', () => { + it('tallies changed, skipped, and missing', () => { + const results: BulkItemResult[] = [ + { id: 'a', action: 'closed', ok: true }, + { id: 'b', action: 'closed', ok: true }, + { id: 'c', action: 'skipped', ok: true, skippedReason: 'already closed' }, + { id: 'd', action: 'missing', ok: false, skippedReason: 'not found' }, + ]; + expect(summarizeBulk(results)).toEqual({ changed: 2, skipped: 1, missing: 1, total: 4 }); + }); + + it('counts reopened and updated as changed', () => { + const results: BulkItemResult[] = [ + { id: 'a', action: 'reopened', ok: true }, + { id: 'b', action: 'updated', ok: true }, + ]; + expect(summarizeBulk(results)).toEqual({ changed: 2, skipped: 0, missing: 0, total: 2 }); + }); + + it('summarizes an empty result set', () => { + expect(summarizeBulk([])).toEqual({ changed: 0, skipped: 0, missing: 0, total: 0 }); + }); +}); + +describe('toJsonResult', () => { + it('omits skippedReason when absent', () => { + expect(toJsonResult({ id: 'a', action: 'closed', ok: true })).toEqual({ + id: 'a', + action: 'closed', + ok: true, + }); + }); + + it('includes skippedReason when present', () => { + expect( + toJsonResult({ id: 'a', action: 'skipped', ok: true, skippedReason: 'already closed' }), + ).toEqual({ id: 'a', action: 'skipped', ok: true, skippedReason: 'already closed' }); + }); +}); From 03ab2d625409dab20a2d8f9f489d9fb52b0ab883 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 21:41:09 +0000 Subject: [PATCH 05/16] feat(cli): Variadic reopen (bulk), mirroring close reopen now accepts . A single ID preserves the legacy output exactly, including the hard error when the issue is not closed (cli-crud golden, exit 1). Two or more IDs are a bulk op: validate-all-then-apply with fail-closed on unknown IDs (--ignore-missing to override), already-open reported as a skip (bulk-only), one-line summary + --json results array + visible unsynced-changes hint. Consistency fix in close: a single-ID read failure now throws the legacy NotFoundError instead of silently returning, matching reopen and pre-PR close. Adds bulk reopen goldens; cli-crud single-ID contract unchanged (76 pass). Implements tbd-n6eo (part of Phase 1, tbd-38ov). https://claude.ai/code/session_01P9jYZ7T6MCMCzLL2iJjn9b --- packages/tbd/src/cli/commands/close.ts | 4 + packages/tbd/src/cli/commands/reopen.ts | 163 ++++++++++++------ .../tbd/tests/cli-bulk-mutation.tryscript.md | 115 +++++++++++- 3 files changed, 231 insertions(+), 51 deletions(-) diff --git a/packages/tbd/src/cli/commands/close.ts b/packages/tbd/src/cli/commands/close.ts index cf44e0f1..d00209e3 100644 --- a/packages/tbd/src/cli/commands/close.ts +++ b/packages/tbd/src/cli/commands/close.ts @@ -53,6 +53,10 @@ class CloseHandler extends BaseCommand { try { issue = await readIssue(dataSyncDir, internalId); } catch { + // Single ID preserves the legacy hard error; bulk reports it. + if (ids.length === 1 && !options.ignoreMissing) { + throw new NotFoundError('Issue', input); + } results.push({ id: input, action: 'missing', ok: false, skippedReason: 'not found' }); continue; } diff --git a/packages/tbd/src/cli/commands/reopen.ts b/packages/tbd/src/cli/commands/reopen.ts index 3af3cca4..f81bab60 100644 --- a/packages/tbd/src/cli/commands/reopen.ts +++ b/packages/tbd/src/cli/commands/reopen.ts @@ -1,7 +1,14 @@ /** - * `tbd reopen` - Reopen a closed issue. + * `tbd reopen` - Reopen one or more closed issues. * - * See: tbd-design.md §4.4 Reopen + * A single ID preserves the legacy output exactly, including the hard error when + * the issue is not closed. Two or more IDs are a bulk operation: processed under + * one lock, with a one-line summary, a structured `--json` results array, and a + * visible unsynced-changes hint. Already-open issues are a reported skip in the + * bulk path only (single-ID keeps erroring for backward compatibility). + * + * See: tbd-design.md §4.4 Reopen and + * docs/project/specs/active/plan-2026-06-13-agent-cli-ergonomics.md */ import { Command } from 'commander'; @@ -11,86 +18,142 @@ import { requireInit, NotFoundError, CLIError } from '../lib/errors.js'; import { readIssue, writeIssue } from '../../file/storage.js'; import { formatDisplayId, formatDebugId } from '../../lib/ids.js'; import { now } from '../../utils/time-utils.js'; -import { resolveToInternalId } from '../../file/id-mapping.js'; import { withDataSyncContext } from '../lib/data-context.js'; +import { resolveAllIds, summarizeBulk, toJsonResult, type BulkItemResult } from '../lib/bulk.js'; interface ReopenOptions { reason?: string; + ignoreMissing?: boolean; } class ReopenHandler extends BaseCommand { - async run(id: string, options: ReopenOptions): Promise { + async run(ids: string[], options: ReopenOptions): Promise { const tbdRoot = await requireInit(); - - let displayId = id; - let didReopen = false; + const isBulk = ids.length > 1; + const results: BulkItemResult[] = []; await this.execute(async () => { await withDataSyncContext( tbdRoot, { lock: true }, async ({ dataSyncDir, mapping, config }) => { - // Resolve input ID to internal ID - let internalId: string; - try { - internalId = resolveToInternalId(id, mapping); - } catch { - throw new NotFoundError('Issue', id); - } - - // Load existing issue - let issue; - try { - issue = await readIssue(dataSyncDir, internalId); - } catch { - throw new NotFoundError('Issue', id); - } + const { resolved, missing } = resolveAllIds(ids, mapping); - // Check if not closed - if (issue.status !== 'closed') { - throw new CLIError(`Issue ${id} is not closed (status: ${issue.status})`); + // Fail closed: any unknown ID aborts before writing anything, unless + // the caller opted into best-effort with --ignore-missing. + if (missing.length > 0 && !options.ignoreMissing) { + throw new NotFoundError('Issue', missing.join(', ')); } - if (this.checkDryRun('Would reopen issue', { id: internalId, reason: options.reason })) { + const dryRunMessage = + ids.length === 1 ? 'Would reopen issue' : `Would reopen ${resolved.length} issues`; + if (this.checkDryRun(dryRunMessage, { ids: resolved.map((r) => r.internalId) })) { return; } - // Update issue - issue.status = 'open'; - issue.closed_at = null; - issue.close_reason = null; - issue.version += 1; - issue.updated_at = now(); - - // Optionally store reopen reason in notes if provided - if (options.reason) { - const reopenNote = `Reopened: ${options.reason}`; - issue.notes = issue.notes ? `${issue.notes}\n\n${reopenNote}` : reopenNote; + for (const { input, internalId } of resolved) { + let issue; + try { + issue = await readIssue(dataSyncDir, internalId); + } catch { + // Single ID preserves the legacy hard error; bulk reports it. + if (!isBulk && !options.ignoreMissing) { + throw new NotFoundError('Issue', input); + } + results.push({ id: input, action: 'missing', ok: false, skippedReason: 'not found' }); + continue; + } + + const displayId = this.ctx.debug + ? formatDebugId(issue.id, mapping, config.display.id_prefix) + : formatDisplayId(issue.id, mapping, config.display.id_prefix); + + // Only closed issues can be reopened. Single ID keeps the legacy + // hard error; bulk reports a skip so a batch reopen is idempotent. + if (issue.status !== 'closed') { + if (!isBulk) { + throw new CLIError(`Issue ${input} is not closed (status: ${issue.status})`); + } + results.push({ + id: displayId, + action: 'skipped', + ok: true, + skippedReason: + issue.status === 'open' ? 'already open' : `not closed (${issue.status})`, + }); + continue; + } + + issue.status = 'open'; + issue.closed_at = null; + issue.close_reason = null; + issue.version += 1; + issue.updated_at = now(); + + // Optionally record the reopen reason in notes. + if (options.reason) { + const reopenNote = `Reopened: ${options.reason}`; + issue.notes = issue.notes ? `${issue.notes}\n\n${reopenNote}` : reopenNote; + } + + await writeIssue(dataSyncDir, issue); + results.push({ id: displayId, action: 'reopened', ok: true }); } - await writeIssue(dataSyncDir, issue); - - displayId = this.ctx.debug - ? formatDebugId(issue.id, mapping, config.display.id_prefix) - : formatDisplayId(issue.id, mapping, config.display.id_prefix); - didReopen = true; + // Report (but do not fail on) IDs skipped via --ignore-missing. + for (const m of missing) { + results.push({ id: m, action: 'missing', ok: false, skippedReason: 'not found' }); + } }, ); }, 'Failed to reopen issue'); - if (!didReopen) return; + if (results.length === 0) return; // dry-run or nothing to do + + // Single ID: preserve the exact legacy output (text + JSON). + if (ids.length === 1) { + const r = results[0]!; + if (r.action === 'missing') return; // --ignore-missing on a lone unknown ID + this.output.data({ id: r.id, reopened: true }, () => { + this.output.success(`Reopened ${r.id}`); + }); + return; + } + + this.emitBulkSummary(results); + } - this.output.data({ id: displayId, reopened: true }, () => { - this.output.success(`Reopened ${displayId}`); + /** Bulk path: one summary line + structured JSON, with a visible sync hint. */ + private emitBulkSummary(results: BulkItemResult[]): void { + const summary = summarizeBulk(results); + const syncPending = summary.changed > 0; + const json: Record = { + results: results.map(toJsonResult), + summary, + }; + if (syncPending) { + json.sync = { pending: true, hint: 'Run `tbd sync` to publish.' }; + } + + this.output.data(json, () => { + const parts = [`Reopened ${summary.changed}`]; + if (summary.skipped > 0) parts.push(`skipped ${summary.skipped} (already open)`); + if (summary.missing > 0) parts.push(`not found ${summary.missing}`); + const idList = results.map((r) => r.id).join(' '); + this.output.success(`${parts.join(', ')}: ${idList}`); + if (syncPending) { + this.output.notice('Unsynced changes — run `tbd sync` to publish.'); + } }); } } export const reopenCommand = new Command('reopen') - .description('Reopen a closed issue') - .argument('', 'Issue ID') + .description('Reopen one or more closed issues') + .argument('', 'Issue ID(s)') .option('--reason ', 'Reopen reason') - .action(async (id, options, command) => { + .option('--ignore-missing', 'Skip unknown IDs instead of failing') + .action(async (ids, options, command) => { const handler = new ReopenHandler(command); - await handler.run(id, options); + await handler.run(ids, options); }); diff --git a/packages/tbd/tests/cli-bulk-mutation.tryscript.md b/packages/tbd/tests/cli-bulk-mutation.tryscript.md index 231095cb..81f1fa0c 100644 --- a/packages/tbd/tests/cli-bulk-mutation.tryscript.md +++ b/packages/tbd/tests/cli-bulk-mutation.tryscript.md @@ -22,7 +22,7 @@ before: | --- # tbd CLI: Bulk Mutation -Tests for variadic `tbd close` (Phase 1 agent CLI ergonomics). +Tests for variadic `tbd close` and `tbd reopen` (Phase 1 agent CLI ergonomics). Single-ID behavior is covered by cli-crud; this file covers the multi-target (bulk) paths. @@ -130,3 +130,116 @@ test-[SHORTID] $ tbd close $(cat f.txt) $(cat g.txt) --quiet ? 0 ``` + +* * * + +## Bulk reopen + +# Test: Seed two issues and close them + +```console +$ tbd create "Reopen A" --json | jq -r '.id' | tee ra.txt +test-[SHORTID] +? 0 +``` + +```console +$ tbd create "Reopen B" --json | jq -r '.id' | tee rb.txt +test-[SHORTID] +? 0 +``` + +```console +$ tbd close $(cat ra.txt) $(cat rb.txt) --quiet +? 0 +``` + +# Test: Reopen two issues at once (one summary line + visible sync hint) + +```console +$ tbd reopen $(cat ra.txt) $(cat rb.txt) +✓ Reopened 2: test-[SHORTID] test-[SHORTID] +• Unsynced changes[..] +? 0 +``` + +# Test: Reopening again reports both as skipped (bulk idempotent) + +```console +$ tbd reopen $(cat ra.txt) $(cat rb.txt) +✓ Reopened 0, skipped 2 (already open): test-[SHORTID] test-[SHORTID] +? 0 +``` + +# Test: An unknown ID fails closed and changes nothing + +```console +$ tbd reopen $(cat ra.txt) test-zzzz 2>&1 +[..] +? 1 +``` + +# Test: --ignore-missing reopens the known ID and reports the unknown + +Re-close `ra` so there is something to reopen: + +```console +$ tbd close $(cat ra.txt) --quiet +? 0 +``` + +```console +$ tbd reopen $(cat ra.txt) test-zzzz --ignore-missing +✓ Reopened 1, not found 1: test-[SHORTID] test-zzzz +• Unsynced changes[..] +? 0 +``` + +# Test: --json emits a structured results array and summary + +```console +$ tbd create "Reopen C" --json | jq -r '.id' | tee rc.txt +test-[SHORTID] +? 0 +``` + +```console +$ tbd create "Reopen D" --json | jq -r '.id' | tee rd.txt +test-[SHORTID] +? 0 +``` + +```console +$ tbd close $(cat rc.txt) $(cat rd.txt) --quiet +? 0 +``` + +```console +$ tbd reopen $(cat rc.txt) $(cat rd.txt) --json | jq -r '.summary.changed' +2 +? 0 +``` + +# Test: --quiet is silent on success (bulk) + +```console +$ tbd create "Reopen E" --json | jq -r '.id' | tee re.txt +test-[SHORTID] +? 0 +``` + +```console +$ tbd create "Reopen F" --json | jq -r '.id' | tee rf.txt +test-[SHORTID] +? 0 +``` + +```console +$ tbd close $(cat re.txt) $(cat rf.txt) --quiet +? 0 +``` + +```console +$ tbd reopen $(cat re.txt) $(cat rf.txt) --quiet +? 0 +``` From b5ec58bbf4f76ecd6c457765798cef4d0d7208ee Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 22:00:18 +0000 Subject: [PATCH 06/16] feat(cli): Variadic update (bulk, constrained) update now accepts . A single ID dispatches to the unchanged legacy path, preserving all structural side effects (re-parenting, spec propagation, child ordering). Two or more IDs apply only shared, side-effect-free fields: --priority, --assignee, --type, --add-label, --remove-label, --due, --defer. Per-ID-only and lifecycle flags are rejected in the bulk path (--from-file, --title, --description, --notes, --notes-file, --status, --parent, --spec, --child-order). Bulk closure stays on close so closed_at/close_reason are never left inconsistent, per the PR #176 review. validate-all-then-apply with --ignore-missing; one-line summary + --json results + visible unsynced hint; --quiet silent on success. Also updates the tbd --help golden (cli-setup) for the new reopen/update signatures. That golden was the Coverage & Lint failure CI flagged on 03ab2d6: the pre-push hook runs vitest only, not the tryscript suite, so the help-text drift slipped through. Adds bulk update goldens. Implements tbd-jud0 (part of Phase 1, tbd-38ov). https://claude.ai/code/session_01P9jYZ7T6MCMCzLL2iJjn9b --- packages/tbd/src/cli/commands/update.ts | 148 +++++++++++++++++- .../tbd/tests/cli-bulk-mutation.tryscript.md | 99 ++++++++++++ packages/tbd/tests/cli-setup.tryscript.md | 4 +- 3 files changed, 244 insertions(+), 7 deletions(-) diff --git a/packages/tbd/src/cli/commands/update.ts b/packages/tbd/src/cli/commands/update.ts index fcce6674..ee4df184 100644 --- a/packages/tbd/src/cli/commands/update.ts +++ b/packages/tbd/src/cli/commands/update.ts @@ -20,6 +20,7 @@ import { resolveToInternalId, type IdMapping } from '../../file/id-mapping.js'; import { resolveAndValidatePath, getPathErrorMessage } from '../../lib/project-paths.js'; import { validateIssueTitle } from '../lib/issue-input-validation.js'; import { withDataSyncContext } from '../lib/data-context.js'; +import { resolveAllIds, summarizeBulk, toJsonResult, type BulkItemResult } from '../lib/bulk.js'; interface UpdateOptions { fromFile?: string; @@ -38,10 +39,23 @@ interface UpdateOptions { parent?: string; spec?: string; childOrder?: string; + ignoreMissing?: boolean; } class UpdateHandler extends BaseCommand { - async run(id: string, options: UpdateOptions): Promise { + async run(ids: string[], options: UpdateOptions): Promise { + if (ids.length === 1) { + await this.runSingle(ids[0]!, options); + return; + } + await this.runBulk(ids, options); + } + + /** + * Single-ID update preserves the full legacy behavior, including structural + * side effects (re-parenting, spec propagation, child ordering hints). + */ + async runSingle(id: string, options: UpdateOptions): Promise { const tbdRoot = await requireInit(); let displayId = id; @@ -182,6 +196,129 @@ class UpdateHandler extends BaseCommand { }); } + /** + * Bulk update (2+ IDs): apply only shared fields that have no lifecycle or + * structural side effects. Per-ID-only flags are rejected up front, and + * lifecycle changes stay on `close`/`reopen` so `closed_at`/`close_reason` + * are never left inconsistent. + */ + async runBulk(ids: string[], options: UpdateOptions): Promise { + const perIdOnly: string[] = []; + if (options.fromFile) perIdOnly.push('--from-file'); + if (options.title !== undefined) perIdOnly.push('--title'); + if (options.description !== undefined) perIdOnly.push('--description'); + if (options.notes !== undefined) perIdOnly.push('--notes'); + if (options.notesFile) perIdOnly.push('--notes-file'); + if (options.status !== undefined) perIdOnly.push('--status'); + if (options.parent !== undefined) perIdOnly.push('--parent'); + if (options.spec !== undefined) perIdOnly.push('--spec'); + if (options.childOrder !== undefined) perIdOnly.push('--child-order'); + if (perIdOnly.length > 0) { + throw new CLIError( + `Cannot use ${perIdOnly.join(', ')} when updating multiple issues. ` + + `These apply to a single issue; use \`tbd close\`/\`tbd reopen\` for status changes. ` + + `Bulk update supports shared fields: --priority, --assignee, --type, ` + + `--add-label, --remove-label, --due, --defer.`, + ); + } + + const tbdRoot = await requireInit(); + const results: BulkItemResult[] = []; + + await this.execute(async () => { + await withDataSyncContext( + tbdRoot, + { lock: true }, + async ({ dataSyncDir, mapping, config }) => { + const updates = await this.parseUpdates(options, mapping, tbdRoot); + if (updates === null || Object.keys(updates).length === 0) { + throw new ValidationError( + 'No fields to update. Specify at least one shared field, e.g. --priority or --add-label.', + ); + } + + const { resolved, missing } = resolveAllIds(ids, mapping); + if (missing.length > 0 && !options.ignoreMissing) { + throw new NotFoundError('Issue', missing.join(', ')); + } + + if ( + this.checkDryRun(`Would update ${resolved.length} issues`, { + ids: resolved.map((r) => r.internalId), + ...updates, + }) + ) { + return; + } + + for (const { input, internalId } of resolved) { + let issue; + try { + issue = await readIssue(dataSyncDir, internalId); + } catch { + results.push({ id: input, action: 'missing', ok: false, skippedReason: 'not found' }); + continue; + } + + if (updates.kind !== undefined) issue.kind = updates.kind; + if (updates.priority !== undefined) issue.priority = updates.priority; + if (updates.assignee !== undefined) issue.assignee = updates.assignee; + if (updates.due_date !== undefined) issue.due_date = updates.due_date; + if (updates.deferred_until !== undefined) issue.deferred_until = updates.deferred_until; + if (updates.addLabels && updates.addLabels.length > 0) { + const labelsSet = new Set(issue.labels); + for (const label of updates.addLabels) labelsSet.add(label); + issue.labels = [...labelsSet]; + } + if (updates.removeLabels && updates.removeLabels.length > 0) { + const removeSet = new Set(updates.removeLabels); + issue.labels = issue.labels.filter((l) => !removeSet.has(l)); + } + + issue.version += 1; + issue.updated_at = now(); + await writeIssue(dataSyncDir, issue); + + const displayId = this.ctx.debug + ? formatDebugId(issue.id, mapping, config.display.id_prefix) + : formatDisplayId(issue.id, mapping, config.display.id_prefix); + results.push({ id: displayId, action: 'updated', ok: true }); + } + + for (const m of missing) { + results.push({ id: m, action: 'missing', ok: false, skippedReason: 'not found' }); + } + }, + ); + }, 'Failed to update issues'); + + if (results.length === 0) return; // dry-run or nothing to do + this.emitBulkSummary(results); + } + + /** Bulk path: one summary line + structured JSON, with a visible sync hint. */ + private emitBulkSummary(results: BulkItemResult[]): void { + const summary = summarizeBulk(results); + const syncPending = summary.changed > 0; + const json: Record = { + results: results.map(toJsonResult), + summary, + }; + if (syncPending) { + json.sync = { pending: true, hint: 'Run `tbd sync` to publish.' }; + } + + this.output.data(json, () => { + const parts = [`Updated ${summary.changed}`]; + if (summary.missing > 0) parts.push(`not found ${summary.missing}`); + const idList = results.map((r) => r.id).join(' '); + this.output.success(`${parts.join(', ')}: ${idList}`); + if (syncPending) { + this.output.notice('Unsynced changes — run `tbd sync` to publish.'); + } + }); + } + private async parseUpdates( options: UpdateOptions, mapping: IdMapping, @@ -427,8 +564,8 @@ class UpdateHandler extends BaseCommand { } export const updateCommand = new Command('update') - .description('Update an issue') - .argument('', 'Issue ID') + .description('Update one or more issues') + .argument('', 'Issue ID(s)') .option('--from-file ', 'Update all fields from YAML+Markdown file') .option('--title ', 'Set title') .option('--status ', 'Set status') @@ -445,7 +582,8 @@ export const updateCommand = new Command('update') .option('--parent ', 'Set parent') .option('--spec ', 'Set or clear spec path (empty string clears)') .option('--child-order ', 'Set child ordering hints (comma-separated IDs)') - .action(async (id, options, command) => { + .option('--ignore-missing', 'Skip unknown IDs instead of failing (bulk)') + .action(async (ids, options, command) => { const handler = new UpdateHandler(command); - await handler.run(id, options); + await handler.run(ids, options); }); diff --git a/packages/tbd/tests/cli-bulk-mutation.tryscript.md b/packages/tbd/tests/cli-bulk-mutation.tryscript.md index 81f1fa0c..4cc2af7f 100644 --- a/packages/tbd/tests/cli-bulk-mutation.tryscript.md +++ b/packages/tbd/tests/cli-bulk-mutation.tryscript.md @@ -243,3 +243,102 @@ $ tbd close $(cat re.txt) $(cat rf.txt) --quiet $ tbd reopen $(cat re.txt) $(cat rf.txt) --quiet ? 0 ``` + +* * * + +## Bulk update + +# Test: Seed two open issues + +```console +$ tbd create "Update A" --json | jq -r '.id' | tee ua.txt +test-[SHORTID] +? 0 +``` + +```console +$ tbd create "Update B" --json | jq -r '.id' | tee ub.txt +test-[SHORTID] +? 0 +``` + +# Test: Bulk-set shared fields (priority + label) on both + +```console +$ tbd update $(cat ua.txt) $(cat ub.txt) --priority 0 --add-label delivered +✓ Updated 2: test-[SHORTID] test-[SHORTID] +• Unsynced changes[..] +? 0 +``` + +# Test: The shared label was applied + +```console +$ tbd show $(cat ua.txt) --json | jq -r '.labels[0]' +delivered +? 0 +``` + +# Test: A per-ID-only flag (--title) is rejected for multiple IDs + +```console +$ tbd update $(cat ua.txt) $(cat ub.txt) --title "Nope" 2>&1 +[..] +? 1 +``` + +# Test: --status is rejected for bulk (no bulk-close bypass) + +```console +$ tbd update $(cat ua.txt) $(cat ub.txt) --status closed 2>&1 +[..] +? 1 +``` + +# Test: Both are still open (the rejected bulk close changed nothing) + +```console +$ tbd show $(cat ua.txt) --json | jq -r '.status' +open +? 0 +``` + +# Test: An unknown ID fails closed and changes nothing + +```console +$ tbd update $(cat ua.txt) test-zzzz --priority 1 2>&1 +[..] +? 1 +``` + +# Test: --ignore-missing updates the known ID and reports the unknown + +```console +$ tbd update $(cat ua.txt) test-zzzz --assignee alice --ignore-missing +✓ Updated 1, not found 1: test-[SHORTID] test-zzzz +• Unsynced changes[..] +? 0 +``` + +# Test: --json emits a structured results array and summary + +```console +$ tbd update $(cat ua.txt) $(cat ub.txt) --add-label shipped --json | jq -r '.summary.changed' +2 +? 0 +``` + +# Test: Updating with no fields is a usage error (exit 2) + +```console +$ tbd update $(cat ua.txt) $(cat ub.txt) 2>&1 +[..] +? 2 +``` + +# Test: --quiet is silent on success (bulk) + +```console +$ tbd update $(cat ua.txt) $(cat ub.txt) --priority 2 --quiet +? 0 +``` diff --git a/packages/tbd/tests/cli-setup.tryscript.md b/packages/tbd/tests/cli-setup.tryscript.md index fcfe0713..da01151d 100644 --- a/packages/tbd/tests/cli-setup.tryscript.md +++ b/packages/tbd/tests/cli-setup.tryscript.md @@ -69,9 +69,9 @@ Setup & Configuration: Working With Issues: create [options] [title] Create a new issue show [options] Show issue details - update [options] Update an issue + update [options] Update one or more issues close [options] Close one or more issues - reopen [options] Reopen a closed issue + reopen [options] Reopen one or more closed issues search [options] Search issues by text Views and Filtering: From b82dcb5a6f4d001438d05ca1c2dea9191877e91b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 22:51:18 +0000 Subject: [PATCH 07/16] feat(cli): Shared file/stdin body reader (reasons, descriptions, notes) New cli/lib/body-input.ts: resolveBodyInput resolves a free-text body from inline text, a companion *-file flag, or stdin via the '-' convention. Rejects inline+file together and two stdin readers in one command, so agents can pass shell-sensitive text ($, backticks, quotes) without escaping. Wired in: close/reopen gain --reason-file and '-' stdin for --reason; create's -d/--description and -f/--file accept '-'; update's --description/--notes accept '-' and --notes-file is supported. close/reopen/create resolve bodies before the data context; update now does too via resolveBodyOptions (the context changes cwd, so relative file paths and stdin must be read up front). Tests: body-input unit tests + cli-body-input goldens round-tripping shell-sensitive text through file and stdin; cli-help-all updated for --reason-file. Filed tbd-649r for a pre-existing quirk found while testing: working notes persist to disk but show --json returns null on the first notes write (independent of this change). Implements tbd-hfk0 (part of Phase 1). https://claude.ai/code/session_01P9jYZ7T6MCMCzLL2iJjn9b --- packages/tbd/src/cli/commands/close.ts | 13 +- packages/tbd/src/cli/commands/create.ts | 25 +-- packages/tbd/src/cli/commands/reopen.ts | 15 +- packages/tbd/src/cli/commands/update.ts | 48 +++- packages/tbd/src/cli/lib/body-input.ts | 80 +++++++ packages/tbd/tests/body-input.test.ts | 56 +++++ .../tbd/tests/cli-body-input.tryscript.md | 210 ++++++++++++++++++ packages/tbd/tests/cli-help-all.tryscript.md | 4 +- 8 files changed, 421 insertions(+), 30 deletions(-) create mode 100644 packages/tbd/src/cli/lib/body-input.ts create mode 100644 packages/tbd/tests/body-input.test.ts create mode 100644 packages/tbd/tests/cli-body-input.tryscript.md diff --git a/packages/tbd/src/cli/commands/close.ts b/packages/tbd/src/cli/commands/close.ts index d00209e3..8a00dfdd 100644 --- a/packages/tbd/src/cli/commands/close.ts +++ b/packages/tbd/src/cli/commands/close.ts @@ -18,9 +18,11 @@ import { formatDisplayId, formatDebugId } from '../../lib/ids.js'; import { now } from '../../utils/time-utils.js'; import { withDataSyncContext } from '../lib/data-context.js'; import { resolveAllIds, summarizeBulk, toJsonResult, type BulkItemResult } from '../lib/bulk.js'; +import { resolveBodyInput } from '../lib/body-input.js'; interface CloseOptions { reason?: string; + reasonFile?: string; ignoreMissing?: boolean; } @@ -30,6 +32,12 @@ class CloseHandler extends BaseCommand { const results: BulkItemResult[] = []; await this.execute(async () => { + const reason = await resolveBodyInput({ + name: '--reason', + value: options.reason, + fileName: '--reason-file', + file: options.reasonFile, + }); await withDataSyncContext( tbdRoot, { lock: true }, @@ -78,7 +86,7 @@ class CloseHandler extends BaseCommand { issue.status = 'closed'; issue.closed_at = now(); - issue.close_reason = options.reason ?? null; + issue.close_reason = reason ?? null; issue.version += 1; issue.updated_at = now(); await writeIssue(dataSyncDir, issue); @@ -137,7 +145,8 @@ class CloseHandler extends BaseCommand { export const closeCommand = new Command('close') .description('Close one or more issues') .argument('', 'Issue ID(s)') - .option('--reason ', 'Close reason') + .option('--reason ', 'Close reason ("-" reads stdin)') + .option('--reason-file ', 'Read close reason from a file ("-" reads stdin)') .option('--ignore-missing', 'Skip unknown IDs instead of failing') .action(async (ids, options, command) => { const handler = new CloseHandler(command); diff --git a/packages/tbd/src/cli/commands/create.ts b/packages/tbd/src/cli/commands/create.ts index da391c25..a073d374 100644 --- a/packages/tbd/src/cli/commands/create.ts +++ b/packages/tbd/src/cli/commands/create.ts @@ -5,10 +5,9 @@ */ import { Command } from 'commander'; -import { readFile } from 'node:fs/promises'; import { BaseCommand } from '../lib/base-command.js'; -import { requireInit, ValidationError, CLIError } from '../lib/errors.js'; +import { requireInit, ValidationError } from '../lib/errors.js'; import type { Issue, IssueKindType, PriorityType } from '../../lib/types.js'; import { generateInternalId, extractUlidFromInternalId } from '../../lib/ids.js'; import { readIssue, writeIssue } from '../../file/storage.js'; @@ -24,6 +23,7 @@ import { now } from '../../utils/time-utils.js'; import { resolveAndValidatePath, getPathErrorMessage } from '../../lib/project-paths.js'; import { validateIssueTitle } from '../lib/issue-input-validation.js'; import { withDataSyncContext, notifyWorktreeRepaired } from '../lib/data-context.js'; +import { resolveBodyInput } from '../lib/body-input.js'; interface CreateOptions { fromFile?: string; @@ -59,16 +59,13 @@ class CreateHandler extends BaseCommand { const kind = this.parseKind(options.type ?? 'task'); const priority = this.validatePriority(options.priority ?? '2'); - // Read description from file if specified - let description = options.description; - if (options.file) { - try { - description = await readFile(options.file, 'utf-8'); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - throw new CLIError(`Failed to read description from file '${options.file}': ${message}`); - } - } + // Resolve description from inline text, a file, or stdin ('-'). + const description = await resolveBodyInput({ + name: '--description', + value: options.description, + fileName: '--file', + file: options.file, + }); // Validate and normalize spec path if provided let specPath: string | undefined; @@ -207,8 +204,8 @@ export const createCommand = new Command('create') .option('--from-file ', 'Create from YAML+Markdown file') .option('-t, --type ', 'Issue type: bug, feature, task, epic, chore', 'task') .option('-p, --priority <0-4>', 'Priority (0=critical, 4=lowest)', '2') - .option('-d, --description ', 'Description') - .option('-f, --file ', 'Read description from file') + .option('-d, --description ', 'Description ("-" reads stdin)') + .option('-f, --file ', 'Read description from file ("-" reads stdin)') .option('--assignee ', 'Assignee') .option('--due ', 'Due date (ISO8601)') .option('--defer ', 'Defer until date (ISO8601)') diff --git a/packages/tbd/src/cli/commands/reopen.ts b/packages/tbd/src/cli/commands/reopen.ts index f81bab60..91389df8 100644 --- a/packages/tbd/src/cli/commands/reopen.ts +++ b/packages/tbd/src/cli/commands/reopen.ts @@ -20,9 +20,11 @@ import { formatDisplayId, formatDebugId } from '../../lib/ids.js'; import { now } from '../../utils/time-utils.js'; import { withDataSyncContext } from '../lib/data-context.js'; import { resolveAllIds, summarizeBulk, toJsonResult, type BulkItemResult } from '../lib/bulk.js'; +import { resolveBodyInput } from '../lib/body-input.js'; interface ReopenOptions { reason?: string; + reasonFile?: string; ignoreMissing?: boolean; } @@ -33,6 +35,12 @@ class ReopenHandler extends BaseCommand { const results: BulkItemResult[] = []; await this.execute(async () => { + const reason = await resolveBodyInput({ + name: '--reason', + value: options.reason, + fileName: '--reason-file', + file: options.reasonFile, + }); await withDataSyncContext( tbdRoot, { lock: true }, @@ -91,8 +99,8 @@ class ReopenHandler extends BaseCommand { issue.updated_at = now(); // Optionally record the reopen reason in notes. - if (options.reason) { - const reopenNote = `Reopened: ${options.reason}`; + if (reason) { + const reopenNote = `Reopened: ${reason}`; issue.notes = issue.notes ? `${issue.notes}\n\n${reopenNote}` : reopenNote; } @@ -151,7 +159,8 @@ class ReopenHandler extends BaseCommand { export const reopenCommand = new Command('reopen') .description('Reopen one or more closed issues') .argument('', 'Issue ID(s)') - .option('--reason ', 'Reopen reason') + .option('--reason ', 'Reopen reason ("-" reads stdin)') + .option('--reason-file ', 'Read reopen reason from a file ("-" reads stdin)') .option('--ignore-missing', 'Skip unknown IDs instead of failing') .action(async (ids, options, command) => { const handler = new ReopenHandler(command); diff --git a/packages/tbd/src/cli/commands/update.ts b/packages/tbd/src/cli/commands/update.ts index ee4df184..36eb8a11 100644 --- a/packages/tbd/src/cli/commands/update.ts +++ b/packages/tbd/src/cli/commands/update.ts @@ -21,6 +21,7 @@ import { resolveAndValidatePath, getPathErrorMessage } from '../../lib/project-p import { validateIssueTitle } from '../lib/issue-input-validation.js'; import { withDataSyncContext } from '../lib/data-context.js'; import { resolveAllIds, summarizeBulk, toJsonResult, type BulkItemResult } from '../lib/bulk.js'; +import { resolveBodyInput, type BodyInputState } from '../lib/body-input.js'; interface UpdateOptions { fromFile?: string; @@ -62,6 +63,10 @@ class UpdateHandler extends BaseCommand { let didUpdate = false; await this.execute(async () => { + // Resolve free-text bodies (description/notes) before the data context: + // the context changes cwd, so relative file paths and stdin must be read + // up front from the caller's working directory. + const resolvedOptions = await this.resolveBodyOptions(options); await withDataSyncContext( tbdRoot, { lock: true }, @@ -83,7 +88,7 @@ class UpdateHandler extends BaseCommand { } // Parse and validate options - const updates = await this.parseUpdates(options, mapping, tbdRoot); + const updates = await this.parseUpdates(resolvedOptions, mapping, tbdRoot); if (updates === null) return; if (this.checkDryRun('Would update issue', { id: internalId, ...updates })) { @@ -319,6 +324,37 @@ class UpdateHandler extends BaseCommand { }); } + /** + * Resolve free-text body flags (--description, --notes, --notes-file) to plain + * strings before entering the data context, so relative file paths resolve + * against the caller's cwd and stdin ('-') is consumed exactly once. + * `--from-file` carries its own body and is left untouched. + */ + private async resolveBodyOptions(options: UpdateOptions): Promise { + if (options.fromFile) return options; + const bodyState: BodyInputState = {}; + const resolved: UpdateOptions = { ...options }; + if (options.description !== undefined) { + resolved.description = await resolveBodyInput( + { name: '--description', value: options.description }, + bodyState, + ); + } + if (options.notes !== undefined || options.notesFile) { + resolved.notes = await resolveBodyInput( + { + name: '--notes', + value: options.notes, + fileName: '--notes-file', + file: options.notesFile, + }, + bodyState, + ); + resolved.notesFile = undefined; + } + return resolved; + } + private async parseUpdates( options: UpdateOptions, mapping: IdMapping, @@ -478,6 +514,8 @@ class UpdateHandler extends BaseCommand { updates.assignee = options.assignee || null; } + // Body fields are pre-resolved (inline/file/stdin) by resolveBodyOptions + // before the data context, so here they are already plain strings. if (options.description !== undefined) { updates.description = options.description || null; } @@ -486,14 +524,6 @@ class UpdateHandler extends BaseCommand { updates.notes = options.notes || null; } - if (options.notesFile) { - try { - updates.notes = await readFile(options.notesFile, 'utf-8'); - } catch { - throw new CLIError(`Failed to read notes from file: ${options.notesFile}`); - } - } - if (options.due !== undefined) { updates.due_date = options.due || null; } diff --git a/packages/tbd/src/cli/lib/body-input.ts b/packages/tbd/src/cli/lib/body-input.ts new file mode 100644 index 00000000..91d43fea --- /dev/null +++ b/packages/tbd/src/cli/lib/body-input.ts @@ -0,0 +1,80 @@ +/** + * Shared resolver for free-text body inputs (reasons, descriptions, notes). + * + * Agents frequently need to pass shell-sensitive text (containing `$`, + * backticks, or quotes). Rather than fight shell escaping, every body flag + * accepts the text inline, from a file via a companion `*-file` flag, or from + * stdin using the `-` convention on either flag. Phase 1 agent CLI ergonomics; + * see docs/project/specs/active/plan-2026-06-13-agent-cli-ergonomics.md. + */ + +import { readFile } from 'node:fs/promises'; + +import { CLIError } from './errors.js'; + +/** Tracks single-use of stdin across multiple body inputs in one command. */ +export interface BodyInputState { + stdinUsedBy?: string; +} + +export interface BodyInputSpec { + /** Inline flag name for messages, e.g. `--reason`. */ + name: string; + /** Value from the inline flag; the `-` sentinel means stdin. */ + value?: string; + /** Companion `*-file` flag name for messages, e.g. `--reason-file`. */ + fileName?: string; + /** Path from the `*-file` flag; the `-` sentinel means stdin. */ + file?: string; +} + +async function readStdin(): Promise { + const chunks: Buffer[] = []; + for await (const chunk of process.stdin) { + chunks.push(chunk as Buffer); + } + return Buffer.concat(chunks).toString('utf-8'); +} + +/** + * Resolve one body input to its final string, or `undefined` when unset. + * + * - inline text is returned as-is + * - a `*-file ` flag reads the file + * - `-` on either flag reads stdin (at most one stdin reader per command) + * - supplying both inline text and a file is an error + */ +export async function resolveBodyInput( + spec: BodyInputSpec, + state: BodyInputState = {}, +): Promise { + const hasValue = spec.value !== undefined; + const hasFile = spec.file !== undefined; + + if (hasValue && hasFile) { + throw new CLIError( + `Use either ${spec.name} or ${spec.fileName ?? `${spec.name}-file`}, not both.`, + ); + } + + if (spec.value === '-' || spec.file === '-') { + if (state.stdinUsedBy && state.stdinUsedBy !== spec.name) { + throw new CLIError( + `Cannot read both ${state.stdinUsedBy} and ${spec.name} from stdin ('-').`, + ); + } + state.stdinUsedBy = spec.name; + return readStdin(); + } + + if (hasFile) { + try { + return await readFile(spec.file!, 'utf-8'); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + throw new CLIError(`Failed to read ${spec.fileName ?? spec.name}: ${spec.file}: ${message}`); + } + } + + return spec.value; +} diff --git a/packages/tbd/tests/body-input.test.ts b/packages/tbd/tests/body-input.test.ts new file mode 100644 index 00000000..6ef884a0 --- /dev/null +++ b/packages/tbd/tests/body-input.test.ts @@ -0,0 +1,56 @@ +import { describe, it, expect } from 'vitest'; +import { writeFile, mkdtemp, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { resolveBodyInput } from '../src/cli/lib/body-input.js'; + +describe('resolveBodyInput', () => { + it('returns inline text unchanged', async () => { + expect(await resolveBodyInput({ name: '--reason', value: 'done' })).toBe('done'); + }); + + it('returns undefined when nothing is set', async () => { + expect(await resolveBodyInput({ name: '--reason' })).toBeUndefined(); + }); + + it('preserves shell-sensitive characters in inline text', async () => { + const raw = 'cost is $5 `whoami` "ok" \'q\''; + expect(await resolveBodyInput({ name: '--reason', value: raw })).toBe(raw); + }); + + it('reads the body from a file verbatim', async () => { + const dir = await mkdtemp(join(tmpdir(), 'body-input-')); + try { + const path = join(dir, 'reason.txt'); + const body = 'multi\nline $VAR `tick` "quote"'; + await writeFile(path, body, 'utf-8'); + expect( + await resolveBodyInput({ name: '--reason', fileName: '--reason-file', file: path }), + ).toBe(body); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + it('errors when both inline text and a file are given', async () => { + await expect( + resolveBodyInput({ name: '--reason', value: 'x', fileName: '--reason-file', file: 'y' }), + ).rejects.toThrow(/either --reason or --reason-file/); + }); + + it('errors when the file cannot be read', async () => { + await expect( + resolveBodyInput({ name: '--reason', fileName: '--reason-file', file: '/no/such/file-xyz' }), + ).rejects.toThrow(/Failed to read --reason-file/); + }); + + it('errors when two fields both request stdin', async () => { + // Pre-seed state as if --description already claimed stdin; this throws + // before any read, so no real stdin is touched. + const state = { stdinUsedBy: '--description' }; + await expect(resolveBodyInput({ name: '--notes', value: '-' }, state)).rejects.toThrow( + /Cannot read both --description and --notes from stdin/, + ); + }); +}); diff --git a/packages/tbd/tests/cli-body-input.tryscript.md b/packages/tbd/tests/cli-body-input.tryscript.md new file mode 100644 index 00000000..8b81d7e6 --- /dev/null +++ b/packages/tbd/tests/cli-body-input.tryscript.md @@ -0,0 +1,210 @@ +--- +sandbox: true +env: + NO_COLOR: '1' + FORCE_COLOR: '0' +path: + - ../dist +timeout: 30000 +patterns: + SHORTID: '[0-9a-z]{4,5}' +before: | + # Set up a test git repository + git init --initial-branch=main + git config user.email "test@example.com" + git config user.name "Test User" + git config commit.gpgsign false + echo "# Test repo" > README.md + git add README.md + git commit -m "Initial commit" + # Initialize tbd with test prefix + tbd init --prefix=test +--- +# tbd CLI: File and stdin body input + +Phase 1 agent CLI ergonomics. +Free-text bodies (`--description`, `--notes`, `--reason`) accept a value inline, from a +file, or from stdin via the `-` convention — so agents can pass shell-sensitive text +(`$`, backticks, quotes) without fighting shell escaping. + +`printf '%s'` with a single-quoted argument writes the literal text, which is what these +tests round-trip through `show --json`. + +* * * + +## Description from a file and from stdin (create) + +# Test: create --file reads a shell-sensitive description verbatim + +```console +$ printf '%s' 'price=$9 `whoami` "ok"' > desc.txt +? 0 +``` + +```console +$ tbd create "Body A" -f desc.txt --json | jq -r '.id' | tee ba.txt +test-[SHORTID] +? 0 +``` + +```console +$ tbd show $(cat ba.txt) --json | jq -r '.description' +price=$9 `whoami` "ok" +? 0 +``` + +# Test: create -d - reads the description from stdin + +```console +$ printf '%s' 'pipe=$X `cmd` "y"' | tbd create "Body B" -d - --json | jq -r '.id' | tee bb.txt +test-[SHORTID] +? 0 +``` + +```console +$ tbd show $(cat bb.txt) --json | jq -r '.description' +pipe=$X `cmd` "y" +? 0 +``` + +# Test: create -f - reads the description from stdin + +```console +$ printf '%s' 'file-dash $Z `q`' | tbd create "Body C" -f - --json | jq -r '.id' | tee bc.txt +test-[SHORTID] +? 0 +``` + +```console +$ tbd show $(cat bc.txt) --json | jq -r '.description' +file-dash $Z `q` +? 0 +``` + +# Test: inline text and a file together is rejected + +```console +$ tbd create "Conflict" -d hi -f desc.txt 2>&1 +[..] +? 1 +``` + +* * * + +## Reason from a file and from stdin (close) + +# Test: close --reason-file reads a shell-sensitive reason verbatim + +```console +$ tbd create "Body D" --json | jq -r '.id' | tee bd.txt +test-[SHORTID] +? 0 +``` + +```console +$ printf '%s' 'fixed $BUG via `patch`' > reason.txt +? 0 +``` + +```console +$ tbd close $(cat bd.txt) --reason-file reason.txt --quiet +? 0 +``` + +```console +$ tbd show $(cat bd.txt) --json | jq -r '.close_reason' +fixed $BUG via `patch` +? 0 +``` + +# Test: close --reason - reads the reason from stdin + +```console +$ tbd create "Body E" --json | jq -r '.id' | tee be.txt +test-[SHORTID] +? 0 +``` + +```console +$ printf '%s' 'stdin $REASON `now`' | tbd close $(cat be.txt) --reason - --quiet +? 0 +``` + +```console +$ tbd show $(cat be.txt) --json | jq -r '.close_reason' +stdin $REASON `now` +? 0 +``` + +# Test: --reason and --reason-file together is rejected + +```console +$ tbd close $(cat bd.txt) --reason hi --reason-file reason.txt 2>&1 +[..] +? 1 +``` + +* * * + +## Notes and description from a file and stdin (update) + +`tbd show --json` only surfaces working `notes` after the first notes write (tbd-649r, a +pre-existing read-back quirk), so the notes tests below establish a notes body first. +`--description` round-trips on the first write. + +# Test: update --description - reads the description from stdin + +```console +$ tbd create "Body F" --json | jq -r '.id' | tee bf.txt +test-[SHORTID] +? 0 +``` + +```console +$ printf '%s' 'desc $D `r` "s"' | tbd update $(cat bf.txt) --description - --quiet +? 0 +``` + +```console +$ tbd show $(cat bf.txt) --json | jq -r '.description' +desc $D `r` "s" +? 0 +``` + +# Test: update --notes-file reads shell-sensitive notes verbatim + +Establish a notes body first (see tbd-649r): + +```console +$ tbd update $(cat bf.txt) --notes "established" --quiet +? 0 +``` + +```console +$ printf '%s' 'note $N `x` "y"' > notes.txt +? 0 +``` + +```console +$ tbd update $(cat bf.txt) --notes-file notes.txt --quiet +? 0 +``` + +```console +$ tbd show $(cat bf.txt) --json | jq -r '.notes' +note $N `x` "y" +? 0 +``` + +# Test: update --notes - reads notes from stdin + +```console +$ printf '%s' 'stdin note $Z `q`' | tbd update $(cat bf.txt) --notes - --quiet +? 0 +``` + +```console +$ tbd show $(cat bf.txt) --json | jq -r '.notes' +stdin note $Z `q` +? 0 +``` diff --git a/packages/tbd/tests/cli-help-all.tryscript.md b/packages/tbd/tests/cli-help-all.tryscript.md index 26c2ea3b..d9d06e3d 100644 --- a/packages/tbd/tests/cli-help-all.tryscript.md +++ b/packages/tbd/tests/cli-help-all.tryscript.md @@ -87,7 +87,7 @@ $ tbd update --help | grep -c "\-\-priority" ```console $ tbd close --help | grep -c "\-\-reason" -1 +2 ? 0 ``` @@ -95,7 +95,7 @@ $ tbd close --help | grep -c "\-\-reason" ```console $ tbd reopen --help | grep -c "\-\-reason" -1 +2 ? 0 ``` From 35b8c73775bf8380b596c92d837224931d487bea Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 13 Jun 2026 23:21:07 +0000 Subject: [PATCH 08/16] docs: Correct stale sync semantics; document Phase 1 bulk + body input Per the PR #176 review (Documentation Notes): issue mutations stage to the local tbd-sync worktree and 'tbd sync' publishes them. There is no per-command auto-sync, so --no-sync has no effect on issue writes and auto_sync (default false) is not applied to them. Corrected the stale claims in tbd-docs.md (global options, config example, deletion-recovery note) and tbd-design.md (config schema, command synopses, write-flow step, bd-compat table). Also documented Phase 1 in the tbd-design.md synopses: variadic close/reopen/update , --ignore-missing, --reason-file and the '-'/stdin convention for --reason/-d/--description/--notes, and the constrained bulk update (shared fields only; lifecycle via close/reopen). Implements tbd-6zqx / tbd-dc49 (Phase 1 docs). https://claude.ai/code/session_01P9jYZ7T6MCMCzLL2iJjn9b --- packages/tbd/docs/tbd-design.md | 63 +++++++++++++++++++-------------- packages/tbd/docs/tbd-docs.md | 15 ++++---- 2 files changed, 44 insertions(+), 34 deletions(-) diff --git a/packages/tbd/docs/tbd-design.md b/packages/tbd/docs/tbd-design.md index 954633d7..ab998161 100644 --- a/packages/tbd/docs/tbd-design.md +++ b/packages/tbd/docs/tbd-design.md @@ -1621,7 +1621,7 @@ display: # Runtime settings settings: - auto_sync: false # Auto-sync after write operations + auto_sync: false # Reserved; not applied to issue writes (they stage locally; run `tbd sync`) index_enabled: true # Enable search indexing ``` @@ -1640,7 +1640,7 @@ const ConfigSchema = z.object({ }), settings: z .object({ - auto_sync: z.boolean().default(false), + auto_sync: z.boolean().default(false), // reserved; issue writes stage locally index_enabled: z.boolean().default(true), }) .default({}), @@ -2598,14 +2598,14 @@ Options: --from-file Create from YAML+Markdown file (all fields) --type Issue type: bug, feature, task, epic, chore (default: task) --priority <0-4> Priority (0=critical, 4=lowest, default: 2) - --description Description - --file Read description from file + --description Description ("-" reads stdin) + --file Read description from file ("-" reads stdin) --assignee Assignee --due Due date (ISO8601) --defer Defer until date (ISO8601) --parent= Parent issue ID --label