feat(tui): /feedback slash command (consent-gated GitHub issue) - #2
Open
kazimali07 wants to merge 12 commits into
Open
feat(tui): /feedback slash command (consent-gated GitHub issue)#2kazimali07 wants to merge 12 commits into
kazimali07 wants to merge 12 commits into
Conversation
Adds the design for a `/feedback` TUI command that files a prefilled GitHub issue (consent-gated) capturing user feedback plus, on opt-in, scrubbed session errors and non-sensitive environment info. Incorporates external review (panel mounting, reset-survival of the errors buffer, best-effort scrubbing, encoded URL-length cap, per-platform browser open, LLOYAL_GPU env source). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ASMQkyinUcy441vh5QEzC
Five TDD tasks: state plumbing (errors buffer + env meta, reset-survival), pure feedback helpers (scrub/body/url with encoded URL cap), open_feedback command + per-platform browser open, FeedbackPanel flow, Composer wiring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ASMQkyinUcy441vh5QEzC
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ASMQkyinUcy441vh5QEzC
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ASMQkyinUcy441vh5QEzC
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ASMQkyinUcy441vh5QEzC
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ASMQkyinUcy441vh5QEzC
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ASMQkyinUcy441vh5QEzC
Addresses adversarial review findings before the feature ships to a public issue destination: - scrubError now redacts secret shapes (provider key prefixes, JWTs, KEY=VALUE env fragments, bearer/authorization) — a key echoed in an error no longer reaches the public issue. - URLs are collapsed to scheme://host, dropping path+query (ids/keys/terms) and preventing the path-basename step from mangling them. - Each error is scrubbed against its OWN captured query (ErrorRecord.query, stamped in the reducer) so an older query in a persisted error can't leak. - homeDir redaction is path-boundary only (no /home/jo vs /home/joanna mangle). - buildFeedbackBody enforces the encoded-URL cap INCLUDING the truncation note. - Windows browser-open uses PowerShell Start-Process with a single-quoted URL (cmd `start` mis-parses the `&` in every issue URL). - FeedbackPanel uses one message source so body and URL title agree. New tests: 7 feedback-smoke checks + 1 reducer check (query stamping). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ASMQkyinUcy441vh5QEzC
Second hardening pass after red-team execution of scrubError/buildFeedbackBody: - BLOCKER: strip userinfo credentials from ANY scheme://user:pass@host (https, ftp, ws, postgres, mongodb, redis, …) — userinfo lives before the first slash, so the previous origin-collapse leaked it. Connection-string passwords no longer reach the public issue. - BLOCKER: surrogate-safe truncation. Slicing on UTF-16 units could split an emoji and leave a lone surrogate, crashing encodeURIComponent and the whole /feedback command. feedbackTitle, scrubError's length bound, and the message loop now strip a trailing lone surrogate. - MAJOR: the message truncation marker (…(truncated), multibyte) is now accounted for in the cap check, so the encoded URL no longer exceeds 7000. - MAJOR: redact prefix-less secrets (UUID, long hex, long mixed base64), KEY:VALUE (colon) and password-after-colon, and whitespace-wrapped JWTs. - MAJOR: query redaction is now case-insensitive and whitespace-tolerant (backends echo queries re-cased/re-wrapped), guarded to length >= 4. Tests: +11 feedback-smoke checks (URL creds, prefix-less secrets, KEY:VALUE, query normalization, emoji-no-crash, cap-boundary band); synthetic-error fixtures updated to realistic wordy text the scrubber won't collapse. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ASMQkyinUcy441vh5QEzC
Closes the highest-value residual from the second red-team pass: short, human-typed passwords in basic-auth positions that the entropy rules miss. High-confidence patterns only (curl -u / --user, and user:pass@host) to avoid over-redacting ordinary colon text (times, ratios) — covered by a new over-redaction guard test. Remaining residuals (sub-threshold lone secrets in non-URL positions) are inherent best-effort limits, backstopped by user review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ASMQkyinUcy441vh5QEzC
…siduals Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ASMQkyinUcy441vh5QEzC
Security review found that the Windows browser-open interpolated the issue
URL into a PowerShell -Command string (`Start-Process '${url}'`), and the
comment's claim that the URL "can never contain a single quote" was wrong:
encodeURIComponent leaves ' ( ) * ! ~ raw. A quote sourced from an attached
error string (which can echo untrusted corpus/web content) could break out of
the literal. Impact was constrained (no space/;/$ survive encoding) but the
sink was real and the invariant false.
Defense in depth:
- buildIssueUrl now strict-encodes ' ( ) * ! ~ at the source (encodeComponentStrict),
so the URL is safe for any consumer; encodedLen mirrors it so the cap still
holds (verified: worst-case all-quote body stays <= 7000).
- main.ts passes the URL as a bound -FilePath argv element instead of
interpolating it into the -Command string; comment corrected.
Test: buildIssueUrl strict-encoding assertion (no raw quote/paren in URL).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011ASMQkyinUcy441vh5QEzC
There was a problem hiding this comment.
Pull request overview
Adds a consent-gated /feedback slash command to the reasoning.run TUI that opens a prefilled GitHub issue in the user’s browser, optionally attaching scrubbed session errors and a non-sensitive environment snapshot.
Changes:
- Introduces
/feedbackin the Composer via a Composer-localFeedbackPanelflow (message → error consent → open confirm → URL shown). - Adds pure helpers to scrub errors and build a URL-capped prefilled GitHub issue body/URL, plus smoke coverage.
- Extends TUI state to retain an errors ring buffer and environment metadata across query/preflight resets, and adds a new
open_feedbackcommand handled inmain.ts.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tui-ink/state.ts | Adds errors buffer + env meta to AppState and initializes them. |
| src/tui-ink/reducer.ts | Preserves errors/env across resets; appends errors on ui:error/boot:error; handles ui:env. |
| src/tui-ink/feedback.ts | New pure helpers for scrubbing errors and building a URL-capped issue body + URL. |
| src/tui-ink/events.ts | Adds ui:env event type carrying EnvMeta. |
| src/tui-ink/components/FeedbackPanel.tsx | New overlay panel implementing the 4-step /feedback flow and dispatching open_feedback. |
| src/tui-ink/components/Composer.tsx | Registers /feedback, mounts FeedbackPanel, and disables Composer input handlers while the panel is active. |
| src/tui-ink/commands.ts | Adds open_feedback to the command union. |
| src/tui-ink/__reducer-smoke.ts | Adds reducer smoke checks for error buffering, FIFO cap, query stamping, and reset preservation. |
| src/tui-ink/__feedback-smoke.ts | New smoke tests for scrubbing, URL building, strict encoding, and encoded-length cap behavior. |
| src/main.ts | Seeds ui:env and handles open_feedback by spawning a platform opener. |
| package.json | Wires the new feedback smoke test into npm run smoke. |
| docs/superpowers/specs/2026-06-22-feedback-slash-command-design.md | Documents the feature’s UX, privacy model, and architecture/spec details. |
| docs/superpowers/plans/2026-06-22-feedback-slash-command.md | Implementation plan for /feedback (tasks, constraints, test gates). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1034
to
+1046
| } else if (process.platform === "win32") { | ||
| // Not `cmd /c start "" <url>`: cmd.exe re-parses the URL and an | ||
| // unquoted `&` (every issue URL has them between query params) is | ||
| // read as a command separator. Use PowerShell Start-Process, but | ||
| // pass the URL as a SEPARATE argv element bound to -FilePath — do | ||
| // NOT interpolate it into the -Command string (a `'`/`)` etc. | ||
| // would break out of the literal; buildIssueUrl also strict- | ||
| // encodes those, so this is defense in depth). | ||
| child = cpSpawn( | ||
| "powershell.exe", | ||
| ["-NoProfile", "-NonInteractive", "-Command", "Start-Process", "-FilePath", url], | ||
| { stdio: "ignore", detached: true }, | ||
| ); |
| .replace(/\b[0-9a-fA-F]{32,}\b/g, '[redacted-hex]') | ||
| // Long mixed base64/base64url runs (require both a digit and a letter so | ||
| // ordinary SNAKE_CASE identifiers and words are not redacted). | ||
| .replace(/\b(?=[A-Za-z0-9_+/-]*[0-9])(?=[A-Za-z0-9_+/-]*[A-Za-z])[A-Za-z0-9_+/-]{40,}={0,2}/g, '[redacted-token]'); |
Comment on lines
+141
to
+144
| - **On opener failure**, the URL must stay visible (a transient `ui:error` toast is | ||
| not adequate for a long, copyable URL — #7). Emit a durable **scrollback** entry | ||
| (the `Static`/scrollback channel, not the toast) containing the full URL with a | ||
| "couldn't open browser — copy this URL" note. |
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
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.
What this adds
A
/feedbackslash command for the reasoning.run TUI. The user types feedback, optionally attaches the session's errors (consent-gated), and reasoning.run opens a prefilled GitHub issue in their browser. Nothing is transmitted until the user reviews the prefilled issue and clicks Submit themselves — no backend, no token, no telemetry.lloyal-ai/reasoning-run/issues/newURL (labelfeedback).LLOYAL_GPU, model/reranker basename, mode), and — only on opt-in — scrubbed session errors.node:child_process(per-platform).Design
A Composer-local
FeedbackPaneloverlay (mirrors the existing inline-editor pattern — no newuiPhase) runs a 3-step flow: message → error-consent → open-confirm. Pure helpers insrc/tui-ink/feedback.ts(scrubError/buildFeedbackBody/buildIssueUrl) compose the body+URL; a singleopen_feedbackcommand does the per-platform browser open inmain.ts. Anerrorsring buffer +envmeta were added toAppState(surviving thequery/preflight:startresets). Full design + plan underdocs/superpowers/.Privacy & security
The destination is a public issue, so scrubbing is best-effort + the consent/review model is the real guarantee.
scrubErrorredacts: provider API keys, JWTs,KEY=/KEY:env fragments, bearer/basic-auth credentials, URL/connection-string userinfo (any scheme), high-entropy tokens (UUID/hex/base64), the session query (case/whitespace-tolerant, per-error), home dir, and filesystem paths — all surrogate-safe and within an encoded-URL cap. Residual best-effort limits are documented in the spec.This was put through adversarial fuzzing (~7,000+ cases) and a focused security review; findings fixed in-branch include URL-credential leaks, an emoji-driven
encodeURIComponentcrash, an encoded-URL cap overflow, and a constrained PowerShell-injection sink on the Windows open path (URL is now strict-encoded at the source and passed as a bound-FilePatharg).Testing
src/tui-ink/__feedback-smoke.ts(27 checks) + reducer checks, wired intonpm run smoke.tsc --noEmit,npm run smoke,npm run buildall green./feedbacklives) without crashing.Note for reviewer
The
feedbacklabel should be created on the repo (Issues → Labels) or GitHub silently drops the?labels=feedbackparam — the issue still files either way.🤖 Generated with Claude Code