Skip to content

fix(onboarding,brevo): /start always 302 + Brevo 401 carries correct agent_action (API-5/6)#172

Merged
mastermanas805 merged 2 commits into
masterfrom
fix/start-302-brevo-agent-action
May 29, 2026
Merged

fix(onboarding,brevo): /start always 302 + Brevo 401 carries correct agent_action (API-5/6)#172
mastermanas805 merged 2 commits into
masterfrom
fix/start-302-brevo-agent-action

Conversation

@mastermanas805
Copy link
Copy Markdown
Member

Two QA findings from the 2026-05-29 round (/tmp/qa-session/shared/INBOX.md).

API-5 (P2) — GET /start always 302s

Per CLAUDE.md 'Live API surface': '/start (302 → dashboard /claim?t=jwt)'. Previously, an invalid/missing/expired token surfaced the raw JSON envelope {"ok":false,"error":"invalid_token"} with HTTP 400. /start URLs land in agents' terminal logs (upgrade: https://api.instanode.dev/start?t=...); when a user copy-pastes a stale one into a browser, they see naked JSON instead of a friendly recovery flow.

Fix: pass the token through verbatim and let the dashboard's ClaimPage handle every validation case (expired / unrecognised / already-claimed / empty). The JTI lookup now happens once at /claim time where it's actually load-bearing — no more DB lookups on every drive-by /start hit. landing_viewed metric still increments so the funnel pivot stays measurable.

Cases:

  • Missing ?t → 302 to /claim with no t= query, dashboard renders empty state.
  • Garbage token → 302 to /claim?t=garbage, dashboard renders 'invalid token'.
  • Unknown JTI → 302 to /claim?t=<jwt>, dashboard does the lookup.
  • Happy path — unchanged (302 to /claim?t=<jwt>).

API-6 (P2) — Brevo 401 agent_action targets operator

POST /webhooks/brevo/<bogus-secret> returned 401 with the canonical unauthorized error class — and the canonical agent_action for that class (Tell the user their INSTANODE_TOKEN is missing or invalid. Have them log in at https://instanode.dev/login...). The Brevo webhook is unrelated to user auth: an agent reading the error envelope would tell the user to log in for a new INSTANODE_TOKEN, which is uselessly wrong. The actual incident is an operator-side Brevo dashboard config drift.

Fix: new error class brevo_secret_mismatch with operator-targeted agent_action that tells the caller to verify the Brevo dashboard webhook URL contains the configured BREVO_WEBHOOK_SECRET. HTTP status stays 401 — only the error CODE + agent_action copy change. Existing operator alerting that pivots off metrics.BrevoWebhookEventsTotal{result="unauthorized"} is unaffected.

Multi-surface checklist (memory rule 22)

Surface Touched? Why
api/internal/handlers/openapi.go Yes /start contract changed from {302,400} to {302} only; t now optional
api/internal/handlers/helpers.go Yes New brevo_secret_mismatch agent_action
content/llms.txt No /start was already documented as 302; the dashboard-renders-error change is not customer/agent-facing
METRICS-CATALOG.md No No new metrics
dashboard No ClaimPage already handles the 'no/expired token' UI state

Test coverage

  • TestResidualStartLanding_MissingToken_RedirectsToClaim
  • TestResidualStartLanding_GarbageToken_StillRedirects
  • TestResidualStartLanding_UnknownJTI_StillRedirects
  • TestResidualStartLanding_HappyPath_Redirects — unchanged
  • TestBrevoTxWebhook_SecretMismatch_AgentActionMentionsBrevo — pins API-6: error code is brevo_secret_mismatch, body mentions Brevo, does NOT carry INSTANODE_TOKEN or the user-login recovery script

QA evidence

  • /tmp/qa-session/API/start_bogus.txt, start_bogus_headers.txt — pre-fix 400 JSON
  • /tmp/qa-session/API/brevo_bogus.json — pre-fix INSTANODE_TOKEN agent_action

🤖 Generated with Claude Code

Manas Srivastava and others added 2 commits May 29, 2026 14:15
…agent_action (API-5/6)

== API-5 (P2) — GET /start ALWAYS 302s ==

Per CLAUDE.md 'Live API surface': '/start (302 → dashboard /claim?t=jwt)'.
Previously, an invalid/missing/expired token surfaced the raw JSON envelope
'{"ok":false,"error":"invalid_token"}' with HTTP 400. /start URLs land
in agents' terminal logs ('upgrade: https://api.instanode.dev/start?t=...');
when a user copy-pastes a stale one into a browser, they see naked JSON
instead of a friendly recovery flow.

Fix: pass the token through verbatim and let the dashboard's ClaimPage
handle every validation case (expired / unrecognised / already-claimed /
empty). The JTI lookup now happens once at /claim time where it's actually
load-bearing — the platform side no longer wastes a DB lookup on every
drive-by /start hit. landing_viewed metric still increments so the funnel
pivot stays measurable.

Edge cases:
  - Missing ?t — 302 to /claim with no t= query, dashboard renders empty state.
  - Garbage token — 302 to /claim?t=garbage, dashboard renders 'invalid token'.
  - Unknown JTI — 302 to /claim?t=<jwt>, dashboard does the lookup.
  - Happy path — unchanged (302 to /claim?t=<jwt>).

== API-6 (P2) — Brevo 401 agent_action targets operator, not user ==

POST /webhooks/brevo/<bogus-secret> returned 401 with the canonical
'unauthorized' error class — and the canonical agent_action for that class
('Tell the user their INSTANODE_TOKEN is missing or invalid. Have them log
in at https://instanode.dev/login...'). The Brevo webhook is unrelated to
user auth: an agent reading the error envelope would tell the user to log
in for a new INSTANODE_TOKEN, which is uselessly wrong. The actual incident
is an OPERATOR-side Brevo dashboard config drift.

Fix: introduce a new error class 'brevo_secret_mismatch' with operator-
targeted agent_action that tells the caller to verify the Brevo dashboard
webhook URL contains the configured BREVO_WEBHOOK_SECRET. HTTP status stays
401 — only the error CODE + agent_action copy change. Existing operator
alerting that pivots off metrics.BrevoWebhookEventsTotal{result="unauthorized"}
is unaffected.

== Multi-surface checklist (memory rule 22) ==

| Surface | Touched | Why |
|---|---|---|
| api/internal/handlers/openapi.go | Yes | /start contract changed from {302,400} to {302} only; spec updated to reflect always-302 + 't' parameter now optional |
| api/internal/handlers/helpers.go | Yes | New 'brevo_secret_mismatch' entry in codeToAgentAction map |
| content/llms.txt | No | /start was already documented as 302 — the dashboard-renders-error behaviour is the only new contract, and it's not a customer/agent-facing change (every agent that follows the 302 already lands at the dashboard regardless) |
| METRICS-CATALOG.md | No | No new metrics |
| dashboard | No | ClaimPage already handles the 'no/expired token' UI state |

== Test coverage ==

- TestResidualStartLanding_MissingToken_RedirectsToClaim — no t= → 302
- TestResidualStartLanding_GarbageToken_StillRedirects — invalid JWT → 302
- TestResidualStartLanding_UnknownJTI_StillRedirects — valid sig + unknown JTI → 302
- TestResidualStartLanding_HappyPath_Redirects — unchanged
- TestBrevoTxWebhook_SecretMismatch_AgentActionMentionsBrevo — pins API-6:
  error code is brevo_secret_mismatch, agent_action mentions Brevo, does NOT
  carry INSTANODE_TOKEN or the user-login recovery script

== Four-pass deploy ritual (memory rule 23) ==

- [x] Local build + vet + new tests green
- [ ] CI green (this PR)
- [ ] Auto-deploy via deploy.yml on merge
- [ ] Live verify: curl https://api.instanode.dev/healthz | jq .commit_id == merge SHA

== QA evidence ==

- /tmp/qa-session/API/start_bogus.txt + start_bogus_headers.txt — pre-fix 400 JSON
- /tmp/qa-session/API/brevo_bogus.json — pre-fix INSTANODE_TOKEN agent_action

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…_action to U3 contract

Follow-up to the round-1 build-and-test failures:

1. brevo_secret_mismatch agent_action — reworded to comply with U3 contract
   enforced by TestAgentActionContract: must open with 'Tell the user', contain
   a full https://instanode.dev/ URL, and stay under 280 chars. New copy:

     Tell the user this is a Brevo-webhook config mismatch, not their auth.
     Operators must verify the Brevo dashboard webhook URL matches the
     configured BREVO_WEBHOOK_SECRET — see https://instanode.dev/docs/email.

   Still routes operators to the correct fix (Brevo dashboard webhook URL,
   not the user-login flow), but in the standard agent-action voice. 210 chars.

2. Existing /start tests aligned with the API-5 always-302 contract:
   - TestStartLanding_AlreadyClaimedRedirectsToDashboardWithFlag — now asserts
     302 to /claim?t=, no longer asserts the already_claimed=true flag (dashboard
     handles the already-claimed UI now).
   - TestStartLanding_MissingTokenReturns400 — now asserts 302 with no t= query.
   - TestStartLanding_UnknownJTI_400 — now asserts 302 to /claim?t=.
   - TestOnboarding_GetStart_ExpiredJWT_Returns400LinkExpired — 302 to /claim.
   - TestOnboarding_GetStart_TamperedJWT_Returns400InvalidLink — 302 to /claim.
   - TestStartLanding_NoToken_Returns400 — 302 to /claim (no t=).
   - TestStartLanding_TamperedJWT_Returns400 — 302 to /claim?t=.
   - TestStartLanding_AlreadyClaimed_Returns302 — 302 to /claim?t= (no flag).
   - TestOnboarding_JWTWithFutureIssuedAt_Returns400 — 302 to /claim?t=.

   Test function NAMES kept verbatim for grep stability; ASSERTIONS updated
   to the new always-bounce contract per CLAUDE.md 'Live API surface' line.

3. TestBrevoTxWebhook_SecretMismatch_AgentActionMentionsBrevo — tightened the
   negative assertion to match the rephrased operator copy ('log in at
   https://instanode.dev/login to mint a new one' is the load-bearing piece
   we must NOT carry).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mastermanas805 mastermanas805 merged commit 764da11 into master May 29, 2026
14 of 15 checks passed
@mastermanas805 mastermanas805 deleted the fix/start-302-brevo-agent-action branch May 29, 2026 09:31
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