From a55b82cb6e97662c135e8d7d57e57757680e8cf6 Mon Sep 17 00:00:00 2001 From: TOMOKI977 Date: Thu, 24 Sep 2026 15:17:30 -0400 Subject: [PATCH] docs(openspec): plan github-alerts change Exploration, proposal, specs, design and tasks for routing GitHub issue and PR alerts to the Telegram forum topic linked to each repo. --- openspec/changes/github-alerts/design.md | 142 ++++++++++++++++++ openspec/changes/github-alerts/explore.md | 55 +++++++ openspec/changes/github-alerts/proposal.md | 84 +++++++++++ .../github-alerts/specs/github-alerts/spec.md | 64 ++++++++ .../specs/github-webhook/spec.md | 77 ++++++++++ .../specs/repo-topic-links/spec.md | 89 +++++++++++ openspec/changes/github-alerts/tasks.md | 72 +++++++++ 7 files changed, 583 insertions(+) create mode 100644 openspec/changes/github-alerts/design.md create mode 100644 openspec/changes/github-alerts/explore.md create mode 100644 openspec/changes/github-alerts/proposal.md create mode 100644 openspec/changes/github-alerts/specs/github-alerts/spec.md create mode 100644 openspec/changes/github-alerts/specs/github-webhook/spec.md create mode 100644 openspec/changes/github-alerts/specs/repo-topic-links/spec.md create mode 100644 openspec/changes/github-alerts/tasks.md diff --git a/openspec/changes/github-alerts/design.md b/openspec/changes/github-alerts/design.md new file mode 100644 index 0000000..31b4da9 --- /dev/null +++ b/openspec/changes/github-alerts/design.md @@ -0,0 +1,142 @@ +# Design: GitHub Alerts Routed to Linked Forum Topics + +## Technical Approach + +This change reuses the existing hexagonal layout. A new Hono route, `POST /github/webhook`, reads the raw body and verifies `X-Hub-Signature-256` with WebCrypto before anything is parsed. An adapter mapper (`adapters/github`) then turns the JSON into a small domain `GithubEvent`, or `null` when the event is unsupported. A pure use case, `routeGithubEvent`, resolves org to claim to team to link, formats a plain-text alert and sends it through an `AlertSender` port. That port is backed by grammY's `Api.sendMessage` with `message_thread_id`. Linking uses three admin commands that call pure use cases (`linkRepoToTopic`, `unlinkRepo`, `listRepoLinks`), so a future LLM layer can call the same code. There is no cron, no event storage and no change to the logger allowlist. + +## Architecture Decisions + +| Topic | Choice | Rejected (tradeoff) | +|---|---|---| +| Signature check | Validate the header against `^sha256=[0-9a-f]{64}$`, then `crypto.subtle.sign("HMAC")` over the `arrayBuffer()` body, then `timingSafeEqual`, all before `JSON.parse`. An empty or missing secret is treated as a config error, never as a verify against an empty key | `subtle.verify` (constant-time is not asserted by an existing runtime test); parse-then-verify (spoofing risk) | +| Secret scope | One global `GITHUB_WEBHOOK_SECRET` | Per-team secrets (no UX to reveal them once). Future path: an additive encrypted secret column on `github_org_claims` plus a `/github/webhook/:claimId` route, because the secret must be known before the body is parsed | +| Tenancy for routing | `GithubOrgClaimRepo.findTeamByOrg` is the only cross-team lookup (documented, like `MembershipRepo.findByUser`). Everything after it takes `TeamId` first | Scan links across teams (leak-prone) | +| Claim enforcement | Checked in the use case and enforced again in D1 with a composite FK `(team_id, org_login)` pointing at the claim | Use-case check only | +| Event filtering | Done in the adapter mapper. The domain never sees raw payload shapes | Domain parses JSON (couples the domain to GitHub) | +| Message | Plain text with no `parse_mode`. Title capped at 256 chars and the whole message at 4096 | MarkdownV2 (escaping bugs cause failed sends) | +| Sender | `new Api(BOT_TOKEN)` in composition. No `Bot` and no `PII_KEYRING` on this route | `buildBot()` (a broken keyring would also break alerts) | +| Logging | Existing `LogEvent`. `reason` holds only fixed strings (`ignored:unlinked-repo`, and so on) | New allowlisted fields (repo names are not needed to operate) | + +### GitHub route status policy (mirrors the Telegram RES-001/002 rule: permanent means 2xx, transient infra means 500) + +| Case | Status | +|---|---| +| Signature header missing, malformed or wrong | 401, body never parsed | +| `GITHUB_WEBHOOK_SECRET` unset or empty | 500, logged as a `ConfigError` reason | +| `ping` | 200 | +| Invalid JSON, a non-object, or an unsupported event or action | 200, logged | +| Org not claimed, or repo not linked | 200, `outcome: "ok"` with an `ignored:*` reason | +| Telegram send fails (for example, the topic was deleted) | 200, `errorCode: "AlertSendFailed"` | +| Unexpected error (for example, D1) | 500. GitHub does not auto-retry, so the delivery stays visible for a manual redeliver | + +## Data Flow + +``` +GitHub ─POST /github/webhook─> Hono: raw bytes ─> HMAC check ─401?─┐ + └─> JSON.parse ─> mapGithubEvent (null => 200 ignored) │ + └─> routeGithubEvent(event, deps) │ + claimRepo.findTeamByOrg ─> linkRepo.get(teamId, repo) + ─> teamRepo.get(teamId).chatId ─> formatGithubAlert + ─> AlertSender.send(chatId, threadId, text) ─> 200 +Telegram /linkrepo (in topic) ─> resolveGroupMembership ─> linkRepoToTopic +``` + +## File Changes + +| Path | Action | Description | +|---|---|---| +| `migrations/0002_github_alerts.sql` | Create | Claims and links tables | +| `src/domain/github.ts` | Create | `RepoFullName`, `parseRepoFullName`, `GithubEvent`, `formatGithubAlert` | +| `src/domain/{entities,ports,errors}.ts` | Modify | `RepoTopicLink`; three ports; `InvalidRepoError`, `OrgNotClaimedError`, `AlertSendFailedError` | +| `src/domain/usecases/{link-repo,unlink-repo,list-repo-links,route-github-event}.ts` | Create | Pure use cases | +| `src/adapters/d1/{github-org-claim-repo,repo-topic-link-repo}.ts` | Create | Tenant-scoped SQL | +| `src/adapters/github/{signature,event-mapper}.ts` | Create | HMAC verify and allowlisted field extraction | +| `src/adapters/telegram/alert-sender.ts` | Create | `api.sendMessage` wrapper | +| `src/adapters/telegram/commands.ts` | Modify | `/linkrepo`, `/unlinkrepo`, `/repos` | +| `src/index.ts`, `src/composition.ts`, `src/env.ts` | Modify | Route, `buildGithubRouter(env)`, secret | +| `vitest.config.ts`, `.dev.vars.example` | Modify | Test and example `GITHUB_WEBHOOK_SECRET` | + +```sql +-- 0002_github_alerts.sql (lowercase logins, epoch-ms) +CREATE TABLE github_org_claims ( + org_login TEXT PRIMARY KEY CHECK (org_login = lower(org_login)), -- one org -> one team + team_id TEXT NOT NULL REFERENCES teams(id), + created_at INTEGER NOT NULL, + UNIQUE (team_id, org_login) +); +CREATE TABLE repo_topic_links ( + team_id TEXT NOT NULL, + repo_full_name TEXT NOT NULL CHECK (repo_full_name = lower(repo_full_name)), + org_login TEXT NOT NULL, + thread_id INTEGER NOT NULL, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL, + PRIMARY KEY (team_id, repo_full_name), -- one topic per repo per team + FOREIGN KEY (team_id, org_login) REFERENCES github_org_claims(team_id, org_login) +); +``` + +## Interfaces / Contracts + +```ts +type GithubEvent = { org: string; repo: RepoFullName; kind: "pull_request" | "issues"; + action: "opened" | "closed" | "merged" | "review_requested"; + number: number; title: string; url: string; actor: string; reviewer?: string }; // logins only +interface GithubOrgClaimRepo { + findTeamByOrg(orgLogin: string): Promise; // sole cross-team lookup + isClaimedBy(teamId: TeamId, orgLogin: string): Promise; +} +interface RepoTopicLinkRepo { + get(teamId: TeamId, repo: RepoFullName): Promise; + upsert(teamId: TeamId, link: RepoTopicLink): Promise; // ON CONFLICT DO UPDATE thread_id + remove(teamId: TeamId, repo: RepoFullName): Promise; + list(teamId: TeamId): Promise; +} +interface AlertSender { send(chatId: number, threadId: number, text: string): Promise; } // throws AlertSendFailedError +linkRepoToTopic({ teamId, actorMembershipId, repo, threadId }, deps): Promise<{ repo; previousThreadId: number | null }> +routeGithubEvent(event, deps): Promise<{ kind: "delivered" | "send-failed"; teamId } | { kind: "ignored"; reason: "unclaimed-org" | "unlinked-repo" }> +// alert-sender.ts +await api.sendMessage(chatId, text, { message_thread_id: threadId, link_preview_options: { is_disabled: true } }); +``` + +`merged` means `closed` with `pull_request.merged === true`. `reviewer` is `requested_reviewer.login`, or `requested_team.slug` when a team was requested. Link and unlink need an admin, the thread must not be null (the same refusal as `/datachannel`), and the repo owner must be claimed by the team. + +## Testing Strategy + +| Layer | What | Approach | +|---|---|---| +| Domain (first) | Parsing, admin gate, claim gate, move reply, routing outcomes, truncation | Vitest with fakes in `test/fakes` | +| D1 | Upsert and move, FK rejection of an unclaimed org, two-team isolation | vitest-pool-workers | +| Adapters | HMAC (valid, wrong, same-length wrong, malformed header, empty secret); mapper allowlist (a commit email in the fixture never appears in the output) | Workers runtime | +| HTTP | The status policy table above; `sendMessage` carries `message_thread_id`; console output holds no payload fixture strings | `SELF.fetch` with `vi.stubGlobal(fetch)`, as in `webhook-e2e.test.ts` | + +## Threat Matrix + +| Boundary | Applicability | +|---|---| +| Documentation-like paths, git selection, commit, push, PR commands | N/A: no shell, subprocess, VCS or executable-file boundary | +| HTTP routing (new ingress) | Applicable. The RED tests are the signature and status rows above; they must propagate to tasks | + +## Migration / Rollout + +The migration is additive. Operator steps: + +1. `openssl rand -hex 32`, save it in the password manager, then `npx wrangler secret put GITHUB_WEBHOOK_SECRET`. +2. `npx wrangler d1 migrations apply hack-bot-db --remote`, then deploy. +3. In the GitHub org, open Settings → Webhooks → Add. Payload URL: `https://hack-bot..workers.dev/github/webhook`. Content type: **application/json**. Secret: the same value. Events: Pull requests and Issues. The ping should return 200. +4. Claim the org. First find the team id with `npx wrangler d1 execute hack-bot-db --remote --command "SELECT id, telegram_chat_id FROM teams"`. Then run `npx wrangler d1 execute hack-bot-db --remote --command "INSERT INTO github_org_claims (org_login, team_id, created_at) VALUES (lower(''), '', unixepoch()*1000)"`. +5. Run `/linkrepo /` inside the target topic. + +Rollback: delete the org webhook, or redeploy the previous version. The tables can stay. + +**PR slicing** (feature-branch chain, each PR under 400 lines): + +1. The migration and domain (`github.ts`, ports, errors, four use cases), plus fakes and tests (~350). +2. The D1 repos and their tests (~250). +3. `signature.ts`, the route skeleton (401, 500, ping and malformed JSON), the env and the test binding (~250). +4. The mapper, alert sender, `buildGithubRouter` wiring and the end-to-end delivery tests (~300). +5. The `/linkrepo`, `/unlinkrepo` and `/repos` commands and their tests (~250). This PR must merge before the operator runs step 5. + +## Open Questions + +- [x] Can any team member run `/repos`, or only admins? Resolved: any registered team member, anywhere in the group; only admins can link or unlink. diff --git a/openspec/changes/github-alerts/explore.md b/openspec/changes/github-alerts/explore.md new file mode 100644 index 0000000..931978b --- /dev/null +++ b/openspec/changes/github-alerts/explore.md @@ -0,0 +1,55 @@ +## Exploration: GitHub alerts (org webhook alerts + digest via Cloudflare cron) + +> Mirror of Engram `sdd/github-alerts/explore` (id 2337). Read-only investigation against `main`. +> Note: the product vision (Engram `product/vision`, `product/roadmap`) later changed the delivery target from the team data channel to per-repo linked forum topics. See `proposal.md`. + +### 1. Current architecture / extension points +- Hexagonal Worker. Hono entry `src/index.ts`: single route `POST /telegram/webhook` (checks `X-Telegram-Bot-Api-Secret-Token` via `crypto.subtle.timingSafeEqual`, length-checked first) + `GET /health`. Composition root `src/composition.ts::buildBot(env)` wires D1 repos -> use cases -> grammY bot per request; isolate-cached `FieldCipher` only (no other module state). +- `src/domain/{entities,ports,errors,access-policy,usecases/*}.ts` is framework-free (no grammY/Hono/D1 imports). Every tenant-scoped port method takes `TeamId` first (type-checkable tenancy guard). +- Adapters: `src/adapters/d1/*-repo.ts` (tenant-scoped SQL, one `DB.batch()` per write+audit), `src/adapters/crypto/*` (AES-GCM key ring, AAD = `table|teamId|rowKey|field`), `src/adapters/telegram/{bot,commands,context,team-picker,chat-admin-checker}.ts` (thin grammY edge; `registerCommands(bot, deps)` in commands.ts, each command uses `runCommand` with an explicit `errorReplies` map). +- Schema (`migrations/0001_init.sql`, only migration so far): `teams(id, telegram_chat_id UNIQUE, data_topic_thread_id NULL, created_at)`, `members`, `memberships(UNIQUE(team_id,member_id), UNIQUE(team_id,id))`, `profile_fields` (encrypted except `github_username`, explicitly plaintext "used for integrations in later changes" per pii-protection spec — this change is that integration), `audit_log`, `dm_selections`. +- Team = 1 Telegram supergroup (`teams.telegram_chat_id`). Data channel = a forum topic (`data_topic_thread_id`), bound via admin-only `/datachannel`; alerts/digest should reuse this as delivery target — no new "channel" concept needed. +- `Env` (`src/env.ts`) currently only has `DB, BOT_TOKEN, WEBHOOK_SECRET, PII_KEYRING, BOT_INFO`. No cron trigger configured in `wrangler.jsonc` yet (`triggers.crons` absent), no queues/DOs. +- Test conventions: `test/domain` (pure, fakes), `test/adapters/d1` (vitest-pool-workers + migrations), `test/http` (`SELF.fetch`, e.g. `test/http/webhook-secret.test.ts` pattern for secret/signature tests), `test/runtime-assumptions` for platform behavior checks. Strict TDD, RED first. + +### 2. GitHub webhook integration +- **Org webhook (not GitHub App) recommended for MVP.** One webhook configured once at the GitHub org level delivers all repo events to one endpoint; avoids GitHub App complexity (JWT signing, installation tokens, install flow) which has no UI story yet for a solo/no-dashboard bot. Defer GitHub App to a later change if private-repo API polling or self-serve multi-org install is needed. +- **HMAC-SHA256 verification** (`X-Hub-Signature-256: sha256=`) differs from the Telegram pattern (static token compare): must `crypto.subtle.importKey("raw", secretBytes, {name:"HMAC",hash:"SHA-256"}, false, ["verify"])` then `crypto.subtle.verify` over the **raw body bytes** (verify before `JSON.parse`, mirroring index.ts's "check auth before touching body" order but needs the raw ArrayBuffer, not just a header). +- **Org/repo -> team mapping**: new table, e.g. `github_links(team_id, org_login, repo_full_name NULL /*org-wide*/, created_at)`. +- **Secret storage**: real multi-tenant design wants a secret per team (each team's own org sets its own secret), but building the "create/rotate/reveal-once" UX without a dashboard is nontrivial. MVP recommendation: single global secret via Worker secret (`GITHUB_WEBHOOK_SECRET`), schema still shaped as per-team-capable for a fast follow. Flag as an explicit decision point for `sdd-propose`. +- **First events to support**: `pull_request` (opened, closed/merged) and `issues` (opened, closed) — high value, low noise. Defer `workflow_run`/`check_run`, `release`, `discussion`, `push` (too noisy without filtering). + +### 3. Alert configuration model +- D1: `github_alert_rules(team_id, repo_full_name NULL, event_type, enabled, created_at)`. +- Telegram commands (admin-only, same pattern as `/datachannel`'s `UnauthorizedError` reuse): `/githubrepo add|remove `, `/alerts on|off `. Refuse enabling alerts until the team's data channel is bound (mirrors existing `/datachannel` gating). +- Delivery via `bot.api.sendMessage(team.chatId, text, {message_thread_id: team.dataTopicThreadId})` — reuses the pattern already used by `chatAdminChecker` (`bot.api` usable outside the webhook update context). + +### 4. Digest +- Needs `wrangler.jsonc` `"triggers": {"crons": [...]}` and an exported `scheduled(event, env, ctx)` handler alongside the Hono `fetch` export (Hono's `app.fetch` doesn't cover `scheduled`; export `{fetch: app.fetch, scheduled}` from `src/index.ts`). +- Aggregate from **stored events**, not live GitHub API queries (avoids GitHub rate limits, keeps one source of truth). Needs a `github_events` table populated by the webhook handler at ingest time. +- True per-team schedule isn't possible with Workers cron (one Worker-wide schedule); MVP = single global cron (e.g. hourly) that checks each team's configured `digest_hour_utc` and only sends when due. Defer arbitrary per-team cron granularity. +- Idempotency: conditional D1 update pattern already used for `changeRole`'s "last admin" guard (`UPDATE ... SET last_digest_at=? WHERE team_id=? AND last_digest_atrule matching), `/github/webhook` HTTP route with HMAC verify (single global secret), admin commands `/githubrepo`, `/alerts`, dispatch to data channel. 2 events only (`pull_request`, `issues`). +- Slice 2 (separate PR/possibly separate change): `github_events` table, `scheduled` handler, `wrangler.jsonc` cron config, idempotency guard, single global daily digest time (no per-team schedule yet). +- Defer: GitHub App/install flow, per-team webhook secrets, CI/workflow alerts, release alerts, per-team cron schedule, event coalescing, retention job. + +### PR breakdown (400-line review budget each) +1. Migration + domain entities/ports/pure use cases for link+rule config, tests (~250-350 lines). +2. D1 adapters for the two new repos + vitest-pool-workers tests (~200-300 lines). +3. `/github/webhook` route + HMAC WebCrypto verification + composition wiring + signature tests (~200-250 lines). +4. Alert dispatch use case (event -> matching rules -> Telegram send) + wiring + tests (~200-300 lines). +5. Admin commands `/githubrepo`, `/alerts` in `commands.ts` + tests (~150-250 lines). +6. (Digest slice, likely its own change/PR set) `github_events` table, `scheduled` handler, cron config, idempotency tests (~250-350 lines). + +**Where**: `src/index.ts`, `src/composition.ts`, `src/domain/{entities,ports,usecases}.ts`, `src/adapters/d1/*`, `src/adapters/telegram/commands.ts`, `migrations/0001_init.sql` (reference only, new migration needed), `wrangler.jsonc`, `openspec/specs/{telegram-webhook,pii-protection}/spec.md`, `openspec/changes/archive/2026-09-23-team-foundation/design.md`. diff --git a/openspec/changes/github-alerts/proposal.md b/openspec/changes/github-alerts/proposal.md new file mode 100644 index 0000000..823095d --- /dev/null +++ b/openspec/changes/github-alerts/proposal.md @@ -0,0 +1,84 @@ +# Proposal: GitHub Alerts Routed to Linked Forum Topics + +## Intent + +Right now PR and issue activity goes unseen unless someone checks GitHub. The team runs one Telegram forum topic per project, so each repo's alerts should land in that project's topic. This is roadmap change 2, and it should reach production fast. + +## Scope + +### In Scope +- `POST /github/webhook` verified with HMAC-SHA256 over the raw body, using one global Worker secret `GITHUB_WEBHOOK_SECRET` +- **Org claim**: each GitHub org is bound to exactly one team through a D1 row, created once by the operator. Links and deliveries are accepted only for repos of an org the team has claimed. +- `/linkrepo ` and `/unlinkrepo `, run inside a topic by team admins only. `/repos` lists the current links. +- A repo belongs to at most one topic per team. A topic can hold many repos. Re-linking moves the repo to the new topic and says so in the reply. +- **Unlinked repos are ignored.** The event is acknowledged and dropped. There is no fallback to the data channel. +- Default events (the user can adjust these): `pull_request` opened, closed/merged and review_requested; `issues` opened and closed +- A short alert message, truncated to Telegram's limit, built from repo, action, actor login, number, title and URL only. Payloads are never stored or logged. + +### Out of Scope (deferred) +- Digest and cron, event storage and retention +- Per-team secrets, GitHub App, self-serve org claim +- CI, release, push and discussion events; per-repo event toggles; coalescing and rate-limit batching; `X-GitHub-Delivery` dedupe +- Audit rows for link changes +- Natural-language linking (roadmap change 4 will reuse these use cases) + +**Why unlinked repos are ignored**: the data channel is where member PII is shown. Sending every repo's traffic there as a fallback would bury that data and push noise into a topic with a different purpose. Explicit opt-in per repo matches the team's "one topic per thing" model and keeps alerts predictable. + +## Capabilities + +### New Capabilities +- `github-webhook`: signature verification, ping handling, event and action filtering, org-claim check +- `repo-topic-links`: org claim, link, unlink and list, admin-only permission, one topic per repo +- `github-alerts`: routing events to the linked topic, message format, PII-minimal fields, delivery-failure handling + +### Modified Capabilities +- None + +## Approach + +Use the existing hexagonal design. The domain gets use cases with no Telegram parsing: `linkRepoToTopic(teamId, actor, repo, threadId)`, `unlinkRepo`, `listRepoLinks` and `routeGithubEvent(event)`. That keeps them callable from a future LLM layer. A new additive migration adds `github_org_claims` and `repo_topic_links`, both keyed by `team_id`. The HTTP adapter verifies the signature before it parses the body. Delivery goes through `bot.api.sendMessage` with `message_thread_id`. After a valid signature the route returns 2xx, except for unexpected infrastructure failures (e.g. D1), which return 500 so the delivery shows as failed in GitHub and can be redelivered. Send failures are logged by reason only. + +## Affected Areas + +| Area | Impact | Description | +|------|--------|-------------| +| `migrations/0002_*.sql` | New | Claims and links tables | +| `src/domain/` | Modified | Entities, ports, use cases | +| `src/adapters/d1/`, `src/adapters/github/` | New | Repos, HMAC verifier, payload mapper | +| `src/adapters/telegram/commands.ts` | Modified | Three commands | +| `src/index.ts`, `src/composition.ts`, `src/env.ts` | Modified | Route, wiring, secret | + +## Risks + +| Risk | Likelihood | Mitigation | +|------|------------|------------| +| Another group links private repos | High without claim | Org-claim check at link time and at delivery | +| Spoofed webhook | Med | HMAC over raw bytes, constant-time compare | +| Chat spam or rate limits | Low | Narrow default events | +| Topic deleted, sends fail | Med | Log, return 2xx, admin re-links | + +## Rollback Plan + +Remove the org webhook in GitHub settings, or redeploy the previous Worker. The migration only adds tables. Drop them only after exporting the data. + +## Dependencies + +- Org-level webhook configured by the operator, the Worker secret, and a one-time claim row (config/data, not code) + +## Success Criteria + +- [ ] Unsigned or wrongly signed requests are rejected before parsing +- [ ] A PR opened on a linked repo appears in its topic +- [ ] Unlinked or unclaimed-org events send nothing +- [ ] Non-admins cannot link or unlink +- [ ] Domain use cases have no grammY or Hono imports + +## Proposal question round + +Assumptions to review: one team, one org, admin-only linking, and the org claim is an operator step. + +## Open questions for the user + +1. **Should unlinked repos stay silent?** Recommended: yes. Opt in per repo. +2. **Is a one-time operator SQL step acceptable for claiming the org?** Recommended: yes for now, and a `/claimorg` command with ownership proof later. +3. **Should review_requested alerts mention the reviewer's Telegram handle, using `github_username`?** Recommended: not in this slice. Plain logins only. diff --git a/openspec/changes/github-alerts/specs/github-alerts/spec.md b/openspec/changes/github-alerts/specs/github-alerts/spec.md new file mode 100644 index 0000000..497bde4 --- /dev/null +++ b/openspec/changes/github-alerts/specs/github-alerts/spec.md @@ -0,0 +1,64 @@ +# GitHub Alerts Specification + +## Purpose + +Routes supported GitHub events for linked repos to their linked forum topic as a minimal-field alert message, without storing or logging raw webhook payloads. + +## Requirements + +### Requirement: Route Alert to the Linked Topic Only + +The system MUST deliver an alert only to the forum topic linked to the event's repo for the claiming team. Events for repos with no link, or whose org has no claim, MUST produce no alert. + +#### Scenario: Linked repo produces an alert + +- GIVEN `owner/repo` is linked to a topic for the claiming team +- WHEN a supported event/action fires for `owner/repo` +- THEN the system sends one alert message to that topic + +#### Scenario: Unlinked repo produces no alert + +- GIVEN `owner/repo` has no link for any team, or its org is unclaimed +- WHEN a supported event/action fires for `owner/repo` +- THEN the system MUST NOT send any message +- AND MUST NOT fall back to any other channel + +### Requirement: Allowlisted Fields Only, No Payload Storage or Logging + +The system MUST build the alert message using only repo full name, action, actor login, number, title, and URL, and MUST NOT persist or log the raw webhook payload. + +#### Scenario: Alert contains only allowed fields + +- GIVEN a supported `pull_request` event for a linked repo +- WHEN the alert message is built +- THEN it contains only repo, action, actor login, number, title, and URL +- AND contains no other payload fields (e.g. no commit emails, no diff content) + +#### Scenario: Processing error does not log the payload + +- GIVEN an error occurs while handling a webhook event +- WHEN the system logs the error +- THEN the log entry MUST NOT contain the raw request body or any payload field values + +### Requirement: Message Truncated to Telegram's Limit + +The system MUST truncate the built alert message so it never exceeds Telegram's 4096-character limit before sending. + +#### Scenario: Long title is truncated + +- GIVEN an issue or PR title long enough that the built message would exceed 4096 characters +- WHEN the alert message is built +- THEN the system truncates it so the final message is at most 4096 characters +- AND the message remains a valid, sendable text + +### Requirement: Delivery Failure Is Logged and Acknowledged + +The system MUST log a delivery failure (e.g. the linked topic was deleted) by reason only, without the payload, and MUST still return a 2xx response for the webhook request. + +#### Scenario: sendMessage fails because the topic was deleted + +- GIVEN a repo is linked to a topic that has since been deleted +- WHEN a supported event fires and delivery to that topic fails +- THEN the system logs the failure reason only +- AND the webhook HTTP response is still 2xx +- AND no retry is attempted within the same request diff --git a/openspec/changes/github-alerts/specs/github-webhook/spec.md b/openspec/changes/github-alerts/specs/github-webhook/spec.md new file mode 100644 index 0000000..414a06f --- /dev/null +++ b/openspec/changes/github-alerts/specs/github-webhook/spec.md @@ -0,0 +1,77 @@ +# GitHub Webhook Specification + +## Purpose + +Verifies that inbound GitHub webhook deliveries are authentic and filters them to the supported event/action set before any org, team, or repo routing happens. + +## Requirements + +### Requirement: HMAC Signature Verification Over Raw Body + +The system MUST verify the `X-Hub-Signature-256` header via HMAC-SHA256 over the raw request body bytes, computed with the global Worker secret `GITHUB_WEBHOOK_SECRET`, using a constant-time comparison, and MUST perform this check before parsing the body as JSON. + +#### Scenario: Missing signature header + +- GIVEN a webhook POST arrives with no `X-Hub-Signature-256` header +- WHEN the request is received +- THEN the system MUST reject it before parsing the body +- AND MUST NOT process any event + +#### Scenario: Wrong signature + +- GIVEN a webhook POST arrives with an `X-Hub-Signature-256` header computed from a different secret +- WHEN the signature is verified against the raw body +- THEN the system MUST reject the request before parsing the body + +#### Scenario: Right-length but wrong signature + +- GIVEN a webhook POST arrives with a signature of the correct hex length but incorrect content +- WHEN the signature is verified using constant-time comparison +- THEN the system MUST reject the request +- AND the rejection MUST NOT leak timing information distinguishing this case from a random wrong-length signature + +#### Scenario: Valid signature + +- GIVEN a webhook POST arrives with a signature matching the raw body under `GITHUB_WEBHOOK_SECRET` +- WHEN the signature is verified +- THEN the system parses the body and continues processing + +### Requirement: Ping Event Acknowledged + +The system MUST respond with a 2xx status to a `ping` event, once signature-verified, without further processing. + +#### Scenario: GitHub sends a ping + +- GIVEN a signature-verified webhook delivery with event type `ping` +- WHEN the request is processed +- THEN the system MUST return a 2xx response +- AND MUST NOT attempt org, repo, or alert routing + +### Requirement: Unsupported Event or Action Ignored + +The system MUST acknowledge with a 2xx response and drop, without producing an alert, any event/action combination outside the supported set (`pull_request` opened, closed, review_requested; `issues` opened, closed). + +#### Scenario: Unsupported event type + +- GIVEN a signature-verified webhook delivery with an event type outside the supported set (e.g. `push`) +- WHEN the request is processed +- THEN the system MUST return a 2xx response +- AND MUST NOT produce an alert + +#### Scenario: Supported event, unsupported action + +- GIVEN a signature-verified `pull_request` event with an action outside the supported set (e.g. `labeled`) +- WHEN the request is processed +- THEN the system MUST return a 2xx response +- AND MUST NOT produce an alert + +### Requirement: Infrastructure Failures Return 500 + +After a valid signature, the system MUST return a 500 response when an unexpected infrastructure failure (e.g. a D1 error) prevents routing, and MUST log it by error name only. This leaves the delivery marked as failed in GitHub so an operator can redeliver it manually. A Telegram delivery failure (e.g. the linked topic was deleted) is NOT an infrastructure failure: it follows the delivery-failure requirement and still returns 2xx, because redelivering cannot fix it. + +#### Scenario: D1 is unavailable during routing + +- GIVEN a signature-verified supported event +- WHEN reading the org claim or repo link fails with an unexpected error +- THEN the system MUST return a 500 response +- AND MUST log the failure by error name, without the payload diff --git a/openspec/changes/github-alerts/specs/repo-topic-links/spec.md b/openspec/changes/github-alerts/specs/repo-topic-links/spec.md new file mode 100644 index 0000000..4dba7dc --- /dev/null +++ b/openspec/changes/github-alerts/specs/repo-topic-links/spec.md @@ -0,0 +1,89 @@ +# Repo-Topic Links Specification + +## Purpose + +Manages the org claim and the per-team mapping of GitHub repos to forum topics, enforcing that only claimed orgs can be linked and that link mutations are admin-only. + +## Requirements + +### Requirement: Org Claim Required for Linking + +The system MUST allow linking a repo to a topic only if the repo's GitHub org has a `github_org_claims` row binding it to the requesting team. The claim row MUST be created by a one-time operator D1 step; no in-product command creates it in this change. + +#### Scenario: Claimed org repo can be linked + +- GIVEN the team's org has a `github_org_claims` row for `owner` +- WHEN a team admin runs `/linkrepo owner/repo` inside a topic +- THEN the system creates the link between `owner/repo` and that topic + +#### Scenario: Unclaimed org repo is rejected + +- GIVEN no `github_org_claims` row exists for `owner` +- WHEN a team admin runs `/linkrepo owner/repo` inside a topic +- THEN the system MUST refuse to create the link +- AND MUST NOT store any row for that repo + +### Requirement: Admin-Only Link/Unlink Inside a Topic + +The system MUST allow `/linkrepo` and `/unlinkrepo` only when run by a team admin inside a forum topic, and MUST refuse otherwise. + +#### Scenario: Admin runs /linkrepo inside a topic + +- GIVEN the caller is a team admin +- WHEN they run `/linkrepo owner/repo` inside a forum topic +- THEN the system processes the link request + +#### Scenario: Non-admin attempts to link or unlink + +- GIVEN the caller is not a team admin +- WHEN they run `/linkrepo` or `/unlinkrepo` inside a topic +- THEN the system MUST refuse +- AND MUST NOT change any stored link + +#### Scenario: Admin runs the commands outside a topic + +- GIVEN the caller is a team admin +- WHEN they run `/linkrepo` or `/unlinkrepo` in the group's general chat (not inside a topic) +- THEN the system MUST refuse +- AND MUST instruct the admin to run it inside the intended topic + +### Requirement: One Topic Per Repo, Re-Link Moves It + +The system MUST allow a repo to be linked to at most one topic per team. Linking an already-linked repo to a different topic MUST move the mapping and MUST tell the admin the previous topic is no longer receiving alerts for that repo. + +#### Scenario: First link + +- GIVEN `owner/repo` has no existing link for the team +- WHEN an admin runs `/linkrepo owner/repo` inside topic A +- THEN the system creates a link from `owner/repo` to topic A + +#### Scenario: Re-link moves the repo + +- GIVEN `owner/repo` is linked to topic A for the team +- WHEN an admin runs `/linkrepo owner/repo` inside topic B +- THEN the system updates the link to point to topic B +- AND the reply states the repo moved from topic A to topic B + +### Requirement: Any Member Lists the Team's Claimed-Org Links + +The system MUST allow any registered member of the team to run `/repos` anywhere in the team's group (general chat or any topic), and MUST refuse non-members. Listing is read-only. `/repos` MUST list only links for repos belonging to orgs the team has claimed. + +#### Scenario: Non-admin member lists links + +- GIVEN the caller is a registered team member who is not an admin +- WHEN they run `/repos` in the group's general chat +- THEN the reply lists the team's links +- AND no stored link changes + +#### Scenario: Non-member runs /repos + +- GIVEN the caller is not a registered member of the team +- WHEN they run `/repos` in the group +- THEN the system MUST refuse + +#### Scenario: List reflects current links + +- GIVEN the team has links for `owner/repo-a` and `owner/repo-b` +- WHEN a team member runs `/repos` +- THEN the reply lists both repos and their linked topics +- AND excludes any link that no longer has a matching org claim diff --git a/openspec/changes/github-alerts/tasks.md b/openspec/changes/github-alerts/tasks.md new file mode 100644 index 0000000..09b0f9d --- /dev/null +++ b/openspec/changes/github-alerts/tasks.md @@ -0,0 +1,72 @@ +# Tasks: GitHub Alerts Routed to Linked Forum Topics + +## Review Workload Forecast + +| Field | Value | +|-------|-------| +| Estimated changed lines | ~1400 (migration, domain, D1, HMAC, HTTP, Telegram, tests) | +| 400-line budget risk | Medium (aggregate); each PR individually Low | +| Chained PRs recommended | Yes | +| Suggested split | PR1 domain → PR2 D1 → PR3 route skeleton → PR4 delivery wiring → PR5 commands | +| Delivery strategy | ask-on-risk | +| Chain strategy | stacked-to-main | + +Decision needed before apply: No (resolved — stacked-to-main) +Chained PRs recommended: Yes +Chain strategy: stacked-to-main +400-line budget risk: Medium + +### Suggested Work Units + +| Unit | Goal | Likely PR | Focused test command | Runtime harness | Rollback boundary | +|------|------|-----------|----------------------|-----------------|-------------------| +| 1 | Migration + domain (types, ports, errors, 4 use cases) with fakes | PR1 (~350) | `npm test -- test/domain` | N/A — pure Vitest | delete `src/domain/github.ts`, new usecases, `0002_*.sql` | +| 2 | D1 repos: claims, links, FK/isolation tests | PR2 (~250) | `npm test -- test/adapters/d1` | vitest-pool-workers D1 | delete `src/adapters/d1/{github-org-claim-repo,repo-topic-link-repo}.ts` | +| 3 | `signature.ts` + route skeleton (401/500/ping/malformed JSON) + env binding | PR3 (~250) | `npm test -- test/http/github-webhook.test.ts` | `SELF.fetch` (`webhook-e2e.test.ts` pattern) | delete `/github/webhook` route registration | +| 4 | Mapper, alert sender, `buildGithubRouter`, end-to-end delivery + 500-on-D1-failure tests | PR4 (~300) | `npm test -- test/http` | `SELF.fetch` + `vi.stubGlobal(fetch)` | delete `src/adapters/github/event-mapper.ts`, `src/adapters/telegram/alert-sender.ts` | +| 5 | `/linkrepo`, `/unlinkrepo`, `/repos` commands + tests | PR5 (~250) | `npm test -- test/adapters/telegram/commands.test.ts` | grammY stub, `SELF.fetch` | revert `commands.ts` command registration | + +## Phase 1: Domain Foundation (PR1) + +- [ ] 1.1 RED: `github.ts` — `parseRepoFullName` (lowercase, `owner/repo` shape), `formatGithubAlert` truncation at 4096 (spec: Message Truncated). +- [ ] 1.2 GREEN: `src/domain/github.ts` types, `RepoFullName`, `GithubEvent`, `formatGithubAlert`. +- [ ] 1.3 Add `RepoTopicLink` entity; `GithubOrgClaimRepo`, `RepoTopicLinkRepo`, `AlertSender` ports; `InvalidRepoError`, `OrgNotClaimedError`, `AlertSendFailedError` in `entities.ts`/`ports.ts`/`errors.ts`. +- [ ] 1.4 RED: `link-repo-to-topic` tests — claimed org links, unclaimed org rejected, admin-only, must be inside a topic, re-link moves and reply names old/new topic (spec: repo-topic-links, all "Requirement" scenarios). +- [ ] 1.5 GREEN: `src/domain/usecases/link-repo.ts`. +- [ ] 1.6 RED/GREEN: `unlink-repo.ts`, `list-repo-links.ts` (spec: any member reads, excludes unclaimed-org links). +- [ ] 1.7 RED: `route-github-event` — unclaimed org ignored, unlinked repo ignored (no fallback), linked repo delivers, send failure returns `send-failed` kind not thrown (spec: github-alerts Route/Delivery-Failure). +- [ ] 1.8 GREEN: `src/domain/usecases/route-github-event.ts`. +- [ ] 1.9 `migrations/0002_github_alerts.sql` per design (claims + links, composite FK). + +## Phase 2: D1 Adapters (PR2) + +- [ ] 2.1 RED: migration test — table/FK/UNIQUE/CHECK enforcement, cross-team isolation on links. +- [ ] 2.2 GREEN: `src/adapters/d1/github-org-claim-repo.ts`, `repo-topic-link-repo.ts` (upsert `ON CONFLICT DO UPDATE thread_id`). + +## Phase 3: Signature and Route Skeleton (PR3) + +- [ ] 3.1 RED: HMAC tests — missing header, wrong secret, right-length-wrong-content (timing-safe), valid signature (spec: HMAC Signature Verification, all scenarios). +- [ ] 3.2 GREEN: `src/adapters/github/signature.ts`, WebCrypto HMAC + `timingSafeEqual`, empty/missing secret → `ConfigError` (500), before `JSON.parse`. +- [ ] 3.3 RED: route status tests — `ping` 200, invalid JSON/non-object/unsupported event 200, D1 failure 500 (spec: Ping, Unsupported, Infrastructure Failures). +- [ ] 3.4 GREEN: `src/index.ts` route registration, `env.ts` `GITHUB_WEBHOOK_SECRET`, `.dev.vars.example`, `vitest.config.ts` test binding. + +## Phase 4: Delivery Wiring (PR4) + +- [ ] 4.1 RED: mapper allowlist test — commit email fixture never appears in mapped `GithubEvent`; `merged` derived from `closed`+`pull_request.merged`; `reviewer` from login/team slug. +- [ ] 4.2 GREEN: `src/adapters/github/event-mapper.ts`. +- [ ] 4.3 RED: `AlertSender` test — `sendMessage` carries `message_thread_id`; send failure surfaces as `AlertSendFailedError`, not thrown to caller. +- [ ] 4.4 GREEN: `src/adapters/telegram/alert-sender.ts` (`new Api(BOT_TOKEN)`, no `Bot`/`PII_KEYRING`). +- [ ] 4.5 GREEN: `composition.ts` `buildGithubRouter(env)` wiring deps end to end. +- [ ] 4.6 RED: e2e — linked repo alert delivered; unlinked/unclaimed silent; send failure logs reason-only and returns 2xx; log output has no payload fixture strings (spec: Delivery Failure, Allowlisted Fields). + +## Phase 5: Link Commands (PR5) + +- [ ] 5.1 RED: `/linkrepo`/`/unlinkrepo` — admin-only, must be inside a topic (refuse in general chat with instruction), re-link reply names both topics (spec: Admin-Only Link/Unlink, One Topic Per Repo). +- [ ] 5.2 RED: `/repos` — any registered member anywhere in the group, read-only, non-member refused, excludes unclaimed-org links. +- [ ] 5.3 GREEN: wire all three commands in `src/adapters/telegram/commands.ts`. + +## Phase 6: Operator Rollout + +- [ ] 6.1 (after PR3 merges) `npx wrangler secret put GITHUB_WEBHOOK_SECRET`. +- [ ] 6.2 (after PR4 merges) Configure org webhook: content type `application/json`, same secret, Pull requests + Issues events; verify ping returns 200. +- [ ] 6.3 (after PR1 merges, before PR5's `/linkrepo` is used) Claim the org via `wrangler d1 execute` insert into `github_org_claims`.