Skip to content

feat(agent): append the feedback survey invite to surveyed recipes - #345

Merged
thecodedrift merged 4 commits into
survey/03-feedback-recipesfrom
survey/04-survey-gate
Sep 18, 2026
Merged

thecodedrift merged 4 commits into
survey/03-feedback-recipesfrom
survey/04-survey-gate

Conversation

@thecodedrift

@thecodedrift thecodedrift commented Sep 18, 2026

Copy link
Copy Markdown
Member

Stack (root → tip):

Slice 4 of 4 of qualitative-survey-feedback (stacked on #344). The switch-on: with this, agent onboard / create-sg-rule / create-vale-rule / create-remote-rule and taskless onboard start carrying the invite.

What

  • src/survey/invite.ts: surveyGateIsOpen (telemetry on, CI not true/1, topic surveyed, next_ask absent/corrupt/passed) and withSurveyInvite, which on an open gate captures survey shown { $survey_id }, writes next_ask 10 days out, and appends feedback-invite rendered header: false after the recipe's last section. Closed gate returns the recipe byte-identical.
  • agent.ts and onboard.ts both call it after getRecipe(); onboard --mark-complete and the already-onboarded early return do not. The prompts render path is untouched, so the agent/prompts byte-parity test passes as-is (the suite runs with the opt-out set, which closes the gate).
  • isCiEnvironment() in util/interactive.ts is now the one reading of CI; shouldLaunchWizard uses it too.
  • The two existing telemetry mocks gain isTelemetryEnabled: () => false so they keep measuring cli_agent alone.
  • Archived on this tip (3b4d737) after the end-to-end verification 4.6 asked for: a dev build of this branch fired cli_agentsurvey shown on the first fetch, served the bare recipe on the second fetch inside the window, and survey sent arrived in PostHog with all five questions resolved to their text (completed recognised as the single-choice Yes). Scenario retention checked per CLAUDE.md: analytics 57→58, cli 72→73, cli-taskless-bootstrap 26→29, plus the new cli-feedback-survey spec with 19. The test response (uuid 01a0b255-83c6-7f57-8bb9-215aef81a256, opens with LOCAL VERIFICATION 4.6) is still live and should be archived from the survey's responses view.

Tests

survey-invite.test.ts (17): open gate appends exactly the fragment with one # Topic: line, captures survey shown, writes the cadence; each of the four topics is surveyed; within-window, opt-out (cadence not even read), CI=true/1, unsurveyed topic, and corrupt-file repair; then through the commands: agent create-sg-rule prints the invite after cli_agent, agent check does not, taskless onboard does, --mark-complete and an already-onboarded project do not.

Refs #341

@github-actions github-actions Bot added the Open OpenSpec Contains unresolved OpenSpec changes. All openspec changes must eventually reach an archive state. label Sep 18, 2026
@github-actions github-actions Bot removed the Open OpenSpec Contains unresolved OpenSpec changes. All openspec changes must eventually reach an archive state. label Sep 18, 2026
@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 6m 52s —— View job


Review complete

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contained no comments, reviews, or review threads for this PR (only the /review trigger comment itself), so this is the first review of PR #345. Everything below is [New].

  • Read .prior-review.json
  • Understand the request (/review)
  • Fetch PR diff and metadata (gh pr diff 345)
  • Read changed files in full context (agent.ts, onboard.ts, invite.ts, cadence.ts, constants.ts, interactive.ts, telemetry.ts, recipes.ts, the new/changed test files, and the OpenSpec deltas)
  • Run finder angles (correctness, cross-file, removed-behavior, cleanup, altitude, conventions)
  • Verify candidates
  • Post inline comments
  • Post final summary

Assessment

This is a tight, well-scoped slice with strong test coverage (17 new tests in survey-invite.test.ts covering every gate branch, both serving paths, and the corrupt-cadence-repair case). I traced the gate logic (surveyGateIsOpen/withSurveyInvite in packages/cli/src/survey/invite.ts), both call sites (agent.ts, onboard.ts), the cadence store (cadence.ts), and the shared isCiEnvironment extraction in util/interactive.ts, and checked for regressions against prompts.test.ts byte-parity, the vitest global telemetry opt-out (vite.config.ts sets DO_NOT_TRACK/TASKLESS_TELEMETRY_DISABLED for the whole suite, which is why the execFile-spawned integration tests in onboard.test.ts never hit the gate), and the OpenSpec archive (the MODIFIED Requirements deltas fully restate every prior scenario, so nothing was silently dropped per the CLAUDE.md archive-loss warning).

One finding, posted inline on packages/cli/src/survey/invite.ts:

  • [New] surveyGateIsOpen reads next_ask well before withSurveyInvite writes the new value (separated by a getRecipe call and a getTelemetry await). Since each CLI invocation is its own process, two concurrent surveyed-topic invocations (two agent sessions, an editor + terminal) can both read the same stale cadence, both see the gate open, and both fire survey shown / append the invite before either write lands. Low severity — no crash or data loss, just a possible duplicate invite/event against the intended 10-day cadence — and there's no test for the concurrent case. Flagging for a decision on whether it's an accepted tradeoff.

Nothing else survived verification: the telemetry-mock additions (isTelemetryEnabled: () => false in the two agent-*-telemetry.test.ts files) are necessary and correct — without them, the mocked ../src/telemetry module would lack that export and invite.ts would throw when routing through a surveyed topic like create-sg-rule. The isCiEnvironment extraction in util/interactive.ts is behavior-preserving. The two serving paths (agent.ts, onboard.ts) correctly share one cadence file per survey (not per topic), matching the spec's "one epoch timestamp per survey" requirement, and both correctly skip the gate on --mark-complete / the already-onboarded early return.
· branch: survey/04-survey-gate

Comment thread packages/cli/src/survey/invite.ts Outdated
@thecodedrift

Copy link
Copy Markdown
Member Author

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

Read. The one finding (read-then-write window on next_ask) is being addressed on this branch as an accepted tradeoff with the window narrowed; the inline thread will carry the reasoning and the commit.

— AI Coding Agent

withSurveyInvite wraps the text agent <topic> and onboard print. The
gate is telemetry on, CI not true/1, a surveyed topic, and next_ask
passed; when open it captures survey shown with $survey_id, writes
next_ask 10 days out, and appends feedback-invite rendered header-less
after the recipe's last section. The prompts export never sees it, and
the parity test keeps passing because the suite runs with the opt-out
set. The one reading of CI moves to isCiEnvironment so the wizard and
the gate agree.
Verified end to end against the live survey from a dev build of this
tip: survey shown after cli_agent on the first fetch, a bare recipe on
the second fetch inside the window, and survey sent arriving with all
five questions resolved to their text. Every scenario present before
the archive is still present afterwards (analytics 57 to 58, cli 72 to
73, cli-taskless-bootstrap 26 to 29), plus the new cli-feedback-survey
capability with 19.
Nothing locks next_ask, so two CLI processes that read it in the same
instant can both see the gate open and both serve the invite. The
write used to land after getRecipe and a getTelemetry await; it now
lands the moment surveyGateIsOpen returns true, so the race is the
width of one read-then-write rather than a render and a client init.
Accepted tradeoff: the worst case is one duplicate invite and one
duplicate survey shown, which the funnel over-counts by design. A lock
file was rejected because it would need a TTL to survive a crashed
process. The new test has the mocked getTelemetry read the cadence
file when called and asserts it already holds the advanced value.
@thecodedrift
thecodedrift added this pull request to stack #346 September 18, 2026 17:58
@thecodedrift
thecodedrift merged commit d6664ad into main Sep 18, 2026
4 checks passed
@thecodedrift
thecodedrift deleted the survey/04-survey-gate 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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant