Skip to content

feat(feedback): add feedback send/dismiss, the survey constants, and the cadence store - #343

Merged
thecodedrift merged 2 commits into
survey/01-ignore-scratch-filesfrom
survey/02-feedback-command
Sep 18, 2026
Merged

thecodedrift merged 2 commits into
survey/01-ignore-scratch-filesfrom
survey/02-feedback-command

Conversation

@thecodedrift

@thecodedrift thecodedrift commented Sep 18, 2026

Copy link
Copy Markdown
Member

Stack (root → tip):

Slice 2 of 4 of qualitative-survey-feedback (stacked on #342). Reachable only by someone who types it: nothing serves an invite until slice 4.

What

  • src/survey/constants.ts: the survey id, the five { key, id, question } entries (Q3's id is the post-single-choice one, 6ebdfabb-…), COMPLETED_CHOICES in PostHog's casing, the four surveyed topics (used only by the gate), and the 10-day / 20-day intervals. The only place the identifiers live.
  • src/survey/cadence.ts: next_ask under <XDG config>/taskless/surveys/<survey id>/, a bare epoch-ms string. Absent or unparseable reads as "ask now"; writes are best-effort.
  • src/schemas/feedback.ts: human keys only (verbatim, goal, completed, optional workedWell / needsImprovement). A blank optional is rejected; an omitted one is unanswered.
  • src/commands/feedback.ts: dismiss captures survey dismissed { $survey_id }; send --from <file> validates, captures survey sent { $survey_id, $survey_response_<id>… } with unanswered optionals absent, and leaves the file in place (the recipe's clean-up step deletes it; /.tmp-* is ignored regardless). Both push next_ask out 20 days. Under the opt-out both print that nothing was sent and exit 0; send validates first so a bad payload is reported either way.
  • isTelemetryEnabled() exported from telemetry.ts rather than restating the two variables.
  • feedback joins SUBCOMMAND_NAMES and UNLISTED_COMMANDS: the invite is the only door, pending a general feedback channel.

Tests

  • survey-cadence.test.ts: constants pinned to the live survey; store round-trip, absent, corrupt-then-repaired, per-survey isolation (with XDG_CONFIG_HOME in a temp dir).
  • feedback-schema.test.ts: enum, missing required, blank optional, and that a smuggled $survey_* key is stripped.
  • feedback-command.test.ts: both verbs with telemetry mocked (exact event shapes, next_ask advance, opt-out, INVALID_INPUT naming the field, no .taskless/ bootstrap), plus the built CLI: absent from agent, --help lists both verbs, --json error envelope.

Refs #341

…the cadence store

The PostHog survey's id and question map live in src/survey/constants.ts
and nowhere else; the payload the agent writes uses human keys and
feedback send translates. survey sent carries $survey_id and one
$survey_response_<id> per answered question, survey dismissed carries
$survey_id, and neither adds a survey-specific property of its own.
next_ask lives under surveys/<survey id>/ in the XDG config directory as
a bare epoch; both verbs push it out 20 days. Under the telemetry
opt-out both say nothing was sent and exit 0. feedback is unlisted in
the agent index: the invite is the only door.
@thecodedrift
thecodedrift force-pushed the survey/02-feedback-command branch from 93c46ea to 10b54ee Compare September 18, 2026 02:25
@thecodedrift

Copy link
Copy Markdown
Member Author

@claude /review

@claude

claude Bot commented Sep 18, 2026

Copy link
Copy Markdown

Claude finished @thecodedrift's task in 5m 14s —— View job


Review complete

  • Read .prior-review.json (prior review data)
  • Fetch PR diff and metadata
  • Review changed files for correctness, security, performance, tests, clarity
  • Post inline comments for concrete issues
  • Post final summary

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contained no review threads and no prior review bodies, only the trigger comment (@claude /review) itself. This is therefore the first review of this PR; the whole diff was assessed fresh.

Summary

This is a clean, well-tested slice. I read every changed file (feedback.ts, schemas/feedback.ts, survey/cadence.ts, survey/constants.ts, the telemetry.ts/agent.ts/names.ts/index.ts wiring, and all three test files), plus the surrounding files the diff touches indirectly (telemetry.ts, telemetry-run.ts, index.ts's shutdown flow, types/errors.ts, and the equivalent rule create --from code path for comparison).

Things I specifically checked and found correct:

  • Opt-out ordering: feedback send validates the payload before checking isTelemetryEnabled(), exactly as described, and the tests ("validates before honoring the opt-out") confirm it.
  • PostHog contract: buildSurveyResponse emits exactly $survey_id plus one $survey_response_<id> per answered question, omitting unanswered optionals rather than sending empty strings — matches the schema's blank-vs-omitted distinction and is covered by tests.
  • Telemetry flush: captures aren't followed by an explicit shutdown() in feedback.ts, but src/index.ts's top-level finally already calls shutdownTelemetry() for every invocation (shared singleton), so nothing is dropped.
  • Cadence store: XDG path construction, corrupt/absent-read-as-undefined, truncate-to-ms round trip, and per-survey isolation all check out against auth/token.ts's getConfigDirectory().
  • --from handling (read/parse/validate/error) is a faithful copy of the existing rule create --from pattern, including the local fail() closure style already used in demo.ts and rules.ts.
  • Unlisted command wiring: feedback is correctly added to both SUBCOMMAND_NAMES (so the satisfies check in index.ts holds) and UNLISTED_COMMANDS (so it's excluded from the agent topic index), and the built-CLI test asserts the latter.
  • Import grouping, export type usage for types reachable from exported signatures, and the changeset's patch bump (correct under the repo's 0.y.z-stays-patch rule, and correctly grown on the existing bottom-of-stack changeset file) all follow the conventions in .conventions/STYLEGUIDE-CODE.md and root CLAUDE.md.

One finding, posted inline on packages/cli/src/commands/feedback.ts:174:

  • [New] resolveCommandName() (packages/cli/src/telemetry-run.ts) only special-cases rule to retain its subcommand in the cli_run/cli_error command property. feedback send and feedback dismiss both collapse to the bare command "feedback" there, even though they're semantically distinct (send can fail with INVALID_INPUT, dismiss essentially cannot). It doesn't affect the survey sent/survey dismissed events themselves — those already carry the right shape — but it's a gap in the generic per-command run/error telemetry that this PR is a natural candidate to close alongside rule's existing special-case, or to explicitly defer.

Everything else — cadence file corruption handling, the smuggled-$survey_*-key stripping test, the built-CLI --help/--json-envelope tests, and the agent.ts/names.ts wiring — looks correct and well covered.

Branch: survey/02-feedback-command

Comment thread packages/cli/src/commands/feedback.ts
@thecodedrift

Copy link
Copy Markdown
Member Author

Re: @claude[bot] — "Review complete … One finding, posted inline on packages/cli/src/commands/feedback.ts:174"
#343 (comment)

Read. The one finding (resolveCommandName should keep feedback's verb the way it keeps rule's) is being fixed on this branch; the inline thread will carry the commit.

— AI Coding Agent

resolveCommandName only kept the subcommand for rule, so feedback send
and feedback dismiss both reported as "feedback" even though they are
distinct actions: send can fail with INVALID_INPUT, dismiss cannot, and
a cli_error for "feedback" could not say which one it came from. The
special case is now a small declared set of verb-carrying commands
(rule, feedback) rather than a second if.
@thecodedrift
thecodedrift added this pull request to stack #346 September 18, 2026 17:58
@thecodedrift
thecodedrift merged commit d7b3f46 into main Sep 18, 2026
5 checks passed
@thecodedrift
thecodedrift deleted the survey/02-feedback-command branch September 18, 2026 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Open OpenSpec Contains unresolved OpenSpec changes. All openspec changes must eventually reach an archive state.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant