Skip to content

feat: log one invite_outcome event per invite branch - #9

Merged
mroderick merged 3 commits into
mainfrom
log-invite-outcomes
Nov 1, 2026
Merged

mroderick merged 3 commits into
mainfrom
log-invite-outcomes

Conversation

@mroderick

@mroderick mroderick commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Closes #3069 follow-up work identified during the planner#3069 investigation: the unmapped Slack error branch returned an HTTP 200 page and logged nothing, which hid inviter_email_not_verified (a silent Slack platform change) while dashboards showed zero errors.

Summary

  • Implements the log-invite-outcomes openspec change: every terminal POST /invite branch emits one structured event, { event: 'invite_outcome', outcome, detail, slack_error }, additive to the existing log events; HTTP statuses and page text unchanged.
  • slack_error always carries the verbatim Slack string for ok: false replies, mapped or not, so future Slack-side changes stay visible without a redeploy.
  • already_invited / already_in_team count as success; failures use stable detail keys (missing_email, verification_error, verification_failed, secrets_missing, slack_unreachable, unexpected_error).
  • Log-event assertions: test/logs.test.js drives the Worker through wrangler's createTestHarness and asserts server.getLogs() per branch (the plugin runtime exposes no console capture). The node-side suites (createTestHarness + config-pin) run under vitest.harness.config.js / vitest.node.config.js, both wired into npm test.

Review notes

Schema-first reading

  • src/index.js logOutcome() and its call sites: look for a terminal branch emitting nothing (would show as a failing logs-suite case), and confirm no branch passes user input into the event (PII check: the logs suite asserts the submitted email never appears in the success event).
  • Event-shape stability is the contract for the planned Grafana ingestion: outcome (success/failure), detail (low-cardinality key), slack_error (verbatim string, present only on ok: false replies). Changes to these keys would break the future dashboard grouping, prefer additive fields.

Stacked on #8 (test-stack migration). Review that one first; this diff builds on it.

Deliberately not done

  • No friendly-page mapping for new Slack errors (their visibility comes from the log event; page text stays generic).
  • Cloudflare Workers Logs → Grafana ingestion and dashboard land as a separate change after this merges.

@mroderick
mroderick force-pushed the log-invite-outcomes branch from 4587ea8 to ddca3a1 Compare November 1, 2026 18:10
@mroderick
mroderick added this pull request to stack #10 November 1, 2026 20:48
@mroderick
mroderick force-pushed the log-invite-outcomes branch from a164f95 to da2289b Compare November 1, 2026 20:56
Base automatically changed from test-stack-migration to main November 1, 2026 20:59
Planning artifacts for additive structured logging of every POST /invite
branch (see proposal.md). Investigation tails the planner#3069 incident: the
unmapped Slack error branch returned a 200 page and logged nothing, hiding
inviter_email_not_verified for weeks.
Implements the log-invite-outcomes openspec change. Every terminal branch of
POST /invite now emits `{ event: 'invite_outcome', outcome, detail,
slack_error }` in addition to the existing events:

- success invite: success/invited, no slack_error, no email in the payload
- already_invited / already_in_team: success with the Slack string kept
- invalid_email and any other ok:false reply: failure with the verbatim
  slack_error, mapped or not
- missing_email, secrets_missing, verification_error, verification_failed,
  slack_unreachable, unexpected_error: failure with a stable detail key

HTTP statuses and page text are unchanged. Log-assertion coverage lives in
test/logs.test.js, which drives the Worker through wrangler's
createTestHarness and asserts server.getLogs() per branch (the Workers
plugin runtime exposes no console capture; see tasks 0.1-0.2).
- The secrets_missing invite_outcome branch was unreachable in the logs
  suite (harness always seeded secrets) while tasks.md claimed it asserted;
  a second createTestHarness instance now boots with no secrets and pins
  exactly one failure/secrets_missing event on the fail-closed response.
- The mapped-error fixture covered only already_invited; it.each now asserts
  already_in_team (success with the exact string) too.
- The stay-quiet requirement had no automated check; the logs suite now
  asserts zero invite_outcome events for favicon and the join page request.
- Slack replying ok:false without an error field produced an event with
  detail 'slack_error' but no slack_error field, contradicting the spec; the
  generic branch now falls back to a sentinel so the field is always
  present, with a fixture pinning it.
- The PII guard ran only on the success event; expectSingleOutcome now
  checks every asserted event against the email that produced it.
- tasks.md verification wording now names the real assertion paths.
@mroderick
mroderick force-pushed the log-invite-outcomes branch from da2289b to ed1b07e Compare November 1, 2026 20:59
@mroderick
mroderick marked this pull request as ready for review November 1, 2026 20:59
@mroderick
mroderick merged commit 3ac4353 into main Nov 1, 2026
1 check passed
@mroderick
mroderick deleted the log-invite-outcomes branch November 1, 2026 21:01
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