Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ node_modules/
.superpowers/
.pi/
.secrets.choice
.dev.vars
Empty file.
2 changes: 2 additions & 0 deletions openspec/changes/log-invite-outcomes/.openspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-11-01
45 changes: 45 additions & 0 deletions openspec/changes/log-invite-outcomes/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# Design

## Context

`src/index.js` handles `POST /invite` with ten terminal branches. Six already log structured events (`secrets_missing`, `turnstile_verify_error`, `slack_unreachable`, `invite_failed`, plus the scheduled health-check events); four log nothing: turnstile-verification-failed, invalid-email, already-invited, and the unmapped-Slack-error branch that hid planner#3069 for weeks. Slack documents 44 error strings for the invite endpoint and has shown it adds new ones silently, so a fixed mapping table can never guarantee visibility. See proposal.md — Why.

## Goals / Non-Goals

**Goals:**
- Every terminal branch of `POST /invite` produces one machine-readable `invite_outcome` event, additive to existing events.
- Raw Slack error strings reach Workers Logs even when unmapped.
- No change to HTTP statuses, page text, or existing event shapes (tests pin those).
- Grafana-ready: a single series (`event=invite_outcome`) covers success/failure breakout and Slack-error grouping.

**Non-Goals:**
- Changing the user-facing error messages or status codes.
- Mapping new Slack errors to friendly pages (separate, later change if wanted).
- PII in logs (no email values, no names).
- The Cloudflare → Grafana ingestion pipeline and dashboard (separate change after this merges).

## Decisions

**1. One event per branch, not per sub-attempt.** `invite_outcome` is logged once, at the point each branch returns its response. Alternative considered: logging each Slack response separately plus a request-level rollup — rejected, two events per request double the intake for zero dashboard benefit.

**2. `slack_error` carries the verbatim Slack string for every `ok:false` response, mapped or not.** This is the property that makes future Slack-side changes immediately visible. Alternative considered: only logging unknown errors — rejected; dashboards need the mapped-error series (`already_invited`, `invalid_email`) too, and `slack_error` plus `detail` agree by construction.

**3. `detail` uses stable short keys, not the Slack error, for non-Slack branches.** Keys: `missing_email`, `verification_error`, `verification_failed`, `secrets_missing`, `slack_unreachable`, `unexpected_error`, `invited`, plus `slack_error:<string>`-summary via `detail: 'slack_error'` when the Slack call returns an error (with `slack_error` holding the string). Rationale: `detail` is low-cardinality and dashboard-groupable; the high-cardinality string lives in its own field so an unrelated Slack rename cannot fragment the detail series.

**4. Emit inside each return path, not from a wrapper.** The branches build different `Response` objects; a single wrapper around every return would need state threading for marginal benefit. Straight-line `console.log(...)` next to each return keeps the diff small and reviewable. Alternative considered: try/finally wrapper keyed on a mutable outcome variable — rejected as more machinery for the same observable result.

**5. Existing events stay.** `secrets_missing` / `slack_unreachable` / `invite_failed` / `turnstile_verify_error` are pinned by tests and carry the distinctive message payloads (for example turnstile error-codes). `invite_outcome` duplicates some of their information by design; Cloudflare intake for this worker is tiny (81 events in a week), so the redundancy is cheap.

## Risks / Trade-offs

- [Some risk of a missing branch producing no event] → The tasks list enumerates all ten branches explicitly; the test suite asserts one `invite_outcome` per terminal branch scenario, so a gap shows as a failing test rather than a silent hole.
- [Log volume slightly up (two events on some failure paths)] → Intake for this worker is a few thousand events/month at current traffic; negligible cost.
- [New Slack error strings still render the generic page] → Accepted: user-facing text genericity is the point; identifiability comes from `slack_error`.

## Migration Plan

Deploy is the rollout: additive logging, no data or config changes. Rollback is a revert re-deploy. No coordination with the Grafana work needed; it builds on the deployed series afterwards.

## Open Questions

(none)
31 changes: 31 additions & 0 deletions openspec/changes/log-invite-outcomes/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Proposal

## Why

The 2026-10 incident (planner#3069) showed the invite worker's Slack-error branch returns an HTTP 200 page and logs nothing, so a Slack-side change (`inviter_email_not_verified`) made invites fail for weeks while Cloudflare dashboards showed zero errors. Every failure and success branch must emit one structured log event so Workers Logs (and a future Grafana dashboard) can identify invite outcomes, including Slack error strings the worker has never seen.

## What Changes

- Add a single structured log event, `invite_outcome`, emitted on every `POST /invite` branch: `{ event: 'invite_outcome', outcome: 'success' | 'failure', detail, slack_error }`.
- `outcome: 'success'` with `detail: 'invited'` for actual invitations.
- `already_invited` / `already_in_team` are logged as `outcome: 'success'` with their Slack error also recorded in `slack_error`.
- `invalid_email`, turnstile failures, missing email, unreachable Slack, unexpected errors log as `outcome: 'failure'` with a `detail` key; the raw Slack error string is always copied to `slack_error` for any `ok:false` Slack response, mapped or not.
- Keep all existing log events (`secrets_missing`, `turnstile_verify_error`, `slack_unreachable`, `invite_failed`, health-check events) unchanged; the new event is additive.
- No change to HTTP status codes or user-facing page text; pages stay 200 for handled Slack errors.
- No PII logging: email addresses are not copied into `invite_outcome`.

## Capabilities

### New Capabilities

- `invite-observability`: Structured, complete logging of invite-processing outcomes (request branch, Slack response, health check) so external failures are identifiable in Workers Logs without redeploying instrumentation.

### Modified Capabilities

(none — the project has no existing specs)

## Impact

- `src/index.js`: one added log statement per branch (additive; public page responses unchanged).
- `test/`: existing failure-shape tests keep passing; new cases assert the `invite_outcome` event on each branch (success, mapped error, unmapped error, already-invited, turnstile fail, missing email).
- Downstream: enables the planned Cloudflare Workers Logs → Grafana ingestion work (separate change), which will consume `invite_outcome` as its primary series.
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# Spec Delta

## Purpose

Emit one structured log event per invite-processing branch (request handling and Slack response) so any failure — including Slack error strings the worker does not yet know — is identifiable in Workers Logs, later Grafana dashboards, and incident reports, while user-facing page responses stay unchanged.

## ADDED Requirements

### Requirement: Log one invite_outcome event per processed invite submission

Every `POST /invite` request that reaches a terminal branch MUST log exactly one structured event with `event: 'invite_outcome'`, `outcome` (`'success'` or `'failure'`), `detail` (a stable short branch key), and `slack_error` whenever Slack returned `ok: false`.

#### Scenario: Success

- **WHEN** Slack returns `ok: true`
- **THEN** the log carries `outcome: 'success'`, `detail: 'invited'`, and no `slack_error` field.

#### Scenario: Mapped Slack errors keep their string

- **WHEN** Slack returns `already_invited`, `already_in_team`, or `invalid_email`
- **THEN** the log carries that exact string in `slack_error`, with `outcome: 'success'` for the first two and `outcome: 'failure'` for `invalid_email`.

#### Scenario: Unmapped Slack error is still visible

- **WHEN** Slack returns an error string the worker does not map
- **THEN** the response is the regular failure page (HTTP 200), and the log carries `outcome: 'failure'` with `slack_error` equal to the verbatim string.

#### Scenario: Slack call fails before a response

- **WHEN** the Slack HTTP call rejects
- **THEN** the log carries `outcome: 'failure'`, `detail: 'slack_unreachable'`.

#### Scenario: Unexpected exception

- **WHEN** an uncaught error terminates the request
- **THEN** the log carries `outcome: 'failure'`, `detail: 'unexpected_error'`.

### Requirement: Log non-Slack invite branch failures

Failures that terminate the request before the Slack call MUST be visible as `invite_outcome` events with `outcome: 'failure'` and a stable `detail` key, additive to any existing log events. Requests that never reach an invite branch MUST NOT log the event.

#### Scenario: Missing email

- **WHEN** a submission has no email value
- **THEN** the log carries `detail: 'missing_email'` with `outcome: 'failure'`.

#### Scenario: Rejected Turnstile

- **WHEN** the Turnstile siteverify call returns `success: false` for a form submission
- **THEN** the response is the verification-failed page (HTTP 403), and the log carries `detail: 'verification_failed'` with `outcome: 'failure'`.

#### Scenario: Missing secrets

- **WHEN** a required secret is unset
- **THEN** the log carries `detail: 'secrets_missing'` with `outcome: 'failure'`, alongside the existing `secrets_missing` event.

#### Scenario: Turnstile call error

- **WHEN** the Turnstile verification call fails to complete
- **THEN** the log carries `detail: 'verification_error'` with `outcome: 'failure'`, alongside the existing `turnstile_verify_error` event.

#### Scenario: Bot probes stay quiet

- **WHEN** a request serves static assets or a plain 404
- **THEN** no `invite_outcome` event is logged.

### Requirement: Log invite_outcome without personal data

`invite_outcome` events MUST NOT contain email addresses or other user-supplied personal data.

#### Scenario: Success event carries no PII

- **WHEN** an invite succeeds for any email address
- **THEN** the logged event holds only branch identity and Slack metadata, with no field carrying the submitted email value.
33 changes: 33 additions & 0 deletions openspec/changes/log-invite-outcomes/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Tasks

## 0. Test-stack migration (approved during apply: vitest 4 + Cloudflare vitest plugin)

- [x] 0.1 Migrate the suite to vitest 4 and @cloudflare/vitest-plugin; outbound mocks move from fetchMock to MSW (verified: `npm test` exit 0, all suites green)
- [x] 0.2 Add a createTestHarness-based log-assertion suite for invite_outcome events under vitest.harness.config.js (verified: log events asserted per branch below)

## 1. Add invite_outcome events to the request path (src/index.js)

- [x] 1.1 Log `invite_outcome` for the missing-email branch (`detail: 'missing_email'`) — asserted in test/logs.test.js; dev-server smoke shows the event on POST /invite with an empty email (400)
- [x] 1.2 Log `invite_outcome` for the missing-secrets branch (`detail: 'secrets_missing'`) alongside the existing `secrets_missing` event — asserted in test/logs.test.js (second createTestHarness instance running with no secrets)
- [x] 1.3 Log `invite_outcome` for the Turnstile-verification-error branch (`detail: 'verification_error'`) alongside the existing `turnstile_verify_error` event — asserted in test/logs.test.js (non-JSON upstream reply)
- [x] 1.4 Log `invite_outcome` for the Turnstile-verification-failed branch (`detail: 'verification_failed'`), which previously logged nothing — asserted in test/logs.test.js; dev-server smoke shows the event on a rejected challenge (403)
- [x] 1.5 Log `invite_outcome` for the Slack-call-failure branch (`detail: 'slack_unreachable'`) alongside the existing `slack_unreachable` event — asserted in test/logs.test.js (non-JSON upstream reply drives 502)
- [x] 1.6 Log `invite_outcome` with `outcome: 'success'`, `detail: 'invited'` when Slack returns `ok: true`, with no `slack_error` field and no email value — asserted in test/logs.test.js (event holds no email substring)
- [x] 1.7 Log `invite_outcome` for the mapped-error transitions: `already_invited` / `already_in_team` as `outcome: 'success'`, `invalid_email` as `outcome: 'failure'`, each carrying the exact Slack string in `slack_error` — asserted in test/logs.test.js via it.each over both mapped strings
- [x] 1.8 Log `invite_outcome` with the verbatim unmapped Slack error in `slack_error` for the generic-error branch — asserted with `inviter_email_not_verified` (the #3069 shape): HTTP 200 page plus the event
- [x] 1.9 Log `invite_outcome` with `detail: 'unexpected_error'` in the top-level catch alongside the existing `invite_failed` event — asserted in test/logs.test.js (non-form POST body drives 500)

## 2. Static and 404 paths stay quiet (src/index.js)

- [x] 2.1 Confirm asset serving and the plain 404 response emit no `invite_outcome` events — asserted in test/logs.test.js negative cases (favicon and the join page request both seen by the capturing harness with zero events)

## 3. Full verification (test/, full suites)

- [x] 3.1 Run the complete test suites (`vitest.config.js`, `vitest.nosecret.config.js`, `vitest.noslack.config.js`, `vitest.node.config.js`, `vitest.harness.config.js`) and confirm all pass — verified: 23+1+1+2+9 tests all green, `npm test` exit 0
- [x] 3.2 Exercise the real worker end-to-end in `wrangler dev` and confirm the console shows `invite_outcome` lines — verified: missing-email → failure/missing_email (400) and rejected-Turnstile → failure/verification_failed (403) observed in dev output

## Workflow follow-up

- Implementation lands through a feature branch and a draft PR against codebar/slack-invite (never the default branch directly).
- After merge and deploy, a repeat submission against slack.codebar.io should produce an `invite_outcome` event visible in the Cloudflare dashboard live logs.
- The Cloudflare → Grafana ingestion pipeline and dashboard are a separate change, explored once this one is merged.
32 changes: 32 additions & 0 deletions openspec/config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
schema: spec-driven

# Project context (optional)
# Add only constraints that should guide OpenSpec artifacts and workflows.
# Include constraints an agent cannot infer by reading the code.
# Keep general project documentation and discoverable codebase facts out.
# Example:
# context: |
# Designs and tasks must cover Windows, macOS, and Linux
# Write all artifacts in Spanish

# Per-artifact rules (optional)
# Add custom rules for specific artifacts.
# Example:
# rules:
# proposal:
# - Keep proposals under 500 words
# - Always state what is out of scope
# tasks:
# - Break tasks into chunks of max 2 hours

# Per-operation guidance (optional)
# Add advisory guidance for how apply and archive work should be conducted.
# This is separate from artifact rules above.
# Example:
# operations:
# apply:
# guidance:
# - Keep test summaries concise
# archive:
# guidance:
# - Summarize the archive outcome before finishing
Empty file added openspec/specs/.gitkeep
Empty file.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"scripts": {
"dev": "wrangler dev",
"deploy": "wrangler deploy",
"test": "vitest run && vitest run -c vitest.nosecret.config.js && vitest run -c vitest.noslack.config.js && vitest run -c vitest.node.config.js"
"test": "vitest run && vitest run -c vitest.nosecret.config.js && vitest run -c vitest.noslack.config.js && vitest run -c vitest.node.config.js && vitest run -c vitest.harness.config.js"
},
"devDependencies": {
"@cloudflare/vitest-plugin": "^1.4.11",
Expand Down
20 changes: 20 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,15 @@ function htmlResponse(message, { status = 200, failed = false } = {}) {
return new Response(resultPage(message, failed), { status, headers: HTML_HEADERS });
}

// One structured event per terminal branch. `slack_error` carries the verbatim
// string for any ok:false Slack response, mapped or not — new error strings
// Slack ships (see planner#3069) must stay visible while the page stays generic.
function logOutcome(outcome, detail, slackError) {
const event = { event: 'invite_outcome', outcome, detail };
if (slackError !== undefined) event.slack_error = slackError;
console.log(event);
}

function failClosed() {
return htmlResponse('Failed! Verification failed. Please try again.', { status: 403, failed: true });
}
Expand Down Expand Up @@ -88,11 +97,13 @@ export default {
const turnstileToken = typeof turnstileField === 'string' ? turnstileField : '';

if (!email) {
logOutcome('failure', 'missing_email');
return htmlResponse('Failed! your email is required.', { status: 400, failed: true });
}

if (!env.TURNSTILE_SECRET || !env.SLACK_TOKEN) {
console.log({ event: 'secrets_missing', turnstile_secret: !env.TURNSTILE_SECRET, slack_token: !env.SLACK_TOKEN });
logOutcome('failure', 'secrets_missing');
return failClosed();
}

Expand All @@ -101,9 +112,11 @@ export default {
verify = await verifyTurnstile(turnstileToken, env.TURNSTILE_SECRET);
} catch (error) {
console.log({ event: 'turnstile_verify_error', error: String(error) });
logOutcome('failure', 'verification_error');
return failClosed();
}
if (!verify.success) {
logOutcome('failure', 'verification_failed');
return failClosed();
}

Expand All @@ -112,15 +125,18 @@ export default {
slack = await sendSlackInvite(email, env.SLACK_TOKEN);
} catch (error) {
console.log({ event: 'slack_unreachable', error: String(error) });
logOutcome('failure', 'slack_unreachable');
return htmlResponse(
'Failed! Something has gone wrong. Please contact a system administrator.',
{ status: 502, failed: true }
);
}
if (slack.ok) {
logOutcome('success', 'invited');
return htmlResponse(`Success! Check “${escapeHtml(email)}” for an invite from Slack.`);
}
if (slack.error === 'already_invited' || slack.error === 'already_in_team') {
logOutcome('success', 'slack_error', slack.error);
return htmlResponse(
`Success! You were already invited.<br>Visit <a href="https://codebar.slack.com">codebar</a>`
);
Expand All @@ -129,9 +145,13 @@ export default {
if (slack.error === 'invalid_email') {
message = 'The email you entered is an invalid email.';
}
// Slack has been observed to answer ok:false without an error field;
// keep slack_error present so the event never loses its payload.
logOutcome('failure', 'slack_error', String(slack.error ?? '(no error field)'));
return htmlResponse(`Failed! ${message}`, { failed: true });
} catch (error) {
console.log({ event: 'invite_failed', error: String(error) });
logOutcome('failure', 'unexpected_error');
return htmlResponse(
'Failed! Something has gone wrong. Please contact a system administrator.',
{ status: 500, failed: true }
Expand Down
Loading
Loading