AIT-458: feedback tool for agent-observed friction - #68
Conversation
|
Warning Review limit reachedNext included review available in 1 minute. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Your 86 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Comment |
Separate command rather than a flag on `support new`: agents route on descriptions, and nothing in a support description fires when the human is merely confused. The description carries the negative clause (broken thing or waiting human means open a ticket instead). Governed by the existing telemetry switch, so there is no second consent surface to explain: telemetry off means nothing leaves the machine.
Feedback rides the telemetry switch, so the disclosure has to say so. Bumps the disclosure version to 3 so existing installs see the changed text once.
- The disclosure claimed no command arguments are ever sent, two lines above a new line about a command whose whole argument is sent. Say the exception. - The disclosure only printed from the Sentry init path, so a CLI built without a DSN would upload the message having never shown it. Print it at the moment the data actually leaves. - Bare `hookmyapp feedback` blocked forever on stdin — the command has no required option, so that is the likely typo. Error on a TTY instead. - Validate --surface and the telemetry switch before reading stdin, so a typo reports itself instead of buffering a pipe. - Guard the optional note, matching how printDetail already treats it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f87965867b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // This is the moment data leaves the machine, so it is the moment the | ||
| // disclosure has to have been shown — it cannot depend on Sentry having | ||
| // initialized (no DSN in a local or self-built CLI means no banner). | ||
| maybePrintFirstRunDisclosure(); |
There was a problem hiding this comment.
When the config directory is read-only or otherwise unwritable and the v3 disclosure has not yet been persisted, this call prints the disclosure and then throws from writeConfig, so /support/feedback is never called. This regresses the CLI's documented fail-open handling for disclosure writes in sandboxed/read-only environments; catch persistence failures here, as initSentryLazy() does, so an inability to save the one-time flag does not prevent the report.
Useful? React with 👍 / 👎.
| ' No command arguments, file contents, or env var values are sent — except the', | ||
| ' message you pass to `hookmyapp feedback`, which is sent on purpose.', |
There was a problem hiding this comment.
The disclosure says the feedback message is the only command argument sent, but the request also transmits the user-selected --surface value in src/commands/support.ts:406. Even though it is restricted to a small enum, this makes the consent text factually inaccurate whenever the user selects a surface; describe both the message and surface metadata as transmitted.
Useful? React with 👍 / 👎.
- --surface defaulted to 'cli' and was always sent, so friction the human hit in the docs or the dashboard was recorded as CLI friction. Omit when unknown. - maybePrintFirstRunDisclosure() throws on a read-only config dir, which would have blocked the submission entirely. Fail open, as the Sentry call site does. - The disclosure named only the message; --surface is uploaded too. Say both. - The help said 'never a transcript', which does not exclude tokens, PII, or customer message content. Carry the same prohibition support new has.
Draft WIP for AIT-458 —
submit_feedbackMCP tool +hookmyapp feedbackCLI riding on the existing support/Chatwoot plumbing.Refs AIT-458