Skip to content

feat(core): extract inline pipeline and add database foundation for dual-mode dispatch - #13

Merged
chrisleekr merged 3 commits into
mainfrom
feat/make-foundation-for-dual-mode
Apr 13, 2026
Merged

feat(core): extract inline pipeline and add database foundation for dual-mode dispatch#13
chrisleekr merged 3 commits into
mainfrom
feat/make-foundation-for-dual-mode

Conversation

@chrisleekr

@chrisleekr chrisleekr commented Apr 13, 2026

Copy link
Copy Markdown
Owner

Description

Lays the groundwork for dual-mode dispatch (inline vs daemon orchestration) by extracting the execution pipeline from the router and adding the database + config foundation — all behind feature flags that default to inline, preserving zero-change behaviour for existing deployments.

Before

flowchart LR
    classDef routerNode fill:#1E3A5F,color:#FFFFFF
    classDef stepNode fill:#2D6A4F,color:#FFFFFF

    WH["Webhook Event"]:::routerNode --> RT["router.ts<br/>idempotency + auth +<br/>concurrency + pipeline"]:::routerNode
    RT --> S1["Create Tracking Comment"]:::stepNode
    S1 --> S2["Fetch GraphQL"]:::stepNode
    S2 --> S3["Build Prompt"]:::stepNode
    S3 --> S4["Clone Repo"]:::stepNode
    S4 --> S5["Execute Claude Agent"]:::stepNode
    S5 --> S6["Finalize Comment"]:::stepNode
Loading

After

flowchart LR
    classDef routerNode fill:#1E3A5F,color:#FFFFFF
    classDef pipelineNode fill:#2D6A4F,color:#FFFFFF
    classDef dbNode fill:#7B2D8B,color:#FFFFFF
    classDef configNode fill:#8B4513,color:#FFFFFF

    WH["Webhook Event"]:::routerNode --> RT["router.ts<br/>routing only:<br/>idempotency, auth,<br/>concurrency"]:::routerNode
    RT --> IP["inline-pipeline.ts<br/>full execution pipeline"]:::pipelineNode
    IP --> S1["Create Comment → Fetch →<br/>Prompt → Clone → Execute →<br/>Finalize → Cleanup"]:::pipelineNode
    RT -.->|"future: non-inline modes"| DP["Daemon Dispatch"]:::configNode
    DB["src/db/<br/>Postgres singleton +<br/>migration runner"]:::dbNode -.->|"active only when<br/>DATABASE_URL set"| DP
    CFG["config.ts<br/>AGENT_JOB_MODE,<br/>triage, data layer"]:::configNode -.->|"defaults to inline"| RT
Loading

Changes

Pipeline extraction:

  • Extracted execution pipeline from router.ts → new src/core/inline-pipeline.ts (move, not rewrite)
  • router.ts now handles only routing concerns (idempotency, auth, concurrency) and delegates to runInlinePipeline()

Database foundation:

  • New src/db/index.ts — lazy Postgres connection pool singleton via Bun.sql (null when DATABASE_URL unset)
  • New src/db/migrate.ts — SQL migration runner with _migrations tracking table
  • New src/db/migrations/001_initial.sql — initial schema
  • src/app.ts — runs migrations on startup when DB configured; closes pool on shutdown

Config expansion:

  • New Zod-validated fields: agentJobMode (enum: inline/shared-runner/ephemeral-job/auto, default inline), triage classifier settings, maxTurnsPerComplexity, K8s job spawner, shared runner auth, data layer URLs
  • validateDataLayerConfig() — non-inline modes require DATABASE_URL + VALKEY_URL
  • parseMaxTurnsEnv() — JSON parsing with clear error messages

Context enrichment:

  • BotContext.labels: string[] — captures GitHub labels at trigger time (needed for future triage routing)

Dev infrastructure:

  • New docker-compose.dev.yml — Postgres (pgvector) + Valkey for local dev
  • bun run dev:deps / bun run dev:deps:down scripts

Documentation:

  • Updated CLAUDE.md, README.md, docs/ARCHITECTURE.md to reflect new structure

Tests:

  • Config tests for all new schema fields, enum validation, data layer cross-validation
  • Context tests for labels parsing from both issue and PR payloads
  • Router test descriptions updated to reflect pipeline extraction

Related Issues

  • Part of dual-mode dispatch initiative

Testing

  • I have tested these changes locally
  • I have added/updated tests as needed
  • All existing tests pass

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Optional daemon orchestration with selectable execution modes, ephemeral/shared-runner controls, cost/TTL limits, and triage pre-classifier.
    • Optional persistent database support with migrations and tracking; webhook parsing now includes issue/PR labels.
  • Bug Fixes

    • Improved server readiness gating and more graceful shutdown with database cleanup.
  • Documentation

    • Updated architecture docs and local dev quick-start (including new dev compose guidance).
  • Chores

    • Expanded example env entries and added dev scripts to start/stop local backing services.

…ual-mode dispatch

Extract execution pipeline from router.ts into src/core/inline-pipeline.ts,
add Postgres connection pool (src/db/) with migration runner, expand config
with dispatch mode, triage, and data layer settings — all defaulting to
inline mode for zero-change existing deployments.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 13, 2026 08:18
@coderabbitai

coderabbitai Bot commented Apr 13, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9bbb032d-949a-4ebe-9d11-3f15cf4fd5e3

📥 Commits

Reviewing files that changed from the base of the PR and between c0f5945 and 65ae42e.

📒 Files selected for processing (2)
  • src/webhook/router.ts
  • test/webhook/router.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • test/webhook/router.test.ts
  • src/webhook/router.ts

📝 Walkthrough

Walkthrough

Adds an optional daemon orchestration layer: new env vars and .env.example entries, a Postgres-backed DB layer with migrations and tests, extraction of the inline execution pipeline to src/core/inline-pipeline.ts, router delegating to that pipeline, startup/shutdown DB lifecycle changes, and dev Docker Compose + scripts for supporting services.

Changes

Cohort / File(s) Summary
Environment & Dev infra
\.env.example, docker-compose.dev.yml, package.json, scripts/init-test-db.sql
Added commented env entries for dispatch/daemon orchestration, triage, auth tokens, DB/Valkey URLs, job settings; added dev compose for Postgres+Valkey and npm scripts dev:deps / dev:deps:down; added DB init SQL.
Configuration
src/config.ts, test/config.test.ts
Extended Zod config with dispatch/job/triage fields and maxTurnsPerComplexity; added parseBooleanEnv and parseMaxTurnsEnv; added superRefine to require data-layer URLs when not inline; tests expanded to cover new parsing/validation behaviors.
Database layer & migrations
src/db/index.ts, src/db/migrate.ts, src/db/migrations/001_initial.sql, test/db/migrate.test.ts
Added lazy Bun SQL pool helper (getDb/requireDb/closeDb), migration runner with advisory lock and transactional application, initial executions/daemons schema, and integration tests verifying idempotency and table creation.
Runtime / App lifecycle
src/app.ts, Dockerfile
Startup runs migrations when DB configured and blocks non-health routes until ready; shutdown attempts graceful DB pool close; Dockerfile now copies migrations into production image.
Inline pipeline extraction
src/core/inline-pipeline.ts, src/core/context.ts, src/types.ts
Extracted inline execution logic into exported runInlinePipeline(ctx) implementing tracking comment lifecycle, retry/backoff, token/data fetch, prompt build, checkout, MCP/tool resolution, agent execution, finalization, and cleanup; added labels: string[] to BotContext.
Router refactor
src/webhook/router.ts, test/webhook/router.test.ts
Router retains routing responsibilities (in-memory + durable idempotency, allowlist, concurrency) and now delegates execution to runInlinePipeline(ctx); non-inline modes return a "not yet implemented" comment; tests updated for fail-fast behavior.
Docs & Developer notes
CLAUDE.md, docs/ARCHITECTURE.md, README.md
Docs updated to reflect router vs pipeline responsibilities, new src/db/ layer, dev step bun run dev:deps, and updated architecture/control-flow descriptions.
Tests & Context updates
test/core/context.test.ts, other tests*
Test factories updated to include labels; expanded unit tests for config parsing, boolean/JSON env parsing, pipeline behavior, and migration integration tests.
New files
src/core/inline-pipeline.ts, src/db/..., src/db/migrations/*
New modules implementing pipeline and DB features, plus associated migrations and tests.

Sequence Diagram(s)

sequenceDiagram
  participant GitHub as "GitHub (Webhook)"
  participant Router as "Router\n(src/webhook/router.ts)"
  participant DB as "Postgres\n(src/db)"
  participant Pipeline as "Inline Pipeline\n(src/core/inline-pipeline.ts)"
  participant Agent as "Claude Agent / MCP"
  participant Workdir as "Temp Workdir / Repo"

  GitHub->>Router: POST webhook event
  Router->>Router: in-memory idempotency fast-path
  Router->>DB: durable idempotency / concurrency checks (optional)
  Router->>Router: owner allowlist & concurrency guard
  Router->>Pipeline: delegate runInlinePipeline(ctx)
  Pipeline->>GitHub: create "Working..." tracking comment (with retry/backoff)
  Pipeline->>GitHub: request installation token & fetch PR/issue data
  Pipeline->>Workdir: checkout/clone repo to temp dir
  Pipeline->>Agent: resolve MCP/tooling and execute agent
  Agent-->>Pipeline: results (turns, cost, content)
  Pipeline->>GitHub: finalize/update tracking comment (with retry)
  Pipeline->>Workdir: cleanup temp dir
  Pipeline->>DB: optionally persist execution record (if DB configured)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐰 I hopped through env and migration rows,
Pulled pipelines out where the router goes,
Postgres hums and Valkey purrs,
Tracking comments saved in whirs,
Hop-hop — labels, jobs, and graceful closes.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'extract inline pipeline and add database foundation for dual-mode dispatch' accurately reflects the main changes: pipeline extraction, database setup, and multi-mode architecture preparation.
Docstring Coverage ✅ Passed Docstring coverage is 89.47% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/make-foundation-for-dual-mode

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Extracts the request execution pipeline out of the webhook router and introduces the initial database/config scaffolding required for future dual-mode dispatch (inline vs orchestrated), while keeping default behavior as inline.

Changes:

  • Moved the end-to-end execution pipeline into src/core/inline-pipeline.ts, leaving src/webhook/router.ts focused on routing concerns.
  • Added Postgres foundation (lazy pool singleton + SQL migration runner + initial schema) and wired migrations into app startup when DATABASE_URL is configured.
  • Expanded config schema and context parsing to support future orchestration/triage needs (e.g., agentJobMode, maxTurnsPerComplexity, and BotContext.labels).

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/webhook/router.ts Simplifies router to idempotency/auth/concurrency and delegates execution to inline pipeline.
src/core/inline-pipeline.ts New extracted inline pipeline containing the full execution workflow.
src/db/index.ts Adds lazy Postgres pool singleton and shutdown close helper.
src/db/migrate.ts Adds migration runner that applies ordered .sql files with _migrations tracking.
src/db/migrations/001_initial.sql Introduces initial schema for executions/daemons and indexes.
src/app.ts Runs migrations on startup when DB is configured; attempts to close DB on shutdown.
src/config.ts Adds dispatch/triage/data-layer config fields, cross-validation, and MAX_TURNS_PER_COMPLEXITY parsing.
src/types.ts Extends BotContext with labels: string[].
src/core/context.ts Populates BotContext.labels from issue/PR payload labels.
test/webhook/router.test.ts Updates router test descriptions and supplies labels in context factory.
test/core/context.test.ts Adds coverage for parsing labels from issue/PR payloads.
test/config.test.ts Adds tests for new config defaults/validation and parseMaxTurnsEnv.
test/db/migrate.test.ts Adds integration tests for migration runner (skipped if Postgres unavailable).
docker-compose.dev.yml Adds local dev Postgres (pgvector) + Valkey services.
package.json Adds dev:deps scripts for starting/stopping dev infra.
README.md Documents optional bun run dev:deps for non-inline modes.
docs/ARCHITECTURE.md Updates diagrams/tree to reflect pipeline extraction and new DB layer.
CLAUDE.md Updates architecture/process documentation and adds dev infra commands.
.env.example Documents new orchestration/triage-related env vars.
Comments suppressed due to low confidence (2)

src/app.ts:199

  • closeDb() is fired-and-forgotten, but process.exit(0) is called immediately after server.close(). process.exit terminates the process even if the DB close is still in flight, so the pool may not be closed cleanly. Consider awaiting closeDb() before exiting (or avoid process.exit and let the event loop drain).
function shutdown(signal: string): void {
  logger.info({ signal }, "Received shutdown signal");
  isReady = false;

  void closeDb();

  server.close(() => {
    logger.info("Server closed, exiting");
    process.exit(0);
  });

.env.example:125

  • .env.example documents the triage env vars but does not include MAX_TURNS_PER_COMPLEXITY, even though the config loader now reads and validates it. Add an example value (or at least a commented placeholder) so operators can discover/configure it consistently.
# Triage pre-classifier
# TRIAGE_ENABLED=true
# TRIAGE_MODEL=haiku-3-5
# TRIAGE_CONFIDENCE_THRESHOLD=1.0
# TRIAGE_MAX_TOKENS=256


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/db/migrations/001_initial.sql
Comment thread src/db/migrate.ts
Comment thread test/db/migrate.test.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/config.ts (1)

221-231: ⚠️ Potential issue | 🟠 Major

Enforce single-tenant ALLOWED_OWNERS for OAuth mode, not just presence.

This currently accepts ALLOWED_OWNERS="owner-a,owner-b" even though OAuth mode is supposed to be single-tenant. Tighten the assertion to require exactly one parsed owner.

🔒 Proposed fix
 export function assertOauthRequiresAllowlist(cfg: Config): void {
   if (
     cfg.provider === "anthropic" &&
-    (cfg.claudeCodeOauthToken?.trim().length ?? 0) > 0 &&
-    cfg.allowedOwners === undefined
+    (cfg.claudeCodeOauthToken?.trim().length ?? 0) > 0 &&
+    (cfg.allowedOwners === undefined || cfg.allowedOwners.length !== 1)
   ) {
     throw new Error(
-      "ALLOWED_OWNERS is required when CLAUDE_CODE_OAUTH_TOKEN is set. " +
+      "ALLOWED_OWNERS must contain exactly one owner when CLAUDE_CODE_OAUTH_TOKEN is set. " +
         "See https://code.claude.com/docs/en/agent-sdk/overview",
     );
   }
 }

As per coding guidelines, src/config.ts: When using CLAUDE_CODE_OAUTH_TOKEN authentication, require ALLOWED_OWNERS to be set to single-tenant value for security compliance with subscription quota restrictions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/config.ts` around lines 221 - 231, The current
assertOauthRequiresAllowlist check only verifies ALLOWED_OWNERS is defined but
allows multiple owners; update assertOauthRequiresAllowlist(cfg: Config) to
parse cfg.allowedOwners (split on commas, trim and filter out empty entries) and
require exactly one owner when cfg.claudeCodeOauthToken is set and cfg.provider
=== "anthropic"; if the parsed owners array length !== 1, throw an Error (update
the message to state ALLOWED_OWNERS must contain exactly one owner when
CLAUDE_CODE_OAUTH_TOKEN is used) so OAuth mode is enforced as single-tenant.
🧹 Nitpick comments (1)
package.json (1)

56-58: Add a first-class bun audit script while touching the scripts block.

The new helper scripts land without an explicit audit command, so runtime dependency scanning is still easy to skip locally and in CI. Consider adding audit and folding it into check.

♻️ Proposed update
-    "check": "bun run typecheck && bun run lint && bun run format && bun test",
+    "check": "bun run typecheck && bun run lint && bun run format && bun run audit && bun test",
+    "audit": "bun audit",
     "dev:deps": "docker compose -f docker-compose.dev.yml up -d",
     "dev:deps:down": "docker compose -f docker-compose.dev.yml down",

As per coding guidelines package.json: Use bun audit for runtime dependency security scanning.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` around lines 56 - 58, Add a top-level "audit" npm script that
runs "bun audit" and include it in the existing "check" script so runtime
dependency scanning is always executed; update the "check" script (which
currently references "bun run typecheck && bun run lint && bun run format && bun
test") to also run "bun run audit" and add a new "audit": "bun audit" entry to
the scripts block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.env.example:
- Around line 83-124: Add documentation and a sample value for
MAX_TURNS_PER_COMPLEXITY in the .env.example so operators can discover the
override and JSON shape parsed by src/config.ts; specifically, insert a
commented env entry named MAX_TURNS_PER_COMPLEXITY with a short description and
a representative JSON example (e.g. mapping complexity levels to numeric turn
limits) plus the default/example values and note that src/config.ts will parse
it as JSON, so include formatting guidance (quotes, braces) and a small example
to copy-paste.

In `@docker-compose.dev.yml`:
- Around line 10-11: The ports mapping currently exposes services on all
interfaces via "5432:5432"; update the docker-compose service port bindings for
the Postgres and Valkey services (the ports entries) to bind to localhost by
changing the host side to 127.0.0.1 (e.g., "127.0.0.1:5432:5432" and the
equivalent for Valkey) so the dev services are only reachable from the local
machine.

In `@src/app.ts`:
- Around line 169-174: The app currently starts listening before DB migrations
run (getDb() / runMigrations) so incoming webhooks can hit an unmigrated schema;
fix by running database migrations as part of the pre-listen startup sequence
(call runMigrations(db) and await it before server.listen() / before
runStartupChecks()) or alternatively ensure the request handler uses isReady to
return 503 for all non-health routes until runMigrations completes and isReady
is set true; update the startup flow (symbols: getDb, runMigrations,
server.listen, runStartupChecks, isReady) so migrations finish before any
non-health traffic is forwarded.
- Around line 194-198: The code calls void closeDb() before server.close(),
which can prematurely close DB pools and let the process exit before the close
Promise settles; change the shutdown flow to call and await closeDb() inside the
server.close callback (or make the callback async) so that after server.close
drains listeners you run await closeDb(), then logger.info("Server closed,
exiting") and only then call process.exit(0); also remove the earlier void
closeDb() call so closeDb() is only invoked after the HTTP server has finished
draining.

In `@src/config.ts`:
- Around line 183-203: The validateDataLayerConfig function currently treats
whitespace-only DATABASE_URL or VALKEY_URL as present; update the checks in
validateDataLayerConfig to trim data.databaseUrl and data.valkeyUrl (e.g., use
data.databaseUrl?.trim()) before verifying undefined/empty, and add ctx.addIssue
when the trimmed value is empty so non-inline modes reject whitespace-only URLs;
reference validateDataLayerConfig and the data.databaseUrl / data.valkeyUrl
checks to locate and change the validation logic.
- Around line 306-312: Replace the current permissive coercion for
TRIAGE_ENABLED with a strict parser: implement a helper (e.g.,
parseBooleanEnv(name: string, raw: string|undefined)) that returns true for
normalized tokens ["true","1","yes"], false for ["false","0","no"], undefined
for undefined, and throws an Error for any other value; then use this helper to
set triageEnabled (replace the existing ternary logic) and ensure the resulting
value is validated by your zod config schema so invalid env values fail startup
instead of silently becoming false.

In `@src/core/inline-pipeline.ts`:
- Around line 136-143: Wrap the error-path call to finalizeTrackingComment in
the same retryWithBackoff used on the success path so transient API/GitHub
errors don't leave the tracking comment stuck; specifically, replace the
single-call await finalizeTrackingComment(...) in the trackingCommentId !==
undefined error branch with a retryWithBackoff(() =>
finalizeTrackingComment(ctx, trackingCommentId, { success: false, error:
"..."})) and on ultimate failure log the caught error via ctx.log.error
(preserve the existing message "Failed to update tracking comment with error").
Ensure the retry parameters match the success-path usage.
- Around line 103-145: The current flow treats post-success bookkeeping errors
(finalizeTrackingComment, retryWithBackoff, cleanup) as overall failures because
they bubble to the outer catch; change this so executeAgent(enrichedCtx, ...)
errors are the only ones that trigger the failure tracking comment.
Specifically: run executeAgent and capture its result inside a try/catch that on
failure posts the failure tracking comment (using ctx and trackingCommentId) and
rethrows or returns; after a successful executeAgent call, perform
buildFinalOpts(result), retryWithBackoff(() =>
finalizeTrackingComment(enrichedCtx, resolvedTrackingCommentId, finalOpts),
...), and await cleanup() each inside their own try/catch blocks (or a single
non-throwing block) that log errors via enrichedCtx.log or ctx.log but do not
rethrow or set success=false; keep the outer error logging of unexpected errors,
but ensure that exceptions from finalizeTrackingComment or cleanup do not change
the recorded success status from the already-successful executeAgent.

In `@src/db/migrate.ts`:
- Around line 42-74: The migration loop can race across replicas; wrap the whole
migration pass in a DB-level lock to serialize runners by acquiring a Postgres
advisory lock (e.g. call sql`SELECT pg_advisory_lock(<fixed_key>)` before
snapshotting _migrations and the file loop, and release it after with
pg_advisory_unlock) so only one process reads/applies migrations at a time;
modify the code around the existing applied/appliedSet/files loop (symbols:
MIGRATIONS_DIR, appliedSet, sql.begin/tx, and the INSERT INTO _migrations
statement) to acquire the lock before reading applied versions and release it
when finished.

In `@src/db/migrations/001_initial.sql`:
- Line 12: Remove the CREATE EXTENSION IF NOT EXISTS vector; statement from the
initial migration and instead add a separate migration that runs CREATE
EXTENSION IF NOT EXISTS vector; right before the first migration that actually
adds pgvector columns (e.g., create a new migration file named something like
"add_pgvector_extension" and place the CREATE EXTENSION statement there). This
ensures the "CREATE EXTENSION IF NOT EXISTS vector;" SQL is only applied when
pgvector is required and avoids blocking deployments that don't use vector
columns.

In `@src/webhook/router.ts`:
- Around line 121-123: The code unconditionally calls runInlinePipeline(ctx) so
config.agentJobMode is ignored; modify the try block in the webhook router to
branch on config.agentJobMode (e.g., 'inline' -> await runInlinePipeline(ctx);
'shared-runner'/'ephemeral-job' -> dispatch to their respective handlers) or
immediately throw/return a clear error when agentJobMode is set to a non-inline
value until those dispatchers are implemented; update any surrounding logic to
ensure the chosen path is executed (references: runInlinePipeline and
config.agentJobMode).

---

Outside diff comments:
In `@src/config.ts`:
- Around line 221-231: The current assertOauthRequiresAllowlist check only
verifies ALLOWED_OWNERS is defined but allows multiple owners; update
assertOauthRequiresAllowlist(cfg: Config) to parse cfg.allowedOwners (split on
commas, trim and filter out empty entries) and require exactly one owner when
cfg.claudeCodeOauthToken is set and cfg.provider === "anthropic"; if the parsed
owners array length !== 1, throw an Error (update the message to state
ALLOWED_OWNERS must contain exactly one owner when CLAUDE_CODE_OAUTH_TOKEN is
used) so OAuth mode is enforced as single-tenant.

---

Nitpick comments:
In `@package.json`:
- Around line 56-58: Add a top-level "audit" npm script that runs "bun audit"
and include it in the existing "check" script so runtime dependency scanning is
always executed; update the "check" script (which currently references "bun run
typecheck && bun run lint && bun run format && bun test") to also run "bun run
audit" and add a new "audit": "bun audit" entry to the scripts block.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bd57e57a-3bd2-4ee2-b6a9-b96a8aaa157d

📥 Commits

Reviewing files that changed from the base of the PR and between ac300d5 and fc18c6f.

📒 Files selected for processing (19)
  • .env.example
  • CLAUDE.md
  • README.md
  • docker-compose.dev.yml
  • docs/ARCHITECTURE.md
  • package.json
  • src/app.ts
  • src/config.ts
  • src/core/context.ts
  • src/core/inline-pipeline.ts
  • src/db/index.ts
  • src/db/migrate.ts
  • src/db/migrations/001_initial.sql
  • src/types.ts
  • src/webhook/router.ts
  • test/config.test.ts
  • test/core/context.test.ts
  • test/db/migrate.test.ts
  • test/webhook/router.test.ts

Comment thread .env.example
Comment thread docker-compose.dev.yml Outdated
Comment thread src/app.ts
Comment thread src/app.ts Outdated
Comment thread src/config.ts
Comment thread src/core/inline-pipeline.ts
Comment thread src/core/inline-pipeline.ts
Comment thread src/db/migrate.ts Outdated
Comment thread src/db/migrations/001_initial.sql Outdated
Comment thread src/webhook/router.ts
Fixes all valid review comments from Copilot and CodeRabbit on PR #13:

- Bind docker-compose dev ports to 127.0.0.1 (security)
- Copy src/db/migrations/ into Docker production image (was missing)
- Remove premature pgvector extension from initial migration
- Add advisory lock to serialize migrations across replicas
- Fix shutdown order: close DB after HTTP server drain, not before
- Return 503 for non-health routes until startup checks complete
- Reject whitespace-only DATABASE_URL/VALKEY_URL in non-inline modes
- Add strict parseBooleanEnv — reject unrecognized TRIAGE_ENABLED values
- Enforce exactly-one-owner for OAuth allowlist (single-tenant)
- Separate post-success bookkeeping errors from pipeline failures
- Add retryWithBackoff to error-path tracking comment update
- Fail fast for non-inline AGENT_JOB_MODE (not yet implemented)
- Document MAX_TURNS_PER_COMPLEXITY in .env.example
- Add init-test-db.sql for docker-compose test DB provisioning

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/webhook/router.ts`:
- Around line 122-129: When config.agentJobMode !== "inline" the router
currently only logs and returns, leaving the user unaware and the in-memory
idempotency entry set; update this branch (the check using config.agentJobMode
and before returning instead of silently returning from the router that calls
runInlinePipeline) to provide explicit user feedback—either post a comment back
to the user/PR similar to the existing capacity comment flow so they know
"non-inline mode not implemented", or throw an error to let a higher-level
handler respond; ensure the chosen approach also clears or invalidates the
in-memory idempotency entry so the request can be retried if appropriate.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d8a79d78-75ac-4dcb-8082-8f78300f1ed9

📥 Commits

Reviewing files that changed from the base of the PR and between fc18c6f and c0f5945.

📒 Files selected for processing (12)
  • .env.example
  • Dockerfile
  • docker-compose.dev.yml
  • scripts/init-test-db.sql
  • src/app.ts
  • src/config.ts
  • src/core/inline-pipeline.ts
  • src/db/migrate.ts
  • src/db/migrations/001_initial.sql
  • src/webhook/router.ts
  • test/config.test.ts
  • test/webhook/router.test.ts
✅ Files skipped from review due to trivial changes (4)
  • Dockerfile
  • scripts/init-test-db.sql
  • docker-compose.dev.yml
  • .env.example
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/app.ts
  • test/webhook/router.test.ts
  • src/db/migrations/001_initial.sql

Comment thread src/webhook/router.ts
Silent return left users with no feedback when the bot was misconfigured
for a non-inline mode. Now posts a comment (matching the capacity comment
pattern) before returning, so the user knows to contact the admin.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@chrisleekr
chrisleekr merged commit f05e818 into main Apr 13, 2026
10 checks passed
@chrisleekr
chrisleekr deleted the feat/make-foundation-for-dual-mode branch April 13, 2026 08:58
chrisleekr pushed a commit that referenced this pull request Apr 17, 2026
# [1.1.0](v1.0.0...v1.1.0) (2026-04-17)

### Bug Fixes

* **orchestrator:** use IN ${db(ids)} in repo-knowledge to fix Bun.sql array binding ([#26](#26)) ([d5e1b17](d5e1b17))
* **research:** update schedule ([ac300d5](ac300d5))

### Features

* **auth:** add CLAUDE_CODE_OAUTH_TOKEN support with ALLOWED_OWNERS allowlist ([#10](#10)) ([445d354](445d354))
* **ci:** add scheduled research workflow with claude-code-action ([#9](#9)) ([f67c5db](f67c5db))
* **core:** extract inline pipeline and add database foundation for dual-mode dispatch ([#13](#13)) ([f05e818](f05e818))
* **daemon:** add persistent repo memory, env var injection, and dev E2E tooling ([#14](#14)) ([585156f](585156f))
* triage-dispatch-modes Slice B — setup + foundational (T001-T013) ([#18](#18)) ([d0533eb](d0533eb))
* triage-dispatch-modes Slice C — US1 MVP label/keyword routing (T014-T026) ([#19](#19)) ([2b345ee](2b345ee))
* **triage:** Slice D — US2 auto-mode probabilistic dispatch ([#20](#20)) ([457eb4e](457eb4e))
* **triage:** Slice E (part 1) — isolated-job capacity gate + pending queue + drainer ([#21](#21)) ([7653b0d](7653b0d))
* **triage:** slice E part 2 — isolated-job completion watcher (T042/T046–T049) ([#22](#22)) ([c0e86dd](c0e86dd))
* **triage:** slice F — US4 telemetry aggregates + log contract (T050–T054) ([#24](#24)) ([bb7fa9f](bb7fa9f))
@chrisleekr

Copy link
Copy Markdown
Owner Author

🎉 This PR is included in version 1.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants