Skip to content

Commit 1ad6048

Browse files
chrisleekrclaude
andauthored
feat(core): extract HTTP router and add output-safety primitives
Foundation layer for the repo-config and isolated-workflow-runner work that follows in this stack. No behaviour change to dispatch or the pipeline. - Extract the fetch handler out of `src/app.ts` into `src/http-router.ts`, so route wiring is testable without booting the server process. - Add `src/utils/bot-identity.ts` and `src/utils/github-actor.ts` to give the bot a single identity/actor resolution point instead of scattered literals. - Add `src/utils/exact-credential-redaction.ts` and widen the redaction paths in `src/utils/log-redaction.ts`. - Add `src/mcp/servers/inline-comment-dedup.ts` so repeated inline review comments collapse instead of stacking on re-runs. - Add `src/db/postgres-error.ts` to classify Postgres failures at one place. - Extend the LLM output scanner and sanitizer coverage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KUPpJPtxAaHWrBsjytRGyM
1 parent e2fea3f commit 1ad6048

40 files changed

Lines changed: 2326 additions & 366 deletions

docs/build/conventions.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,14 +78,15 @@ bun run check
7878

7979
## CI pipeline
8080

81-
Four workflow files form the pipeline; each owns one responsibility.
81+
Five pipeline files; each owns one responsibility.
8282

8383
| Workflow | Trigger | Owns |
8484
| -------------------------------------- | --------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
8585
| `.github/workflows/ci.yml` | `pull_request` + `push: main` + `workflow_call` | Quality gates only: typecheck, lint, format, audit:ci, test, build |
8686
| `.github/workflows/secrets-scan.yml` | `push: branches-ignore: [gh-pages]` + `workflow_dispatch` | Standalone gitleaks scan, decoupled so every push (incl. chore/docs) is gated |
8787
| `.github/workflows/release-please.yml` | `push: [main, beta]` | release-please maintains a Release PR per branch; merging it cuts the release then calls `docker-build.yml`, and on a stable `main` release dispatches `github-app-released` to `chrisleekr/helm-charts` to open the chart-sync PR |
88-
| `.github/workflows/docker-build.yml` | `workflow_call` + `workflow_dispatch` | Reusable image builder: matrix split-and-merge (amd64 on `ubuntu-24.04`, arm64 on `ubuntu-24.04-arm`), Trivy scan |
88+
| `.github/workflows/docker-build.yml` | `workflow_call` + `workflow_dispatch` | Reusable image builder: matrix split-and-merge (amd64 on `ubuntu-24.04`, arm64 on `ubuntu-24.04-arm`), Trivy scan. Tags each variant `<version>-<variant>` plus a mutable `latest-<variant>` on prod releases |
89+
| `.gitlab-ci.yml` | every branch (gates) + `main` (images) | GitLab CI: the same quality gates on every branch, then `latest-orchestrator` / `latest-daemon` to the GitLab container registry on `main` |
8990

9091
Notes:
9192

docs/build/extending.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ Two extension points in this codebase: workflow handlers and MCP servers. Both f
44

55
## Adding a workflow
66

7-
A workflow is a verb the bot performs on a target (issue or PR). Six are registered today; adding a seventh is appending one entry to `src/workflows/registry.ts` plus a handler file.
7+
A workflow is a verb the bot performs on a target (issue or PR). Seven are registered today; adding an eighth is appending one entry to `src/workflows/registry.ts` plus a handler file.
88

99
### Step 1: write the handler
1010

@@ -25,14 +25,15 @@ export type WorkflowHandler = (ctx: WorkflowRunContext) => Promise<HandlerResult
2525
| `logger` | `pino.Logger` | Structured logging. |
2626
| `octokit` | `Octokit` | API client with installation token. |
2727
| `deliveryId` | string \| null | Webhook delivery id for tracing. |
28-
| `daemonId` | string | Daemon process id. |
28+
| `daemonId` | string | Stable isolated-runner identity. |
2929
| `setState(state, humanMessage)` | function | Persist partial state mid-execution. |
3030

3131
`HandlerResult` is a discriminated union:
3232

3333
```typescript
3434
| { status: "succeeded"; state: unknown; humanMessage?: string }
3535
| { status: "failed"; reason: string; state?: unknown; humanMessage?: string }
36+
| { status: "incomplete"; reason: string; state?: unknown; humanMessage?: string }
3637
| { status: "handed-off"; state?: unknown; humanMessage?: string; childRunId: string }
3738
```
3839

@@ -70,7 +71,7 @@ If the workflow should be reachable via mentions, extend the system prompt in `s
7071

7172
### Step 4: document and test
7273

73-
- Add `docs/use/workflows/<name>.md` matching the template used by the six built-ins.
74+
- Add `docs/use/workflows/<name>.md` matching the template used by the seven built-ins.
7475
- Add `test/workflows/handlers/<name>.test.ts` covering the happy path and one failure mode. Integration via `test/workflows/dispatcher.test.ts` is automatic: if the registry entry is valid, dispatch works.
7576
- The `check:docs-sync` script in CI fails any PR that touches `src/workflows/**` without updating the workflow docs tree.
7677

@@ -201,4 +202,4 @@ If your extension reacts to a GitHub event the bot does not yet handle (e.g. `pu
201202
2. **Add a webhook handler** in `src/webhook/events/<event>.ts` that parses the payload and dispatches via `dispatchByLabel` (label path) or `dispatchByIntent` (comment path). Webhook handlers must return within 10 s, fire `processRequest` with fire-and-forget semantics.
202203
3. **Register the event handler** in `src/app.ts` alongside the existing `app.webhooks.on(...)` calls.
203204

204-
Webhook handlers do **not** run business logic, they parse the event, build a `BotContext`, and dispatch. All bot work happens in workflow handlers, called from the daemon.
205+
Webhook handlers do **not** run business logic, they parse the event, build a `BotContext`, and dispatch. Structured workflow handlers execute in one-attempt runner Pods. Legacy direct and scoped jobs execute on shared daemons.

docs/index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ hide:
55

66
# GitHub App
77

8-
A GitHub App that responds to `@chrisleekr-bot` mentions on pull requests and issues, powered by the Claude Agent SDK. Every webhook is acknowledged in under ten seconds and handed to the daemon fleet over WebSocket; when triage flags the job as heavy or the queue backs up, the orchestrator spawns an ephemeral daemon Pod on Kubernetes so the same image scales on demand.
8+
A GitHub App that responds to `@chrisleekr-bot` mentions on pull requests and issues, powered by the Claude Agent SDK. Every webhook is acknowledged in under ten seconds. Structured workflows run in one-attempt Kubernetes Pods; legacy direct and scoped jobs use the shared daemon fleet, which can add ephemeral Pods for heavy work or queue overflow.
99

1010
## Three doors
1111

docs/operate/github-app.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ Direct link: <https://github.com/settings/apps/new>.
3030
| SSL verification | Enabled (default, keep it). |
3131
| Webhook secret | Output of `openssl rand -hex 32`. Save as `GITHUB_WEBHOOK_SECRET`. |
3232

33-
The path `/api/github` is set by `pathPrefix` in `createNodeMiddleware` (`src/app.ts`). Don't change the path unless you also change the source.
33+
The path `/api/github/webhooks` is the `WEBHOOK_PATH` constant in `src/http-router.ts`, passed as the `path` option to `createWebMiddleware` in `src/app.ts`. The router matches it explicitly before delegating, so changing it means changing both. Don't change the path unless you also change the source.
3434

3535
For local dev, set the Webhook URL to a [smee.io](https://smee.io) channel and relay it to your local server:
3636

src/ai/llm-client.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ export interface LLMCreateParams {
143143
readonly messages: readonly LLMMessage[];
144144
/** Hard upper bound on generated tokens; prevents runaway cost on malformed responses. */
145145
readonly maxTokens: number;
146-
/** Optional temperature; default 0 for classification determinism. */
146+
/** Passed through only when set. Claude 5 models reject the field. */
147147
readonly temperature?: number;
148148
}
149149

@@ -329,7 +329,10 @@ export function buildRequest(
329329
model: p.model,
330330
max_tokens: p.maxTokens,
331331
messages: p.messages.map((m) => ({ role: m.role, content: m.content })),
332-
temperature: p.temperature ?? 0,
332+
// Only sent when a caller asks for it. Claude 5 models reject the field
333+
// outright ("`temperature` is deprecated for this model", 400), so
334+
// defaulting it here made every single-turn call fail against them.
335+
...(p.temperature !== undefined ? { temperature: p.temperature } : {}),
333336
};
334337
if (system !== undefined) base["system"] = system;
335338
return base;
@@ -345,7 +348,10 @@ export function buildRichRequest(
345348
model: p.model,
346349
max_tokens: p.maxTokens,
347350
messages: p.messages.map((m) => ({ role: m.role, content: m.content })),
348-
temperature: p.temperature ?? 0,
351+
// Only sent when a caller asks for it. Claude 5 models reject the field
352+
// outright ("`temperature` is deprecated for this model", 400), so
353+
// defaulting it here made every single-turn call fail against them.
354+
...(p.temperature !== undefined ? { temperature: p.temperature } : {}),
349355
};
350356
if (system !== undefined) base["system"] = system;
351357
if (p.tools !== undefined && p.tools.length > 0) {

src/app-log-fields.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ export const HTTP_LOG_EVENTS = {
3131
schedulerRunRejectedPayload: "http.scheduler.run.rejected_payload",
3232
schedulerRunEnqueued: "http.scheduler.run.enqueued",
3333
schedulerRunFailed: "http.scheduler.run.failed",
34+
requestFailed: "http.request.failed",
3435
} as const;
3536

3637
/**
@@ -135,6 +136,19 @@ export const HttpLogFieldsSchema = z.discriminatedUnion("event", [
135136
err: z.unknown(),
136137
})
137138
.strict(),
139+
/**
140+
* Error: the request router itself threw, caught by the `Bun.serve` `error`
141+
* hook. Distinct from `schedulerRunFailed`, which is one endpoint's own catch:
142+
* this is the last-resort boundary for any route. Carries no request data, an
143+
* unhandled throw may have been triggered by attacker-shaped input, so only
144+
* the secret-scrubbed `err` is recorded.
145+
*/
146+
z
147+
.object({
148+
event: z.literal(HTTP_LOG_EVENTS.requestFailed),
149+
err: z.unknown(),
150+
})
151+
.strict(),
138152
]);
139153

140154
export type HttpLogFields = z.infer<typeof HttpLogFieldsSchema>;

0 commit comments

Comments
 (0)