fix(self-driving): degrade instead of aborting when GitHub is declined - #1089
fix(self-driving): degrade instead of aborting when GitHub is declined#1089posthog[bot] wants to merge 3 commits into
Conversation
A declined GitHub connection was the wizard's biggest named abort. STEP 3 now records GitHub as a pending follow-up and continues the run instead of emitting `[ABORT] github connection declined`, so the user still gets sources enabled, scouts tuned, and a filled inbox. The abort case is kept as a safety net with a loosened matcher (tolerates stray punctuation around the contract string) and copy that names the user's choice and reads in the browser sync panel. Generated-By: PostHog Desktop Task-Id: b9aefec2-e4de-4f5f-bf3b-98fde420c756
🦔 ReviewHog reviewed this pull requestNothing worth raising this time, so here's a calming picture instead:
|
🧙 Wizard CIRun the Wizard CI and test your changes against wizard-workbench example apps by replying with a GitHub comment using one of the following commands: Test all apps:
Test all apps in a directory:
Test an individual app:
Show more apps
Results will be posted here when complete. |
|
ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
| GitHub connection is a valid outcome, not a failure. This supersedes any | ||
| "abort on decline" wording in the skill reference. Instead: tell the user | ||
| Self-driving will still enable sources, tune scouts, and fill their inbox, | ||
| but that turning a finding into a pull request needs GitHub, which they can | ||
| connect later from the integrations settings. Record GitHub as a pending | ||
| follow-up for STEP 7, mark this task completed, and continue with STEP 3b. | ||
| Every later step works without GitHub — STEP 5 simply cannot auto-connect | ||
| GitHub Issues, which becomes another follow-up. |
There was a problem hiding this comment.
Success screen offers an unavailable PR action
Why we think it's a valid issue
- Checked:
buildOutroDatafor self-driving, the success-outro render path, and whether any state channel could tell it that GitHub is unconnected. - Found: The defect is real and unconditional.
src/lib/programs/self-driving/index.ts:91declaresbuildOutroData: (_session, credentials) => {...}— the session parameter is underscore-prefixed and unused — and line 103 always emits`Kick off a PR when you like the proposed fix ($${PRICE_PER_PR_USD} flat)`innextSteps.items.OutroScreen.tsx:58-63renders every item verbatim. No branch exists. - Found: The state is newly reachable because of this PR. Before it, a declined GitHub connection produced
[ABORT] github connection declined→OutroKind.Error(linear.ts:143-158), so the success outro was only reached with GitHub connected. STEP 3 now tells the agent "do NOT abort ... mark this task completed, and continue", so the run reachesOutroKind.Successwith no GitHub integration. - Found: The PR's own docs confirm the action is unavailable in that state. The rewritten
ARCHITECTURE.md:133row says "findings still flow to the inbox; a finding becomes a PR once GitHub is connected", and the new abort copy indetect.tssays "findings need GitHub to become code fixes". - Found: The STEP 7 follow-up does not compensate. It instructs the agent to name the pending GitHub item in the written report (
reportFile: REPORT_FILE), which is a separate markdown artifact. ThenextStepslist on the terminal success screen is wizard-owned and deterministic, so no agent output can amend it. - Found: The suggested fix needs no new plumbing — the exact mechanism already exists.
[DASHBOARD_URL]/[NOTEBOOK_URL](signals.ts:37) are captured from agent output atagent-interface.ts:1609-1624, stored on the session (wizard-session.ts:361), and consumed conditionally inevents-audit/index.ts:73-84. A GitHub-pending marker would follow that established pattern, so this is not an abstraction invented for the review. - Impact: A user who declines GitHub now finishes on a success screen that tells them to kick off paid PRs from findings, an action their project cannot perform, with no hint that connecting GitHub is the missing step. That directly undercuts the PR's goal of a clean degraded path, and it is the one screen every degraded run ends on.
- Impact: Note for the author — the finding anchors at
prompt.ts:145-152, which is the cause (the degrade instruction), but the code to change issrc/lib/programs/self-driving/index.ts:91-111.
Issue description
The run now succeeds without GitHub. However, buildOutroData always tells the user to kick off a PR. That action cannot work until the user connects GitHub. The report follow-up does not correct the deterministic success screen.
Suggested fix
Store the GitHub connection result in session state. Use that state in buildOutroData. Show the PR action only when GitHub is connected. Otherwise, show the GitHub connection follow-up.
Prompt to fix with AI (copy-paste)
## Context
@src/lib/programs/self-driving/prompt.ts#L145-152
<issue_description>
The run now succeeds without GitHub. However, `buildOutroData` always tells the user to kick off a PR. That action cannot work until the user connects GitHub. The report follow-up does not correct the deterministic success screen.
</issue_description>
<issue_validation>
- **Checked:** `buildOutroData` for self-driving, the success-outro render path, and whether any state channel could tell it that GitHub is unconnected.
- **Found:** The defect is real and unconditional. `src/lib/programs/self-driving/index.ts:91` declares `buildOutroData: (_session, credentials) => {...}` — the session parameter is underscore-prefixed and unused — and line 103 always emits `` `Kick off a PR when you like the proposed fix ($${PRICE_PER_PR_USD} flat)` `` in `nextSteps.items`. `OutroScreen.tsx:58-63` renders every item verbatim. No branch exists.
- **Found:** The state is newly reachable **because of this PR**. Before it, a declined GitHub connection produced `[ABORT] github connection declined` → `OutroKind.Error` (`linear.ts:143-158`), so the success outro was only reached with GitHub connected. STEP 3 now tells the agent "do NOT abort ... mark this task completed, and continue", so the run reaches `OutroKind.Success` with no GitHub integration.
- **Found:** The PR's own docs confirm the action is unavailable in that state. The rewritten `ARCHITECTURE.md:133` row says "findings still flow to the inbox; **a finding becomes a PR once GitHub is connected**", and the new abort copy in `detect.ts` says "findings need GitHub to become code fixes".
- **Found:** The STEP 7 follow-up does not compensate. It instructs the agent to name the pending GitHub item in the written report (`reportFile: REPORT_FILE`), which is a separate markdown artifact. The `nextSteps` list on the terminal success screen is wizard-owned and deterministic, so no agent output can amend it.
- **Found:** The suggested fix needs no new plumbing — the exact mechanism already exists. `[DASHBOARD_URL]` / `[NOTEBOOK_URL]` (`signals.ts:37`) are captured from agent output at `agent-interface.ts:1609-1624`, stored on the session (`wizard-session.ts:361`), and consumed conditionally in `events-audit/index.ts:73-84`. A GitHub-pending marker would follow that established pattern, so this is not an abstraction invented for the review.
- **Impact:** A user who declines GitHub now finishes on a success screen that tells them to kick off paid PRs from findings, an action their project cannot perform, with no hint that connecting GitHub is the missing step. That directly undercuts the PR's goal of a clean degraded path, and it is the one screen every degraded run ends on.
- **Impact:** Note for the author — the finding anchors at `prompt.ts:145-152`, which is the cause (the degrade instruction), but the code to change is `src/lib/programs/self-driving/index.ts:91-111`.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Store the GitHub connection result in session state. Use that state in `buildOutroData`. Show the PR action only when GitHub is connected. Otherwise, show the GitHub connection follow-up.
</potential_solution>
There was a problem hiding this comment.
Confirmed this is a real defect newly introduced by this PR: the self-driving success screen (buildOutroData in index.ts) unconditionally lists 'Kick off a PR ($X flat)' and 'Cap the spend with a monthly PR limit'. Before this change a declined GitHub connection aborted, so that screen was only reached with GitHub connected; now STEP 3 continues to a success outro without GitHub, so a user who declined ends on a screen advertising a paid PR action their project can't perform yet. I did not fix it here because the safe, correct fix needs a human decision and a live end-to-end check, not an unattended edit: it requires threading a new GitHub-connection signal from the agent into session state (a new agent-output marker like [GITHUB_PENDING] following the existing [DASHBOARD_URL]/[NOTEBOOK_URL] pattern, a new WizardSession field, a new WizardUI setter, and parsing in both harness paths), then branching the outro to hide the PR steps and surface a 'connect GitHub' step instead. Two things make that unsafe to land unreviewed: (a) its correctness depends on the agent reliably emitting the new marker only in the declined branch — LLM/prompt behavior that can only be validated with an e2e run; and (b) there's a genuine design choice between that stateful approach and simply rewording the deterministic pricing copy to read correctly without state (which touches curated pricing copy and is a product call). A maintainer should pick the approach and validate the signal end-to-end. Note the code to change is src/lib/programs/self-driving/index.ts:91-111, not prompt.ts where the thread anchors.
| but that turning a finding into a pull request needs GitHub, which they can | ||
| connect later from the integrations settings. Record GitHub as a pending | ||
| follow-up for STEP 7, mark this task completed, and continue with STEP 3b. |
There was a problem hiding this comment.
Declined GitHub steps count as successful connections
Why we think it's a valid issue
- Checked: The full step-analytics path — the prompt instruction, the event emitter, the key resolver, the status vocabulary, and every other self-driving analytics call that could carry the GitHub outcome.
- Found: The mechanism is exact.
prompt.tsSTEP 3 now says on a decline: "Record GitHub as a pending follow-up for STEP 7, mark this task completed, and continue with STEP 3b."agent-interface.ts:1396-1424fires on any transition toin_progressorcompletedand capturesanalytics.wizardCapture('step', { step_name, step_key, status, step_index, step_count }).step-keys.ts:47maps any label containing/github/itoconnect_github. So a decline emitsstep_key='connect_github', status='completed'— byte-identical to a real connection. - Found: The consumer is named in this repo, one commit ago.
index.ts:83-84: "Key those events by step as well as by label, so a funnel over them (GitHub connect conversion, scout enable rate) keeps counting when a run words its tasks differently."step-keys.ts:26marks the keys "Frozen: these are event property values." HEAD is07758d1 feat(self-driving): key step events by step, not by the agent's wording (#1083), so this funnel was built deliberately in the immediately preceding commit. - Found: Before this PR the two states were distinguishable. A decline emitted
[ABORT] github connection declined, which killed the query (linear.ts:143), so the task never transitioned tocompletedandconnect_github+completedimplied a real connection. The PR removes that separation without adding a replacement. - Found: No discriminator survives.
step_nameis the only other property, and it carries the agent's free-worded label — precisely the drifting valuestep_keyexists to normalize away (step-keys.ts:5-8), so it cannot be keyed on.TaskStatus(wizard-ui.ts:21-26) does have aSkipped = 'skipped'member, butagent-interface.ts:1400gates emission toin_progress/completedonly, so marking the task skipped would emit no event at all and drop the step from progression instead. GreppingwizardCapture(acrosssrc/lib/programs/self-driving/andsrc/lib/wizard-tools/finds no other event that records a GitHub connection result. - Found: The fix needs no new concept.
[DASHBOARD_URL]/[NOTEBOOK_URL](signals.ts:37, captured atagent-interface.ts:1609-1624) already establish an agent→wizard outcome channel, so an additive outcome property follows existing precedent. - Impact: GitHub connect conversion silently rises toward 100% at that step, because every declined run now reports the same completed event as a connected run. The team loses the one signal that would show how many users still decline — the exact measurement this PR's own motivation rests on — and the degradation is invisible in the data rather than visibly broken.
Issue description
The prompt marks the GitHub task completed after a decline. The runner emits this as a completed connect_github step. Existing funnels use this event to measure GitHub connection conversion. Declined connections will now inflate that conversion.
Suggested fix
Record an explicit outcome such as connected or skipped in the step event. Keep the task completed so the run can continue, but exclude skipped outcomes from the connection funnel.
Prompt to fix with AI (copy-paste)
## Context
@src/lib/programs/self-driving/prompt.ts#L148-150
<issue_description>
The prompt marks the GitHub task completed after a decline. The runner emits this as a completed `connect_github` step. Existing funnels use this event to measure GitHub connection conversion. Declined connections will now inflate that conversion.
</issue_description>
<issue_validation>
- **Checked:** The full step-analytics path — the prompt instruction, the event emitter, the key resolver, the status vocabulary, and every other self-driving analytics call that could carry the GitHub outcome.
- **Found:** The mechanism is exact. `prompt.ts` STEP 3 now says on a decline: "Record GitHub as a pending follow-up for STEP 7, **mark this task completed**, and continue with STEP 3b." `agent-interface.ts:1396-1424` fires on any transition to `in_progress` or `completed` and captures `analytics.wizardCapture('step', { step_name, step_key, status, step_index, step_count })`. `step-keys.ts:47` maps any label containing `/github/i` to `connect_github`. So a decline emits `step_key='connect_github', status='completed'` — byte-identical to a real connection.
- **Found:** The consumer is named in this repo, one commit ago. `index.ts:83-84`: "Key those events by step as well as by label, so a funnel over them (**GitHub connect conversion**, scout enable rate) keeps counting when a run words its tasks differently." `step-keys.ts:26` marks the keys "Frozen: these are event property values." HEAD is `07758d1 feat(self-driving): key step events by step, not by the agent's wording (#1083)`, so this funnel was built deliberately in the immediately preceding commit.
- **Found:** Before this PR the two states were distinguishable. A decline emitted `[ABORT] github connection declined`, which killed the query (`linear.ts:143`), so the task never transitioned to `completed` and `connect_github`+`completed` implied a real connection. The PR removes that separation without adding a replacement.
- **Found:** No discriminator survives. `step_name` is the only other property, and it carries the agent's free-worded label — precisely the drifting value `step_key` exists to normalize away (`step-keys.ts:5-8`), so it cannot be keyed on. `TaskStatus` (`wizard-ui.ts:21-26`) does have a `Skipped = 'skipped'` member, but `agent-interface.ts:1400` gates emission to `in_progress`/`completed` only, so marking the task skipped would emit **no event at all** and drop the step from progression instead. Grepping `wizardCapture(` across `src/lib/programs/self-driving/` and `src/lib/wizard-tools/` finds no other event that records a GitHub connection result.
- **Found:** The fix needs no new concept. `[DASHBOARD_URL]` / `[NOTEBOOK_URL]` (`signals.ts:37`, captured at `agent-interface.ts:1609-1624`) already establish an agent→wizard outcome channel, so an additive outcome property follows existing precedent.
- **Impact:** GitHub connect conversion silently rises toward 100% at that step, because every declined run now reports the same completed event as a connected run. The team loses the one signal that would show how many users still decline — the exact measurement this PR's own motivation rests on — and the degradation is invisible in the data rather than visibly broken.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Record an explicit outcome such as `connected` or `skipped` in the step event. Keep the task completed so the run can continue, but exclude skipped outcomes from the connection funnel.
</potential_solution>
There was a problem hiding this comment.
Confirmed — this is a real regression introduced by this PR, and it defeats the PR's own purpose (measuring how many users decline GitHub). On a decline the agent now marks the GitHub task completed, and the run emits a step-progress event keyed connect_github with status=completed — byte-identical to a genuine connection. The 'GitHub connect conversion' funnel that consumes these events (added last commit in #1083) will therefore drift toward 100% and hide every decline. Nothing else in the self-driving analytics path records a GitHub outcome, so no discriminator survives.
I'm escalating rather than fixing this unattended, because the fix is a design decision, not a mechanical one, and its effectiveness can only be proven by a live end-to-end run:
- Which discriminator? Two defensible options, each with a cost. (a) Reuse the existing
skippedtask status on decline — but the step emitter only fires onin_progress/completed, so this means changing generic (non-self-driving) infrastructure to also emitskipped, andskippedarguably pollutes the drop-off funnel since a declined-but-continued run is not a stop. (b) Add an explicitoutcomeproperty (connected/declined) to the step event via an agent→wizard signal (like the existing[DASHBOARD_URL]/[NOTEBOOK_URL]channel) — cleaner semantically but a change to an analytics event contract a live funnel already reads. - Provability: whichever route, it depends on the agent reliably setting the new status/signal on decline, which is prompt-steering behavior only verifiable with an e2e run, not with lint/unit tests here.
- Out of repo: the funnel query itself (excluding declines) lives PostHog-side, not in this codebase.
A maintainer should pick the discriminator (skipped-status vs. new outcome property), confirm whether declined runs should count in the drop-off funnel, and validate the agent actually emits it via an e2e run before this merges — otherwise the conversion metric ships silently broken.
The PR updated the step-3 table row (ARCHITECTURE.md:133) to say GitHub is RECOMMENDED and a declined connection is "not an abort", but three other statements in the same guide still described GitHub as required and the decline as terminal, leaving two opposite contracts for the same step: - §1 step summary (line 82): "required ... abort if declined". - Skill cross-cutting-rules paragraph (line 244): "the required step-3 GitHub gate". - Prod-merge checklist blockquote (line 556): "the decline ... which aborts ... since the run can't proceed without GitHub". All three now match the new degrade posture: GitHub is strongly recommended, a decline records a pending follow-up and the run continues. The §6 "Gating & prerequisites" entry (line 380) is softened from "required" to "needed for repo selection" so it no longer reads as a hard run gate. Docs-only; the wizard's own guide is never loaded into a prompt. Generated-By: PostHog Desktop Task-Id: 8bd91fe5-4ab6-4767-98a0-59986bd00f1a
STEP 3 now degrades instead of aborting when GitHub is declined, so a user who skips GitHub reaches the success outro. That screen advertised "Kick off a PR" as if available, though a finding cannot become a PR without GitHub. Tie the PR next-step to connecting GitHub so the copy reads correctly for both connected and declined runs, without needing new run-time state. Addresses ReviewHog finding "Success screen offers an unavailable PR action". Generated-By: PostHog Desktop Task-Id: b9aefec2-e4de-4f5f-bf3b-98fde420c756
|
Thanks for the three findings — all valid. Here is how I handled each. Finding 1 — ARCHITECTURE.md still said GitHub required: fixed on the branch (your own commit Finding 2 — success screen offered an unavailable PR action: shipped a deterministic, zero-risk mitigation in Finding 3 — declined GitHub steps count as connections: not fixed unattended, for the reasons you escalated. Distinguishing a decline in the emitted Findings 2 (full state-driven form) and 3 are left for a maintainer with an e2e run; this turn removes the most misleading copy and keeps the docs consistent. |
Problem
no mcp server found).[ABORT] github connection declined.SELF_DRIVING_ABORT_CASESturned that into a terminalOutroKind.Error— nothing in the run survived it.Changes
prompt.ts, STEP 3): a declined GitHub connection is now a valid outcome. The agent records GitHub as a pending follow-up and continues — sources still get enabled, scouts tuned, the inbox filled. STEP 7 names the pending GitHub follow-up first so the user can connect later and turn findings into fixes.detect.ts): kept for the case where the skill still forces the abort. Two fixes:/^github connection declined$/ito/^\W*github connection declined\W*$/iso stray punctuation the skill sometimes wraps around the contract string (e.g.github connection declined`.')) no longer falls through to the generic outro.ARCHITECTURE.md) and tests updated to lock in the new degrade posture and the punctuation tolerance.Test plan
pnpm build && pnpm test— 1841 tests pass.pnpm lint— 0 errors.LLM context
Authored by Claude via PostHog Desktop. Scope kept to the three files in the report's blast radius (
prompt.ts,detect.ts,linear.tspath);linear.tsneeded no change once the abort no longer fires on decline. The skill's own "Connect GitHub" reference lives in the external context-mill repo; the prompt now explicitly supersedes any "abort on decline" wording there.Created with PostHog Desktop from this inbox report.