Skip to content

Configurable sections + interactive picker (#3) - #10

Merged
than merged 13 commits into
mainfrom
feat/configurable-sections
Jul 29, 2026
Merged

Configurable sections + interactive picker (#3)#10
than merged 13 commits into
mainfrom
feat/configurable-sections

Conversation

@than

@than than commented Jul 29, 2026

Copy link
Copy Markdown
Owner

Closes #3.

What

sidecar init now lets you choose your own queue sections — emoji, name, hint, and order — via an interactive bubbletea picker, instead of the fixed five (🧠 Needs action / 🚧 In progress / 🚘 Parked / ✅ Done / 📦 Shipped).

The chosen set flows into all three init-time outputs:

  1. the starter template (headers + hint comment)
  2. the CLAUDE.md note (headers + per-section hints for Claude)
  3. the per-turn reconcile hook message (section list)

Picker keys

jk move · space toggle · J/K reorder · e edit · a add · d delete · done · esc cancel

Edit walks emoji → name → hint; each field shows the current value as placeholder (Enter keeps it, typing replaces). Add-then-abandon and empty-name rows drop automatically.

Scope / safety

  • The viewer is untouched — it renders markdown generically and never parses section names. This is purely an init-time feature; nothing persists.
  • The picker only runs behind a stdinIsTerminal() guard. Piped/non-interactive stdin, cancel, all-deselected, or any picker error all fall back to the default five — never a zero-section file, never a TUI in scripts/CI.
  • No new direct dependency; bubbles/textinput (already-present bubbles module) pulls one small indirect dep (atotto/clipboard).

Tests

37 passing. New coverage: Section model, template generation (incl. hintless sections), CLAUDE.md note + reconcile message for custom sections, and the full picker model (toggle/reorder/bounds/add/edit/delete/cancel/empty-fallback) driven via synthetic key messages.

Spec: docs/superpowers/specs/2026-07-28-configurable-sections-design.md
Plan: docs/superpowers/plans/2026-07-28-configurable-sections.md

Not covered (follow-ups)

  • Manual TTY smoke test of the live picker (unit-tested, but needs a human terminal).
  • Clearing an already-set emoji/hint in the picker (current convention: empty submit keeps the old value).
  • README two-pane diagram emoji-width misalignment (separate cosmetic bug).

🤖 Generated with Claude Code

than and others added 10 commits July 28, 2026 12:54
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#3)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wires pickSections into runInit and offerCreate behind stdinIsTerminal(),
so interactive users' chosen sections flow into scaffold and
offerClaudeHook; non-interactive stdin keeps using defaultSections().
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review

Solid PR overall. The scope discipline is real: git diff --stat confirms ui.go, watcher.go, style.go, and render_test.go are untouched, so none of the hard requirements are in play — parent-directory watching, scroll preservation/clamping, the pane-width cap, and the p.Send goroutine are all exactly as they were. No new goroutines, no shared state. go vet clean, go test -race ./... passes.

The Section / renderTemplate split is the right shape, the stdinIsTerminal() guard plus the "empty result falls back to defaults" rule is the right safety posture, and the picker tests are genuinely good — they drive the real Update with synthetic keys instead of testing a mock.

A few things worth fixing before merge.

a. The picker only guards stdin, not stdout

pickSections runs behind stdinIsTerminal(), but Bubble Tea writes to os.Stdout. Under sidecar init | tee setup.log (or any stdout redirect from an interactive shell) stdin is still a char device, so the picker starts, puts stdin in raw mode, and renders frames into the pipe. The user sees nothing and the command appears to hang while it waits for keys.

The existing readChoice prompts degrade fine in that situation because they're just Printf + read; a full-screen TUI doesn't. Guard on both ends — golang.org/x/term is already a direct dependency and runStatic already uses it:

func interactiveTTY() bool {
	return stdinIsTerminal() && term.IsTerminal(int(os.Stdout.Fd()))
}

and call that instead of stdinIsTerminal() at init.go:30 and init.go:315.

b. Ctrl+C means "use the defaults" instead of "abort"

picker.go:106 maps esc, q, and ctrl+c to the same canceled flag, and pickSections turns a nil result into the default five. So Ctrl+C during the picker doesn't stop sidecar init — it writes SIDECAR.md with the default sections and carries on into the git-exclude and CLAUDE.md prompts.

Esc-means-defaults is a defensible documented choice. Ctrl+C isn't; it's a universal abort and users will hit it expecting nothing to be written. Split it out — give the picker an interrupted field, have pickSections return ([]Section, bool), and let runInit return 1 without scaffolding.

While you're in there: result() gates on p.canceled, but gating on !p.done is stricter and equivalent for the happy path — it also covers a Run() that returns for a reason the key handler never saw. done is otherwise only read by tests.

c. The default template lost its pruning instruction

The old starterTemplate comment carried two lines that don't survive renderTemplate:

· ✅ Done = merged, not yet released; 📦 Shipped = released (tag the version).
· Prune 🧠/🚧 as things move; let ✅/📦 accumulate as a log.

Per-section hints cover the first half, but (tag the version) and the entire prune-vs-accumulate rule are gone from every file init now generates — including the default-sections path, where nothing changed from the user's point of view. That prune rule is what keeps the active sections short enough to fit the pane, which is the whole point of the tool.

It generalizes without knowing section names — something like · Prune the active sections as things move; let the terminal ones accumulate as a log. as a fixed line alongside the other two.

d. The CLAUDE.md section list renders as one run-on paragraph

init.go builds secLines as bare lines with no list marker, and the next template line (Put bare URLs on their own line...) follows with no blank line between. In markdown that's a single paragraph — five headers and their hints soft-wrapped into one blob, then the URL rule glued onto the end. The old version was two lines that read as a sentence, so this is a step back.

Prefix each with - in the builder loop and add a blank line before Put bare URLs.

e. Picker help line is wider than the pane sidecar targets

The help string at picker.go:202 is ~86 columns plus a 2-space indent. sidecar init gets run in the same narrow split pane the viewer lives in, so it wraps. Update also early-returns on every non-KeyMsg (picker.go:49), so tea.WindowSizeMsg never arrives and the picker can't adapt.

Simplest fix is splitting the help across two lines. If you'd rather make it responsive, let tea.WindowSizeMsg through the type switch before the KeyMsg check.

Minor

  • ti.Focus() at picker.go:119 returns a tea.Cmd (the blink command) that's dropped, and the non-KeyMsg early return means textinput never sees blink messages either. Purely cosmetic — the cursor just won't blink.
  • Nothing dedupes section headers, so two rows named the same produce two identical ## X headings in the template. Cheap to reject at commit time in endEdit.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@than

than commented Jul 29, 2026

Copy link
Copy Markdown
Owner Author

Thanks for the thorough pass. Adjudication + fixes (commit f2c3711):

a. stdout not guarded — Fixed. Added interactiveTTY() (stdin and stdout must be TTYs) and swapped it in at both pickSections call sites; the plain readChoice prompts keep the stdin-only check.

b. Ctrl+C = defaults — Fixed. Split ctrl+c from esc/q: Ctrl+C sets a new interrupted flag, pickSections now returns ([]Section, bool), and runInit returns 1 without scaffolding (nothing written) while offerCreate returns before scaffolding. Also moved result() to gate on !p.done as suggested.

c. Lost prune instruction — Fixed. renderTemplate regains a section-name-agnostic prune line (Prune early sections as items move; let later ones accumulate as a log.) and the default Shipped hint is back to released (tag the version).

d. CLAUDE.md run-on paragraph — Fixed. Section list now renders as a real markdown list (- prefix, blank-line fenced).

e. Help line width — Fixed. Split across two lines.

Minor (blink) — Left as-is: purely cosmetic, and fixing it means opening the Update type switch that these changes otherwise keep closed.

Minor (dedup) — Deliberately not changing. The spec explicitly chose to allow duplicate section names (no dedup) — it's a documented decision, so I'm keeping it rather than override the spec. Happy to revisit if you feel strongly.

Full suite green (38 tests), go vet + gofmt clean.

@than

than commented Jul 29, 2026

Copy link
Copy Markdown
Owner Author

@claude please re-review the latest changes (commit f2c3711). I addressed all five findings (a–e) from your prior review; the two minors are handled in my earlier comment. Flag anything still outstanding, otherwise a clean bill is great.

@claude

claude Bot commented Jul 29, 2026

Copy link
Copy Markdown

Claude finished @than's task in 2m 38s —— View job


Re-review of commit f2c3711

  • Gather context (PR diff, prior review, author's fix commit f2c3711)
  • Verify fix (a): stdout TTY guard (interactiveTTY())
  • Verify fix (b): Ctrl+C aborts instead of falling back to defaults
  • Verify fix (c): prune instruction restored in template
  • Verify fix (d): CLAUDE.md section list renders as markdown list
  • Verify fix (e): picker help line wrapped to fit narrow pane
  • Check minors (blink, dedup) were left as documented
  • Attempt go vet / go test -race ./... / gofmt -l . — blocked by sandbox permissions in this run, see note below
  • Post final review

All five findings check out in the diff:

a. stdout guardinteractiveTTY() (init.go:57) checks stdinIsTerminal() && term.IsTerminal(os.Stdout.Fd()) and is used at both call sites (init.go:32, init.go:329). readChoice prompts correctly keep the stdin-only check. tee/redirect-from-a-TTY no longer launches the picker into a pipe.

b. Ctrl+C vs Esc — cleanly split. ctrl+c sets interrupted (picker.go:110-112), distinct from esc/qcanceled (picker.go:107-109). pickSections now returns (sections, interrupted bool); runInit prints "canceled — nothing written" and returns 1 before scaffold is ever called (init.go:34-38), and offerCreate returns before scaffolding too (init.go:331-333). result() gating on !p.done instead of p.canceled is correct and doesn't interact badly with the new interrupted path, since pickSections checks p.interrupted and returns before calling result().

c. Prune instruction — restored as a fixed, section-name-agnostic line (sections.go:56), plus the Shipped hint's (tag the version) is back (sections.go:37). Covered by TestRenderTemplateDefault.

d. CLAUDE.md list formattingsecLines now bullet-prefixed (init.go:69) and the template inserts a blank line before "Put bare URLs..." (init.go:77-79), so it renders as a real list followed by a separate paragraph instead of one run-on blob.

e. Help line width — split across two lines (picker.go:206-207), both comfortably under 80 columns with the 2-space indent.

Minors — blink cosmetic gap and no-dedup are both left as-is per your stated reasoning; agreed on both, especially dedup being an explicit spec decision.

One small thing still open

Ctrl+C only aborts in nav mode. Once you're inside an edit (e/a → emoji/name/hint fields), updateEditing (picker.go:139-166) only special-cases tea.KeyEnter and tea.KeyEsc — everything else, including KeyCtrlC, falls through to p.input.Update(km). bubbles/textinput doesn't treat Ctrl+C specially, so it's silently swallowed: a user who hits Ctrl+C mid-edit expecting the same "universal abort" from finding (b) will find it does nothing rather than aborting. Low severity (esc still gets you back to nav, from which ctrl+c works), but it's the same underlying expectation as (b) in a code path that wasn't touched by this fix. Worth a one-line addition to updateEditing's switch if you want Ctrl+C to be interrupt-everywhere rather than interrupt-from-nav-only. Fix this →

Everything else looks solid — clean bill otherwise. Note: I wasn't able to run go vet/go test -race ./.../gofmt -l . in this sandbox (the go toolchain commands required approval that wasn't grantable in this run); the review above is from manual code reading against the actual diff of f2c3711. If you'd like me to actually execute the test suite in a future run, go needs to be in the allowed-tools list for this action.

updateEditing only handled Enter/Esc, so KeyCtrlC fell through to
textinput and was swallowed. Handle it like nav mode: set interrupted
and quit, so Ctrl+C is a universal abort everywhere in the picker.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@than

than commented Jul 29, 2026

Copy link
Copy Markdown
Owner Author

@claude one more pass please — fixed the open item: Ctrl+C now aborts from inside an edit too (updateEditing handles KeyCtrlC → interrupted, matching nav mode), with a TestPickerInterruptWhileEditing covering it. Full suite green (39), vet+gofmt clean. Commit incoming.

@claude

claude Bot commented Jul 29, 2026

Copy link
Copy Markdown

Claude finished @than's task in 1m 17s —— View job


Re-review of commit 3e694bc

  • Gather context (diff f2c3711..3e694bc, prior review thread)
  • Verify Ctrl+C-while-editing fix in updateEditing
  • Verify TestPickerInterruptWhileEditing coverage
  • Attempt go vet / go test -race ./... / gofmt -l . — blocked by sandbox permissions again, see note below
  • Post final review

The fix is exactly scoped to the one open item — git diff f2c3711..3e694bc --stat shows only picker.go (+5) and picker_test.go (+19), nothing else touched.

picker.go:162-166updateEditing's switch km.Type now has a case tea.KeyCtrlC alongside KeyEnter/KeyEsc, setting p.interrupted = true and returning tea.Quit. This mirrors updateNav's case "ctrl+c" (picker.go:110-112) exactly, so Ctrl+C is now a universal abort in both modes.

A couple of things worth confirming, both check out:

  • The row being edited (e.g. a fresh a-added blank row) is left in p.rows on this path since endEdit() is skipped — but that's harmless: result() gates on !p.done (picker.go:190), and done is never set on the interrupt path, so result() returns nil regardless of row state. Same as the existing nav-mode interrupt.
  • km.Type matching is the right check here (not km.String()) — consistent with how the other KeyEnter/KeyEsc cases in the same switch are matched, and how bubbletea reports Ctrl+C.

picker_test.go:139-155TestPickerInterruptWhileEditing enters edit mode with e, confirms p.editing != fieldNone, sends KeyCtrlC directly (bypassing the send()/key() helper since ctrl+c isn't in its switch — same pattern TestPickerInterrupt already uses), and asserts interrupted, a non-nil cmd, and result() == nil. Good, targeted coverage that exercises the real Update path rather than calling updateEditing directly.

This closes out the last open item from the previous review. Nothing else outstanding — clean bill.

Note: I wasn't able to execute go build/go vet/go test -race ./.../gofmt -l . in this sandbox (the commands require approval that wasn't grantable in this run, same limitation as the prior re-review pass) — the review above is from manual reading of the diff and full picker.go/picker_test.go. If you'd like these actually executed in a future run, go needs to be in the allowed-tools list for this action.

…tions

# Conflicts:
#	docs/superpowers/plans/2026-07-28-configurable-sections.md
@than
than merged commit f315b67 into main Jul 29, 2026
1 check passed
@than
than deleted the feat/configurable-sections branch July 29, 2026 16:21
@claude

claude Bot commented Jul 29, 2026

Copy link
Copy Markdown

Reviewed as a Go reviewer with the viewer invariants in mind. Overall: solid, well-scoped work. The scope claim in the description holds — ui.go, watcher.go, style.go, and render_test.go are untouched, so parent-directory watching, scroll preservation/clamping, the width-minus-2 render, and the hex-color rules are all unaffected. go vet is clean and all tests pass. Picker state handling is careful: the insert idiom in a uses copy correctly for the overlapping move, every bounds-sensitive key (space/e/d/J/K) is guarded for the empty-list case, and handling Ctrl+C inside the edit branch (rather than letting textinput swallow it) is exactly right.

One real problem and a few nits.

Re-running init writes the default five over a customized file's config

init.go:28 sets sections := defaultSections() and the already-exists branch (init.go:29-31) never updates it, so at init.go:48 offerClaudeHook gets the default five regardless of what is actually in the file.

The README calls out re-running as the supported upgrade path — "running sidecar init again upgrades an earlier setup in place." So the flow is: I run init, pick 🔥 Hot / 🧊 Cold, later re-run to pick up a new hook version, and now CLAUDE.md and the UserPromptSubmit reminder both instruct Claude to maintain 🧠 Needs action / 🚧 In progress / 🚘 Parked / ✅ Done / 📦 Shipped — sections that do not exist in my file. Claude will then dutifully create them, so the tooling actively undoes the customization the PR just added.

Before this PR the section list was hardcoded everywhere, so it was at least self-consistent; making sections configurable is what opens the gap. Cheapest fix is to recover the sections from the file that already exists rather than assuming defaults:

if _, err := os.Stat(abs); err == nil {
    fmt.Printf("%s already exists — leaving it untouched.\n", target)
    if existing := parseSections(abs); len(existing) > 0 {
        sections = existing
    }
}

where parseSections scans for heading lines with the "## " prefix and splits off a leading emoji (hints are unrecoverable, which is fine — claudeNote and renderTemplate already handle Hint == ""). That also naturally covers files the user hand-edited after init. If that feels like too much for this PR, the fallback is to omit the section list from the note and reconcile message on the already-exists path, so the hook at least does not assert something false.

esc cancel in the help text does not cancel

picker.go:212 advertises esc cancel, but esc sets canceled, result() returns nil, and pickSections (picker.go:249-251) falls back to the input sections — so init writes the default-five file anyway, silently. Same for deselecting everything and pressing Enter. The fallback itself is documented and defensible (never a zero-section file), but the label sells it as an abort, and Ctrl+C right next to it does abort with a message. Either relabel to something like esc keep defaults, or print a line on the fallback path so the user knows their input was discarded.

Nits

  • Duplicate names are not rejected. Adding a second "Done" emits two ## ✅ Done headers in the template and lists it twice in the note, giving Claude two equally valid targets for the same state. A dedupe on label() in result() would cover it.
  • A name or hint containing a comment terminator breaks the template. sections.go:47-58 interpolates user text straight into the <!-- ... --> block, so an embedded terminator closes the comment early and leaks the remaining agent instructions into the rendered pane. Unlikely, but neutering the sequence on the way in is a one-liner.
  • ti.Focus() in beginEdit (picker.go:123) discards the returned tea.Cmd, so the text cursor never blinks. Purely cosmetic; beginEdit would need to return (picker, tea.Cmd) to fix, which may not be worth the churn.
  • readChoice immediately before pickSections in offerCreate (init.go:324-330): the bufio.Scanner reads and drops a buffered chunk of stdin, so type-ahead is lost before the picker starts. Pre-existing pattern, just newly adjacent to a TUI.

Test coverage for the picker is genuinely good — driving the model with synthetic key messages gets real behavioral coverage on toggle/reorder/bounds/add/edit/delete/cancel without needing a TTY.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Configurable sections (custom names/emoji/order) + interactive TUI

1 participant