Repository navigation
feat(cli): link a prefilled GitHub issue from voltage feedback and panics - #130
Conversation
tee8z
left a comment
There was a problem hiding this comment.
Only one finding. Scrubbing, the caps, and outline are sound and well tested. outline reads the command tree rather than a parse, so a line that does not parse still outlines, and no positional or flag value survives. The trim loop always ends, because the title is capped well under 6,000 bytes. The panic hook keeps a dependency's absolute path down to its file name.
Finding: file_feedback uses two tests where the description uses one. The report is marked as a person's only when out.on_terminal() && terminal.can_prompt(), but the links are shown whenever out.on_terminal(). Two cases come out wrong:
| On a terminal | Description and Try it | What the code does |
|---|---|---|
voltage feedback --no-input |
Report fields, "reporter": "agent" |
Prints the two › links (agent-marked body inside) |
stdin piped, as echo | voltage feedback |
— | A person sees the links, but the issue form opens with An AI agent wrote this report already checked |
feedback_on_a_terminal_prints_the_search_and_issue_links asserts the second case as intended, since its stdin is not a terminal.
Fix: decide once (suggestion inline). Then the PTY test needs stdin on the terminal and --no-browser, and a test for --no-input on a terminal should show the report, not the links. On a terminal the report is rendered as the human card, not JSON, so the Try it line for voltage feedback --no-input should say "the report fields" rather than JSON.
Non-blocking: when the trim loop runs, it cuts the end of the body, which is where the Checks boxes are, including the agent marker the description calls "the reliable marker". Today only multibyte-heavy input reaches the loop. Trimming the problem text instead of the tail would always keep the boxes.
CRAP: not run on this head yet; I will add the result when it finishes. CI is green.
|
Also took the non-blocking point in cfccaff. When a report is too long for a link, the loop now trims the text before |
…nics `voltage feedback` prints a new-issue link with the build, platform, and the misbehaving command's words and flag names (never values), plus a link to search existing issues. A person at a terminal gets it opened in a browser; an agent or script never does, gets the report as JSON for `gh issue create`, and its report is marked agent-written. A panic hook prints the same link after the panic message, with the source line and a message scrubbed of IDs, keys, tokens, invoices, addresses, emails, and URLs. Every part is capped so the URL stays under 6,000 bytes. A bug report template gives manual reports the same sections.
df084db to
c94b96e
Compare
tee8z
left a comment
There was a problem hiding this comment.
Reviewed c94b96e against #126 (49590cc). No blocking findings in this layer.
The report path runs without loading credentials/configuration; command outlines remove argument values, report text is scrubbed, and the encoded link is capped. Browser suppression and terminal versus agent reports have process/PTY coverage.
Local gate passed: formatting, Clippy with warnings denied, dependency hygiene, full tests (220 unit + 159 integration), and contract/docs generation checks. Required CRAP check passed: 908/1132 functions measured, 224 unmeasured, no scores >=20; highest 18.10 (wait_for_payment).
Limits: no real browser/GitHub-form interaction or end-to-end panic injection; inspected the panic hook and tested its report-building helpers. Approval applies to this incremental stack layer.
Fixes DEV-2955
Problem
When a person or an agent hit a CLI bug, there was no short path to a useful GitHub issue. Reports had to be written by hand, with no build or platform, and could easily carry IDs or keys copied from a failing command.
Behavior
voltage feedback [--title TEXT] [--no-browser] [-- COMMAND...]prints a link to a prefilled new issue, and a link to search existing issues first.--no-browser› Search existing issues first: URLand› File the issue: URL(orOpened in your browser: URL)--no-input,--json, or--output-filetitle,labels,reporter,body,url,search_url,opened[x] An AI agent wrote this report)--is reduced tovoltage, its subcommand words, and flag names, so-- wallets get WALLET --profile prod --jsonbecomesvoltage wallets get --profile --json. No positional or flag value is kept.voltage --versionoutput, OS and architecture, command), and Checks: a search-first box and the agent box. A hidden comment asks agents to search first, keep the agent box checked, and give steps a person can run.After a panic, the default panic message is followed by
This is a bug in voltage. Report it: URL, with the sourcefile:lineand a scrubbed panic message. A dependency's absolute source path keeps only its file name, so the builder's home directory is not included.Scrubbing and caps. Report text passes through
scrub_line(control and bidi characters), and then any word that contains@or://, is 32 or more characters, or is 16 or more characters with a digit becomes[REDACTED](the existing redaction marker). That covers UUIDs, API keys, tokens, invoices, addresses, emails, and URLs. The title is capped at 120 characters, the command at 300, and the panic message at 500. If the encoded URL is still more than 6,000 bytes, the body is trimmed until it fits.Template.
.github/ISSUE_TEMPLATE/bug_report.mdgives manually filed reports the same sections and thebuglabel. Blank issues stay enabled, because the prefilled link usestitle/body/labelsparameters.Design notes
feedbackis a new module (prefilled reports, scrubbing, and the panic hook).cli::outlinewalks the clap command tree without parsing, so it also works on a line that does not parse.startup::runinstalls the hook, andexecutesendsfeedbacktofile_feedbackbefore the configuration is read.labels=from a URL only for users with triage access, so the label is best effort.urlandwebbrowserare already used).output::REDACTEDbecomespub(crate)so the scrubber reuses the marker.Try it
From the repository root, on this branch, with
.envin place:Expect two
›lines: a search link, and aFile the issue:link. Open the second link. The form shows the title, the body with your version and platform,Command: voltage wallets get --account --json(no UUID, nome), and an unchecked agent box.Expect the browser to open the same form.
voltage feedback --json -- payments send lnbc1example --execute | cat voltage feedback --no-inputThe first prints JSON with
"reporter": "agent","opened": false, and a body with[x] An AI agent wrote this reportandCommand: voltage payments send --execute. The second, on a terminal, shows the report fields as a card, withreporteragentand no›links. No browser opens for either command.The panic link can't be triggered on a normal build. The unit tests cover it, and I checked it by hand with a temporary
panic!that echoed a wallet UUID. The link came out withsrc/startup.rs:LINE, the UUID as[REDACTED], and exit code 101.Checks
docs/guides/troubleshooting.md(new Report a problem),docs/guides/scripting.md,docs/README.md,README.md,docs/quality.md(module ownership), the regenerateddocs/commands.md, andCHANGELOG.md(Unreleased).master(CHANGELOG conflict: kept both lines). The verification gate passes on the head:cargo fmt --check,cargo clippy --locked --all-targets -- -D warnings,cargo machete --with-metadata,cargo test --locked(209 unit, 151 integration), andpython3 scripts/check-coverage.py.masterwithvoltage-pr-review. Tests cover the new paths: an empty title, no command, an agent versus a person,--no-browser,--, negative values, attached short-flag values, a line that does not parse, the length cap, and the trimming loop.scripts/crap-report.sh, cargo-llvm-cov 0.8.5, knots 1.16.0, on the head): 876 of 1089 measured, 0 violations. The highest score is 18.10 (api.rswait_for_payment, baseline). New functions:cli::outline6,feedback::push_word6,startup::file_feedback4.0 (now fully covered), and the rest 1 to 4.startup::executeis 16.file_feedbackis now covered byfeedback_on_a_terminal_prints_the_search_and_issue_links(a487e01), which runsvoltage feedbackwith stdout and stderr on a pseudo-terminal and checks the two›links, their order, and that the wallet ID is absent.Stack
Stack #135, layer 7 of 9 (bottom targets
master):--allonorganizations,environments, andprofiles listVOLTAGE_*variablevoltage context --format envvoltage feedbackand panicsvoltage environments createflake.nixwith a KaifileThe layers are chained with merge commits, because history is not rewritten. Use squash and merge, bottom first.