coop: fix request-changes feedback submission in terminals without key disambiguation - #1841
Open
tomelm wants to merge 25 commits into
Open
coop: fix request-changes feedback submission in terminals without key disambiguation#1841tomelm wants to merge 25 commits into
tomelm wants to merge 25 commits into
Conversation
## Summary This PR establishes the `coop` integration branch as the base for the co-op stacked PR series. Earlier, stripe#1674 was merged into `tomer/coop-stack-base`, but the active stack is now moving to the protected `coop` branch. Because `coop` already has newer release commits and cannot be pushed to directly, this PR reapplies the already-reviewed co-op domain model onto `coop` through the normal protected-branch PR flow. Once this merges, the rest of the stack can target `coop` cleanly. In particular, stripe#1675 should show only the session store files instead of re-showing the domain model files from stripe#1674. ## What this adds - Core co-op session domain types: sessions, steps, nodes, review state, and command responses. - SDK snippet request/response helpers used by later workflow and agent command PRs. - Focused domain tests for session transitions, review behavior, and snippets. ## Review notes This should be reviewed as the same domain-model foundation from stripe#1674, now applied to `coop`. There are no TUI, command registration, workflow service, or deployment changes in this PR; those remain in later stacked PRs. ## Verification - `go test ./pkg/coop -count=1` - `go test ./pkg/coop/... ./pkg/cmd/coop ./pkg/cmd -count=1` - `golangci-lint run --max-issues-per-linter=0 --max-same-issues=0 ./pkg/coop/... ./pkg/cmd/coop` Co-authored-by: codex <noreply@openai.com>
## What this adds Adds JSON session persistence, optimistic version checks, lock files, heartbeat files, session listing, latest-session lookup, deletion, and Windows-safe session replacement. ## Review focus Review this PR as one slice of the co-op stack. It should be understandable on its own against `tomer/coop-split-domain-core`. ## Stack Base: `tomer/coop-split-domain-core` Head: `tomer/coop-split-domain-store` ## Validation Ran `go test ./pkg/coop -count=1` and `golangci-lint run --max-issues-per-linter=0 --max-same-issues=0 ./pkg/coop/...`. Co-authored-by: codex <noreply@openai.com>
## What this adds Adds embedded blueprint loading, prefix matching, metadata listing, session creation from blueprints, and the minimal fixture catalog needed to validate loader behavior. ## Review focus Review this PR as one slice of the co-op stack. It should be understandable on its own against `tomer/coop-split-domain-store`. ## Stack Base: `tomer/coop-split-domain-store` Head: `tomer/coop-split-blueprint-core` ## Validation Ran `go test ./pkg/coop -count=1` and `golangci-lint run --max-issues-per-linter=0 --max-same-issues=0 ./pkg/coop/...`. --------- Co-authored-by: codex <noreply@openai.com>
## What this adds Adds the remaining bundled co-op integration blueprints as catalog data, keeping the loader/review mechanics separate from the larger JSON review. ## Review focus Review this PR as one slice of the co-op stack. It should be understandable on its own against `tomer/coop-split-blueprint-core`. ## Stack Base: `tomer/coop-split-blueprint-core` Head: `tomer/coop-split-blueprint-catalog` ## Validation Ran `go test ./pkg/coop -count=1` and `golangci-lint run --max-issues-per-linter=0 --max-same-issues=0 ./pkg/coop/...`. Co-authored-by: codex <noreply@openai.com>
## What this adds Adds the workflow service that applies agent lifecycle transitions, review/next-action helper logic, command responses, snippet lookup integration, and workflow tests. ## Review focus Review this PR as one slice of the co-op stack. It should be understandable on its own against `tomer/coop-split-blueprint-catalog`. ## Stack Base: `tomer/coop-split-blueprint-catalog` Head: `tomer/coop-split-workflow` ## Validation Ran `go test ./pkg/coop/... -count=1` and `golangci-lint run --max-issues-per-linter=0 --max-same-issues=0 ./pkg/coop/...`. --------- Co-authored-by: codex <noreply@openai.com>
Part 6 of the clean co-op stack targeting `coop`. This PR adds the Cobra command surface for co-op mode without introducing the TUI yet. Changes: - Adds `stripe coop` command wiring. - Adds agent-facing commands for reporting work, checks, review state, status, stop, run, and recommendations. - Adds command tests for the new co-op command package. Review notes: - This should be reviewed first in the remaining stack. - The next PR layers in Charm dependencies and prompt helpers. --------- Co-authored-by: codex <noreply@openai.com>
Part 7 of the clean co-op stack targeting `coop`. This PR adds the prompt helper layer and the initial Charm dependency changes needed by later UI work. Changes: - Adds co-op prompt helper code and tests. - Updates Go module dependencies for the prompt layer. Review notes: - Builds on the co-op command surface from the previous PR. - The next PR adds the production TUI implementation. --------- Co-authored-by: codex <noreply@openai.com>
Part 8 of the clean co-op stack targeting `coop`. This PR adds the production Bubble Tea terminal UI for co-op sessions. Changes: - Adds the TUI app/model/view structure. - Adds layout, outline, detail, markdown, review, selection, mouse, and keymap handling. - Adds the remaining Charm ecosystem dependencies used by the UI. Review notes: - This PR is intentionally the largest remaining production UI layer. - Tests for this UI are isolated in the next PR. --------- Co-authored-by: codex <noreply@openai.com>
Part 9 of the clean co-op stack targeting `coop`. This PR adds focused test coverage for the co-op TUI introduced in the previous PR. Changes: - Adds model, layout, interaction, stress, and view tests. - Adds small test-support hooks in the TUI command/review code. Review notes: - This PR is primarily test coverage for the previous production TUI layer. --------- Co-authored-by: codex <noreply@openai.com>
Part 10 of the clean co-op stack targeting `coop`. This PR adds the co-op start/join launcher path and the hidden debug-agent harness used for local/manual validation. Changes: - Adds start, join, launcher, and debug-agent commands. - Adds launcher/debug-agent tests. - Adds debug-agent documentation and the tmux validation script. Review notes: - The debug agent is intentionally retained as an internal/testing aid. - This branch was adjusted to use the current step-scoped review helpers and session fixture shape. --------- Co-authored-by: codex <noreply@openai.com>
Part 11 of the clean co-op stack targeting `coop`. This PR adds deploy-oriented follow-up support for completed co-op sessions without introducing deployment as a normal integration blueprint. Changes: - Adds deploy and deploy-update next-action suggestions after a session completes. - Adds internal compiled guided actions for deploy follow-ups. These create normal parented co-op sessions, so the TUI can show step-by-step deploy progress in the left rail, but they are not embedded blueprint JSON, are not returned by `ListBlueprints`, and are not started through `stripe coop run`. - Keeps the generic guided-action-to-session machinery in `pkg/coop`, while deploy-specific guided action definitions live in `pkg/coop/followups`. - Adds `stripe coop agent start-followup` for agents to create those guided follow-up sessions from a completed parent session. - Separates deploy-update from deploy: existing deployment infrastructure gets a guided flow that pushes the integration changes to the detected target. - Updates the TUI so deploy choices wait for the guided follow-up session; add-integration still waits for a new integration session. - Adds/updates tests for guided action definitions, command creation, next-action responses, TUI waiting behavior, and parent-session routing. Review notes: - The earlier `deploy-stripe-projects` blueprint was removed in response to review feedback. Deploy is modeled as an internal follow-up action with session steps, not as blueprint source of truth. - The deploy guided action tells the agent to use Stripe Projects plugin detection and commands as the deployment source of truth. - The deploy-update guided action tells the agent to use the existing detected provider path. - The deploy guidance tells the agent not to supplement or modify co-op blueprints as deployment source of truth and not to print secret key material. --------- Co-authored-by: codex <noreply@openai.com>
Committed-By-Agent: codex Co-authored-by: codex <noreply@openai.com> # Conflicts: # go.mod # go.sum
Part 12 of the clean co-op stack targeting `coop`. This PR registers the co-op command in the root command surface and adds supporting documentation/export tooling. Changes: - Registers the co-op command with the root CLI. - Adds co-op README documentation. - Adds blueprint export script documentation and package metadata. - Updates root/log tests for the new command surface. Review notes: - This is the final PR in the clean stack. --------- Co-authored-by: codex <noreply@openai.com>
## Summary
- Validate `${node.<step>.<node>...}` references when a blueprint is
loaded (`LoadBlueprint`), so malformed or unknown node references fail
fast instead of surfacing as confusing errors mid-session.
- Collect valid `step.node` (and `step.node.request`) keys, then check
every `${node...}` reference in the marshaled blueprint against them.
- Allow integer-indexed reference segments (e.g. an indexed
test-request) as valid.
## Tests
- `TestValidateBlueprintReferences` — covers valid references, malformed
references, and unknown referenced nodes.
- `go test ./pkg/coop/... ./pkg/cmd/coop/...` — all green.
## Base
Based directly on `coop`. First of the co-op product-quality follow-ups;
`agent-quality` and `blueprint-schema` build on top and will be opened
as `coop` advances.
…ping (stripe#1773) **Stack 1/3** (base: `coop`). The two release-blocking findings from the co-op mode review. ### 1. Shell-quoting in the tmux launcher (security) The launcher built a `bash -c` script by interpolating paths with `strconv.Quote`, which produces a **Go** string literal, not shell quoting. Inside bash double quotes, `$(...)` and backticks still execute — so an env-controlled path (`TMPDIR`, `XDG_CONFIG_HOME`) or a hostile `--agent` value could inject commands. Replaced with a POSIX `shellQuote` helper (single-quote + `'\''` escaping) at all interpolation sites; kept `strconv.Itoa` for numeric args. ### 2. Wide-terminal layout wrapping (UX) In the split workspace (width ≥ 100) the outline's divider rules and titles were sized to the full terminal width (up to 80) and wrapped into a broken multi-line mess inside the ~43-col left column — visible on any normal wide terminal. Added an `outlineWidthOverride` so the outline renders to the left-column width; the right detail panel keeps full width. Dropped a third reported item (completion-header "tab leak") after verifying it's a `tmux capture-pane` artifact of bubbletea's cursor-movement optimization, not a defect — the real `View()` output is tab-free at every stage. **Tests:** unit test for `shellQuote` metacharacter neutralization; regression test that the split outline fits the left column; updated an existing debug-agent test that had encoded the old unsafe double-quoting. Full race suite + golangci-lint clean; debug-agent tmux smoke test 16/16. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
**Stack 2/3** (base: `coop-fixes-high`). Eight medium-severity findings from the co-op mode review. - **session:** `IsComplete()` returns false for a session with no nodes, so a fresh/malformed empty session isn't shown as "Integration complete." - **tui:** gate the completion view to suggestion nav / select / quit / claim-open; work-view keys (expand, collapse, confirm, follow, reject, copy) no longer leak in to mutate hidden state or fire stray commands against the completion cursor. - **tui:** `g`/`G`/`Home`/`End` now move the outline selection to the first/last item (and scroll to it) instead of being no-ops when the outline fits on screen (they only scrolled the viewport). - **cmd/coop:** agent commands emit a structured `ok:false` JSON response with a hint on infra errors (e.g. heartbeat/store write failures mid-await) instead of plain text, so an agent parsing stdout always gets a recoverable response. - **cmd/coop:** surface the specific blueprint error (e.g. an ambiguous prefix + its candidate list) from `coop run`/`coop start` instead of a generic "not found." - **store:** `LatestSession()` skips unreadable/corrupt files so one bad file doesn't mask valid sessions from `status`/`join` (matches `LatestActiveSession`). - **cmd/coop:** create the store *before* launching the tmux agent pane, so a store failure can't orphan the pane and leave a dangling active session. - **workflow:** remove the unreachable single-node await path (`awaitNodeReview` + now-dead `rejectedResponse`); review always waits at step granularity. **Tests:** regression tests for empty-session completion, completion-view key gating, top/bottom selection movement, and corrupt-file resilience; verified ambiguous-prefix output via the built CLI. Full race suite + golangci-lint clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## Summary While doing post-merge validation of `coop`, the outside-tmux `coop start --debug-agent` path failed when `XDG_CONFIG_HOME` was set. The debug-agent pane inherited the custom config dir, but the TUI `coop join` command in the newly-created tmux session did not, so it looked in the default config dir and could not find the session. This updates tmux command construction so both the TUI pane and agent/debug-agent pane preserve the co-op config environment. ## Validation - `env -u CODEX_CI -u CODEX_THREAD_ID -u CODEX_SANDBOX -u CODEX_SANDBOX_NETWORK_DISABLED go test ./... -count=1` - `golangci-lint run ./...` - `go test -race ./pkg/coop/... ./pkg/cmd/coop -count=1` - `scripts/test-coop-debug-agent-tmux.sh` - Manual outside-tmux smoke with custom `XDG_CONFIG_HOME`, wide terminal size, debug agent review/confirm flow, and completion view --------- Co-authored-by: codex <noreply@openai.com>
Committed-By-Agent: codex Co-authored-by: codex <noreply@openai.com>
**Stack 3/3** (base: `coop-fixes-medium`). Low-severity polish and hardening from the co-op mode review. - **store:** fsync the temp file before rename and fsync the parent directory after, so a crash can't leave a renamed-but-truncated session file (atomic rename gives name-swap atomicity, not data durability). - **store:** reclaim a stale lock left by a crashed writer. Lock files now record pid+time; a lock untouched longer than 30s (healthy writers hold it for milliseconds) is removed and retried, instead of wedging the session until manual deletion. The 30s threshold stays well above the 5s lock timeout, so active-writer contention still times out as before. - **tui:** truncate the header claim URL with `ansi.Truncate` (width-aware) instead of byte-slicing, which could split a multibyte rune. - **helpers:** `filterCompletedSuggestions` allocates a fresh slice instead of reusing the caller's backing array via `suggestions[:0]`. - **workflow:** remove the dead `SelectNextAction` (no callers) and its now-orphaned `contains` helper; they encoded a next-steps write policy that diverged from the live lock-held `Update` path in the TUI. - **test:** the debug-agent tmux smoke test now asserts the wide split workspace has no wrapped divider rules (previously it only checked text presence, so it passed even with the wide-layout bug). Intentionally did **not** touch the bundled blueprint JSON (two blueprints use a single coarse review step) — per the README those are exported from the upstream Workbench source and shouldn't be hand-edited here. **Tests:** full race suite + golangci-lint clean; debug-agent tmux smoke test 16/16 including the new divider assertion. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Committed-By-Agent: claude # Conflicts: # pkg/cmd/root.go
## Summary - frame blueprint nodes as app implementation work rather than CLI-only tasks - add concrete guidance for SDK calls, signed webhooks, UI wiring, secure configuration, and safe test payment methods - require honest app-level verification evidence while accounting for unsupported event triggers and blocked environments This changes agent instructions only. It preserves the existing `nodes` response and does not change co-op commands, blueprint schemas, or session schemas. ## Testing - `go test ./pkg/cmd/coop ./pkg/coop/...` - `go test ./... -count=1` Co-authored-by: tomer-stripe <42354557+tomer-stripe@users.noreply.github.com>
## Summary - make Enter insert a newline and Ctrl/Cmd+Enter submit feedback - preserve bracketed paste and burst input without truncating long messages - keep feedback editable after store errors and update visible key hints ## Tests - `go test ./pkg/coop/... -count=1` - `go test -race ./pkg/coop/tui -count=1` - `go vet ./pkg/coop/tui` --------- Co-authored-by: tomer-stripe <42354557+tomer-stripe@users.noreply.github.com>
The request-changes editor bound submit exclusively to ctrl+enter and super+enter. Only terminals implementing the Kitty keyboard protocol can report those chords; Terminal.app, stock iTerm2 and plain tmux send a bare carriage return instead, indistinguishable from enter. The binding never matched there, the key fell through to the textarea, and every submit attempt inserted a newline — feedback could not be sent at all. Swap the roles so enter submits, and bind newline to ctrl+j in addition to the chords. ctrl+j is the LF control byte, which every terminal delivers, so multi-line feedback stays possible where ctrl+enter cannot be reported. The existing tests passed because they synthesize KeyPressMsg values with ModCtrl directly, which no terminal produces. Add coverage that exercises the keys a terminal can actually send.
ctrl+j always works but ctrl+enter is the more discoverable chord, and which one is available depends on the terminal rather than on anything the CLI can assume. Bubble Tea already queries the Kitty keyboard protocol at startup and replies with a KeyboardEnhancementsMsg only when the terminal supports key disambiguation, so use that signal instead of matching on terminal names: TERM_PROGRAM would misreport both iTerm2 with CSI u enabled and any terminal reached through tmux or SSH. Terminals reporting disambiguation see "ctrl+enter newline"; everything else keeps "ctrl+j newline". Every chord stays bound in both cases, so an incorrect detection only changes the hint, never the behavior. Fold the keyboard, focus and blur cases into one handler for terminal state. They are the same kind of message and it keeps Update under the gocyclo limit.
kwu-stripe
approved these changes
Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Request changes feedback could not be submitted at all in Terminal.app, stock iTerm2, or plain tmux.
#1825 bound submit exclusively to
ctrl+enterandsuper+enter. Only terminals implementing the Kitty keyboard protocol can report those chords — everywhere else Ctrl+Enter arrives as a bare carriage return, indistinguishable from Enter. The binding never matched, the key fell through to the textarea, and every submit attempt just inserted a newline. Cmd+Enter never reached the program at all, since iTerm2 consumes it for fullscreen.Bubble Tea does request Kitty flag 1 (
keyboardEnhancementsFlagsalways sets it), so the original behavior works in Ghostty/kitty/WezTerm — likely why it looked correct when written.Changes
ctrl+j(the LF control byte, delivered by every terminal), withalt+enter,ctrl+enter, andshift+enteralso bound.KeyboardEnhancementsMsgonly when the terminal supports key disambiguation. Terminals that report it seectrl+enter newline; everything else seesctrl+j newline. This is capability detection rather than name matching —TERM_PROGRAMwould misreport both iTerm2 with CSI u enabled and any terminal reached through tmux or SSH. All chords stay bound in both cases, so a wrong detection changes only the hint, never the behavior.ShortHelpso it survives footer truncation at tiny widths.Why the existing tests missed it
They synthesize
tea.KeyPressMsg{Code: tea.KeyEnter, Mod: tea.ModCtrl}directly — a message no real terminal produces. The model layer was always correct; the delivery layer was the problem. The new tests exercise the keys a terminal can actually send, and were confirmed to fail against the old bindings on exactlyctrl+jandalt+enter.Tests
go test ./pkg/coop/... -count=1go test -race ./pkg/coop/tui -count=1go vet ./pkg/coop/...golangci-lint run pkg/coop/...— 0 issues🤖 Generated with Claude Code