feat: triage-dispatch-modes Slice C — US1 MVP label/keyword routing (T014-T026)

[chrisleekr]

Summary

Slice C of the triage-dispatch-modes feature — the US1 MVP. Label + keyword routing now works end-to-end against shared-runner and isolated-job targets, with auto-mode triage still deferred to US2 (Slice D).

Follows PR #18 (Slice B foundational). Upcoming: D (US2 auto-triage), E (US3 capacity), F (US4 telemetry), G (polish).

What lands

Task	Artefact
T015 + T017	Pure classifyStatic(ctx) in src/k8s/classifier.ts. Label precedence (bot:job → isolated-job, bot:shared → shared-runner, bot:job wins when both present), then word-bounded keyword match (docker / compose / dind), else ambiguous. 16 tests, 100% line + function coverage.
T021 + T022	resolveAllowedTools extended with the in-pod branch — when AGENT_JOB_MODE=isolated-job OR AGENT_CONTEXT_B64 is non-empty, grants Bash(docker:* / docker-compose:* / npm:* / npx:* / bun:* / bunx:* / make:* / sh:* / bash:* / cp:* / mv:*). 5 new test cases including a regression guard for the inline / daemon / shared-runner negative paths.
T023	decideDispatch now invokes classifyStatic for non-inline modes; clear classifications win over the static-default echo. Inline mode short-circuits the classifier (operator's explicit AGENT_JOB_MODE=inline is itself the strong signal).
T018 + T014	dispatchToSharedRunner(ctx, decision) posts to ${INTERNAL_RUNNER_URL}/internal/run with X-Internal-Token shared-secret auth, maps every documented status code to a typed SharedRunnerError (validation / unauthorized / duplicate / at-capacity / internal / timeout / network / unconfigured), retries once on 429 per contract. Contract test pins the typed-error surface + the unconfigured guard.
T019	spawnIsolatedJob(ctx, decision) in src/k8s/job-spawner.ts. Lazy K8s client init (loadFromCluster() in-cluster vs loadFromDefault() out-of-cluster); infra-absent thrown before any network call when neither env is set. Builds the V1Job per research.md R8: backoffLimit:0, ttlSecondsAfterFinished, activeDeadlineSeconds:1800, init container waits for Docker daemon, claude-agent + docker:27-dind sidecar with shared emptyDir, resource requests 500m/1Gi → limits 2000m/4Gi.
T020	src/k8s/job-entrypoint.ts — runs INSIDE the spawned pod via bun run src/k8s/job-entrypoint.ts. Decodes BotContext from AGENT_CONTEXT_B64, re-mints Octokit from GitHub App credentials, invokes runInlinePipeline, exits 0/1. Defensive asString / asNumber decoding so a malformed envelope fails cleanly instead of crashing.
T024	dispatch() switch lights up the three target branches: daemon → existing dispatchNonInline (Phase 2 path, unchanged), shared-runner → dispatchToSharedRunner, isolated-job → spawnIsolatedJob. NotImplementedError class kept as a typed surface for future targets.
T025	FR-018 graceful rejection: isolated-job + JobSpawnerError(infra-absent) → recordInfraAbsentRejection writes a rejection execution row with dispatch_target="isolated-job" and posts a tracking comment explaining the platform won't silently downgrade. Other JobSpawnerError kinds bubble to the runtime-failure path.

Deferred to in-PR follow-ups

• T016 — integration test for US1 dispatch cascade: needs a richer mock harness (GitHub API + Claude Agent SDK + K8s client). The unit tests + the contract test + the updated router tests give reasonable coverage; the integration test is the next thing I'll add if reviewers want it before merge.
• T026 — JSDoc audit pass: the new exports in classifier.ts, shared-runner-dispatcher.ts, job-spawner.ts, job-entrypoint.ts already carry JSDoc with @param / @returns / @throws per constitution §VIII. A formal one-pass review of every touched file is the polish step.

Checks

• bun run check → typecheck ✓, lint 0 errors, format ✓, 24/24 test files pass (was 22 in Slice B; added classifier + shared-runner contract).
• git diff --stat main...HEAD (Slice C only) → 7 files changed, ~1100 LOC including tests.

Known-safe behavioural changes

• The old auto-mode behaviour where dispatchNonInline mapped auto → shared-runner is gone — decideDispatch now resolves auto into a concrete target via defaultDispatchTarget, and the dispatch_mode execution-row column reflects that target. Existing tests that asserted dispatchMode === "shared-runner" for auto mode were re-pointed to test daemon mode (the path they were actually exercising via dispatchNonInline).
• The "isolated-job throws NotImplementedError" Slice B test was removed (no longer accurate — we now actually call spawnIsolatedJob). Replaced with a positive test that asserts the FR-018 graceful-rejection comment is posted when K8s auth is absent.

Test plan

• bun run check green
• Existing CI checks expected to pass
• CodeRabbit + Copilot review — will address blocking comments before merge
• Manual smoke (shared-runner): set AGENT_JOB_MODE=shared-runner, INTERNAL_RUNNER_URL=…, INTERNAL_RUNNER_TOKEN=…; trigger a webhook with bot:shared label; verify the dispatch-decision log shows dispatchTarget=shared-runner, dispatchReason=label, triageInvoked=false and the dispatcher hits /internal/run.
• Manual smoke (isolated-job, infra-absent): set AGENT_JOB_MODE=isolated-job without KUBERNETES_SERVICE_HOST / KUBECONFIG; trigger a bot:job-labelled webhook; verify the rejection comment posts and no Job is created.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added support for isolated Kubernetes job execution as a new dispatch target alongside existing options
  • Implemented automatic routing detection using labels and keywords (e.g., Docker, compose, dind) to determine optimal execution environment
  • Enhanced dispatch decision logic with static classification for deterministic, consistent routing

• Tests

  • Added comprehensive test coverage for job isolation, dispatch routing, and execution flow validation

[coderabbitai]

Warning

Rate limit exceeded

@chrisleekr has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 42 minutes and 58 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 42 minutes and 58 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2eaa2e78-a29e-45ac-aa9e-30cf65c3d0da

📥 Commits

Reviewing files that changed from the base of the PR and between 4ec9b09 and 251ecdb.

📒 Files selected for processing (7)

• Dockerfile
• src/k8s/job-entrypoint.ts
• src/k8s/job-spawner.ts
• src/k8s/shared-runner-dispatcher.ts
• src/orchestrator/history.ts
• src/webhook/router.ts
• test/k8s/classifier.test.ts

📝 Walkthrough

Walkthrough

This PR introduces an isolated Kubernetes Job execution mode for webhook event processing. It adds a static event classifier to determine dispatch targets via labels or keywords, a Kubernetes job spawner to submit jobs, an in-pod entrypoint to orchestrate execution, a shared-runner HTTP dispatcher, and integrates these components into the webhook router. Tool resolution is extended to include container-related Bash tools in isolated job mode, detected via environment variables.

Changes

Cohort / File(s)	Summary
Kubernetes Job Execution Infrastructure src/k8s/job-spawner.ts, src/k8s/job-entrypoint.ts, src/k8s/classifier.ts	Implements isolated Kubernetes Job submission and execution. Job spawner constructs Job manifests with init container, main agent container, and docker-dind sidecar; handles auth via kubeconfig/in-cluster detection; exports typed error kinds (infra-absent, auth-load-failed, api-rejected, api-unavailable). Entrypoint decodes base64-serialized BotContext, reconstructs Octokit credentials, normalizes context fields, invokes inline pipeline, and exits with appropriate status. Classifier provides deterministic label/keyword-based dispatch routing (isolated-job vs shared-runner) with precedence rules and regex-based whole-word keyword matching.
Shared-Runner Dispatch src/k8s/shared-runner-dispatcher.ts	Dispatches events to internal runner via POST to /internal/run. Implements response parsing with runtime type guards, HTTP status-to-error-kind mapping (400→validation, 401→unauthorized, 409→duplicate, 500→internal, 504→timeout), single-retry logic for 429 at-capacity responses, and exports typed SharedRunnerError with optional status and executionId fields.
Integration & Tool Resolution src/webhook/router.ts, src/core/prompt-builder.ts	Router integrates static classification via classifyStatic() when non-inline, routes shared-runner dispatch to new dispatcher, replaces isolated-job NotImplementedError with spawnIsolatedJob() wrapped in infra-absence error handling that posts rejection comments. Prompt builder conditionally appends container-related Bash tools (docker, docker-compose, npm, npx, bun, bunx, make, sh, bash, cp, mv) when AGENT_JOB_MODE="isolated-job" or AGENT_CONTEXT_B64 is non-empty.
Test Suite test/k8s/classifier.test.ts, test/k8s/job-entrypoint.ts, test/k8s/job-spawner.ts, test/contract/shared-runner-internal.test.ts, test/core/prompt-builder.test.ts, test/webhook/router.test.ts	Comprehensive tests validating classifier label/keyword precedence and purity, job spawner spec construction and error handling, job entrypoint context deserialization, shared-runner error contract (8 error kinds), isolated-job tool inclusion via environment variables, and router dispatch routing with infra-absence comment posting.

Sequence Diagram(s)

sequenceDiagram
    participant Webhook as Webhook Event
    participant Router as Router
    participant Classifier as Static Classifier
    participant Spawner as Job Spawner
    participant K8sAPI as Kubernetes API
    participant Entrypoint as Job Entrypoint
    participant Pipeline as Inline Pipeline

    Webhook->>Router: POST /webhook
    Router->>Classifier: classifyStatic(ctx)
    Classifier-->>Router: { outcome: "clear", mode: "isolated-job"|"shared-runner", reason: "label"|"keyword" }
    
    alt isolated-job target
        Router->>Spawner: spawnIsolatedJob(ctx, decision)
        Spawner->>K8sAPI: createNamespacedJob(jobSpec)
        K8sAPI-->>Spawner: Job created
        Spawner-->>Router: ExecutionResult { success: true, durationMs: 0 }
    else shared-runner target
        Router->>Router: dispatchToSharedRunner(ctx, decision)
        Router->>Router: POST /internal/run
    end
    
    note over K8sAPI,Entrypoint: Pod Execution (Async)
    K8sAPI->>Entrypoint: Container starts
    Entrypoint->>Entrypoint: Decode AGENT_CONTEXT_B64
    Entrypoint->>Entrypoint: Reconstruct Octokit
    Entrypoint->>Pipeline: runInlinePipeline(ctx)
    Pipeline-->>Entrypoint: Result
    Entrypoint->>Entrypoint: Exit 0|1

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• PR #18: Scaffolds decideDispatch, dispatch routing, and NotImplementedError for isolated-job; this PR implements the isolated-job execution path that was previously stubbed.
• PR #14: Extends resolveAllowedTools with daemon-capabilities-driven MCP injection; this PR further extends tool resolution to include container Bash tools for isolated job pods.
• PR #13: Establishes dual-mode dispatch foundation (inline vs non-inline) and introduces agentJobMode config; this PR builds on that foundation by implementing concrete isolated-job and shared-runner dispatch paths.

Poem

🐇 A job spawns in the K8s clouds so high,
With labels and keywords to classify,
Docker and compose tools bundled tight,
The rabbit's pipeline runs through the night!
hop hop — isolated jobs take flight! 🚀

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: implementing US1 MVP label/keyword routing for triage-dispatch-modes, with specific task references (T014-T026) that align with the substantial feature work across multiple files.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 20260415-000159-triage-dispatch-modes

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[Copilot]

Pull request overview

Implements US1 MVP routing for the triage-dispatch-modes feature by adding deterministic label/keyword classification and wiring dispatch for shared-runner and isolated-job targets, with supporting K8s/shared-runner integrations and expanded tool allow-listing for in-pod execution.

Changes:

• Added classifyStatic(ctx) for label/keyword-based dispatch target classification and integrated it into decideDispatch.
• Implemented shared-runner HTTP dispatch with typed error mapping and a contract test for the error surface/unconfigured guard.
• Added isolated-job K8s Job spawning + in-pod entrypoint, and extended resolveAllowedTools for isolated-job pods, with new unit tests.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
src/webhook/router.ts	Routes via classifyStatic, dispatches to shared-runner/isolated-job, adds infra-absent rejection handling.
src/k8s/classifier.ts	New deterministic label/keyword classifier producing clear vs ambiguous outcomes.
src/k8s/shared-runner-dispatcher.ts	New dispatcher to shared-runner /internal/run with typed errors and one 429 retry.
src/k8s/job-spawner.ts	New K8s Job spec builder + submitter for isolated-job execution.
src/k8s/job-entrypoint.ts	New in-pod entrypoint to reconstruct context and run the inline pipeline.
src/core/prompt-builder.ts	Expands tool allow-list when running inside an isolated-job pod.
test/webhook/router.test.ts	Updates router tests for daemon-mode expectations and adds isolated-job infra-absent rejection test.
test/k8s/classifier.test.ts	Adds unit tests for classifier label precedence, keyword matching, and purity guarantees.
test/core/prompt-builder.test.ts	Adds unit tests for isolated-job tool allow-listing behavior.
test/contract/shared-runner-internal.test.ts	Adds contract test for shared-runner dispatcher typed-error surface and unconfigured guard.

---

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

[Copilot]

The JSDoc for recordInfraAbsentRejection claims it persists an execution row with dispatch_reason="infra-absent", but the createExecution call here only sets dispatchMode and the reason parameter is only used for logging. Either update the comment to match what’s actually persisted, or (if the schema now supports a dispatch-reason column) extend the execution write to store the rejection reason so analytics/debugging can rely on it.

Suggested change

	* FR-018 rejection write-path. Persists an execution row with
	* dispatch_reason="infra-absent" and updates the tracking comment so the
	* maintainer sees a clear refusal. Never silently downgrades to a different
	* target — that defeats the purpose of asking for isolated execution.
	* FR-018 rejection write-path. Records/logs an infra-absent isolated-job
	* rejection and updates the tracking comment so the maintainer sees a clear
	* refusal. Never silently downgrades to a different target — that defeats
	* the purpose of asking for isolated execution.

[Copilot]

This test case’s description/comment is internally contradictory: it says it “does NOT match on keyword-within-URL”, but the assertions below explicitly pin that it does match when bounded by / and -. Please rewrite the test name/comment to reflect the actual expected behavior so future readers don’t misinterpret what’s being pinned.

Suggested change

	it("does NOT match on keyword-within-URL (whole-word semantics)", () => {
	// Word-boundary treats hyphens as word chars in some locales — we pass a
	// URL-like string to make sure a random path containing "docker" still
	// matches (because hyphens/slashes ARE word boundaries).
	const result = classifyStatic(
	makeCtx({ triggerBody: "see https://example.com/docker-tutorial" }),
	);
	// `docker` IS a whole word here (bounded by `/` and `-`), so this DOES match.
	// The test pins the behaviour so the regex doesn't silently change under us.
	it("matches on keyword-within-URL when bounded by `/` and `-` (whole-word semantics)", () => {
	// Pass a URL-like string to verify that `docker` still matches here,
	// because it is a whole word bounded by `/` and `-`.
	const result = classifyStatic(
	makeCtx({ triggerBody: "see https://example.com/docker-tutorial" }),
	);
	// This test pins that behavior so the regex doesn't silently change under us.

[Copilot]

job-entrypoint requires installationId to be present in the decoded AGENT_CONTEXT_B64 payload, but the server-side context serialization (serializeBotContext) does not include an installationId field (it only strips octokit/log). As written, isolated-job pods will always fail the required-fields check and exit 1. Fix by either (a) looking up the installation ID inside the pod via app.octokit.rest.apps.getRepoInstallation({ owner, repo }) (similar to src/orchestrator/connection-handler.ts), or (b) extending the serialized context type to include installationId and ensuring it’s populated before encoding.

[Copilot]

The initContainer runs docker info but uses image (the bot’s Bun/Node image), which the repo’s Dockerfile does not install the Docker CLI into. This will make the initContainer fail immediately (docker: not found) and prevent the Job from starting. Use an image that contains the Docker client (e.g., a docker:* CLI image) or ensure the configured JOB_IMAGE includes a Docker CLI.

[Copilot]

The claude-agent container image is the same image used by the webhook server, but that image currently does not include the Docker CLI. Since isolated-job mode explicitly allows Bash(docker:*) / Bash(docker-compose:*), the agent will fail at runtime even though the sidecar provides the daemon. Ensure the job image bundles a Docker client (or otherwise make docker available in PATH) so allowed tools are actually usable.

[Copilot]

isSuccessResponse() doesn’t validate the status field even though RunnerSuccessResponse requires it and the caller uses parsed.body.status to decide success. A malformed 200 response missing status would currently be treated as "success" and reported as success: false instead of surfacing a protocol error. Tighten the type guard to require status to be either "success" or "failure" (and consider validating the other expected fields you rely on).

[Copilot]

dispatch() now routes shared-runner directly to dispatchToSharedRunner(), but unlike the inline/daemon paths it does not write an execution record first. This contradicts the dispatcher’s own contract comment (it says the caller writes the executions DB row) and means shared-runner runs won’t have the same DB/audit trail or context persistence. Consider creating the execution row (like dispatchNonInline does) before calling dispatchToSharedRunner (and handle DB-unconfigured similarly to other paths).

Suggested change

	case "shared-runner":
	await dispatchToSharedRunner(ctx, decision);
	return;
	case "shared-runner": {
	const db = getDb();
	if (db !== null) {
	try {
	const serializedCtx = serializeBotContext(ctx);
	await createExecution({
	deliveryId: ctx.deliveryId,
	repoOwner: ctx.owner,
	repoName: ctx.repo,
	entityNumber: ctx.entityNumber,
	entityType: ctx.isPR ? "pull_request" : "issue",
	eventName: ctx.eventName,
	triggerUsername: ctx.triggerUsername,
	dispatchMode: "shared-runner",
	contextJson: serializedCtx,
	});
	} catch (recordErr) {
	ctx.log.error({ err: recordErr }, "Failed to create shared-runner execution record (non-fatal)");
	}
	}
	await dispatchToSharedRunner(ctx, decision);
	return;
	}

[Copilot]

Active-concurrency accounting will leak for both shared-runner and isolated-job targets. processRequest() increments activeCount for every request, but for these targets there’s no decrement on success, and the infra-absent rejection path returns without decrementing (unlike dispatchNonInline, which manages the counter). This will eventually force the server into permanent “at capacity” rejections. Add an explicit decrementActiveCount() when the shared-runner call completes, and after isolated-job submission/rejection (or implement an equivalent completion signal for those targets).

[chrisleekr]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[chrisleekr]

Addressed all 7 substantive findings from Copilot's review in 85e93e5:

#	File	Finding	Fix
1	router.ts	Concurrency leak: shared-runner & isolated-job never decrement activeCount	Extended decrement logic in processRequest to all targets whose ownership is handed off at this level; daemon still self-manages via dispatchNonInline
2	router.ts	shared-runner path didn't write an executions row	Mirror the inline path — createExecution before HTTP dispatch
3	router.ts	recordInfraAbsentRejection JSDoc claimed it persists dispatch_reason="infra-absent" but didn't	Extended createExecution to accept optional dispatchReason; wired into all 3 router write-sites so the JSDoc is now literally true
4	shared-runner-dispatcher.ts	isSuccessResponse didn't validate status field	Tightened guard to require status ∈ {"success","failure"}
5	job-entrypoint.ts	installationId never populated by serializeBotContext — pod always aborted	Resolve installation via apps.getRepoInstallation inside the pod (same pattern as orchestrator/connection-handler.ts)
6	job-spawner.ts	Init container ran docker info from the bot image (no docker CLI)	Switch to docker:27-cli (same major as the dind sidecar)
7	Dockerfile	Bot image has no docker CLI — isolated-job claude-agent container couldn't invoke Bash(docker:*)	Copy static docker binary from docker:27-cli into production stage

Also rewrote the contradictory test name in classifier.test.ts:110 (nitpick).

Full check passes: tsc 0 errors, ESLint 0 errors, all 24 test files green. Waiting for CodeRabbit's fresh review (rate-limit window elapsed; re-triggered above).

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (3)

src/k8s/job-entrypoint.ts (1)

81-94: Consider if webhookSecret is strictly required for the App constructor.

The webhookSecret is used for webhook signature verification, not for obtaining installation tokens. The App constructor works without it when only calling getInstallationOctokit(). However, requiring it here maintains consistency with the server-side config validation and prevents misconfiguration.

This is not a bug — just noting the dependency is stricter than the minimum required by the Octokit SDK.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/k8s/job-entrypoint.ts` around lines 81 - 94, The current validation
enforces webhookSecret even though App can be constructed without it for
getInstallationOctokit(); update the check to only require config.appId and
config.privateKey, and when building the App instance pass webhookSecret
conditionally (include webhookSecret: config.webhookSecret only if defined) so
getInstallationOctokit still works without a webhook secret; reference
config.appId, config.privateKey, config.webhookSecret, the App constructor, and
getInstallationOctokit when making this change and add a brief inline comment
noting webhookSecret is only needed for webhook verification.

src/k8s/job-spawner.ts (2)

160-170: Init container uses application image for shell commands.

The init container uses the full application image (image variable) just to run a shell polling loop. This works but loads a larger image than necessary. Consider using a minimal image like busybox or docker:27-cli for the wait-for-docker check.

♻️ Suggested optimization

           initContainers: [
             {
               name: "wait-for-docker",
-              image,
+              image: "docker:27-cli",
               command: [
                 "sh",
                 "-c",
                 "until DOCKER_HOST=tcp://localhost:2375 docker info >/dev/null 2>&1; do sleep 1; done",
               ],
               env: [{ name: "DOCKER_HOST", value: "tcp://localhost:2375" }],
             },
           ],

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/k8s/job-spawner.ts` around lines 160 - 170, The init container
"wait-for-docker" is using the application image (image variable) just to run a
shell loop; change it to a minimal utility image (e.g., "busybox" or
"docker:27-cli") instead of using image to avoid pulling the large app image.
Update the initContainers entry (the one with name "wait-for-docker" in
job-spawner.ts) to set image to the chosen minimal image, keep the command and
env (DOCKER_HOST) intact, and adjust imagePullPolicy if needed for your
registry.

---

112-136: API credentials passed as plain environment variables.

Sensitive credentials (ANTHROPIC_API_KEY, CLAUDE_CODE_OAUTH_TOKEN) are passed directly as environment variables in the Job spec. While functional and common for short-lived Jobs, this means credentials are visible in kubectl describe job output and stored in etcd.

For improved security posture, consider referencing Kubernetes Secrets instead:

🔐 Example using secretKeyRef

// Instead of:
providerEnv.push({ name: "ANTHROPIC_API_KEY", value: config.anthropicApiKey ?? "" });

// Consider:
providerEnv.push({
  name: "ANTHROPIC_API_KEY",
  valueFrom: { secretKeyRef: { name: "claude-credentials", key: "anthropic-api-key" } }
});

This requires pre-creating the Secret in the namespace but prevents credential exposure in Job specs and kubectl output.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/k8s/job-spawner.ts` around lines 112 - 136, Replace the plain-text
environment variable entries for API credentials with secret references: instead
of pushing objects with value set from config.anthropicApiKey and
config.claudeCodeOauthToken into providerEnv via providerEnv.push(...), add
entries using valueFrom.secretKeyRef that reference a Kubernetes Secret (e.g.,
name "claude-credentials" with keys like "anthropic-api-key" and
"claude-code-oauth-token"); ensure the code paths that currently check
config.anthropicApiKey and config.claudeCodeOauthToken switch to creating
secret-based env entries and document/create the required Secret in the
namespace before Job creation so credentials are not embedded in the Job spec or
etcd.

🤖 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/k8s/job-spawner.ts`:
- Around line 227-242: The createNamespacedJob call uses an object parameter but
the BatchV1Api.createNamespacedJob signature expects positional args; update the
call in job-spawner.ts to pass config.jobNamespace and jobSpec as positional
parameters to client.batch.createNamespacedJob (i.e.,
client.batch.createNamespacedJob(config.jobNamespace, jobSpec)) while leaving
the surrounding try/catch and JobSpawnerError handling (including status
extraction and message) intact.

In `@src/k8s/shared-runner-dispatcher.ts`:
- Around line 203-213: The type guard isSuccessResponse currently omits
validating the status field; update it to also check that v["status"] ===
"success" (matching RunnerSuccessResponse's allowed values) so the guard only
returns true for genuine success responses and prevents unsafe access to
parsed.body.status elsewhere (e.g., where parsed.body.status is read). Locate
and modify the isSuccessResponse function to include typeof v["status"] ===
"string" && v["status"] === "success" (or equivalent strict check) in the
returned predicate.

In `@src/webhook/router.ts`:
- Around line 298-314: Both dispatchToSharedRunner and spawnIsolatedJob paths
never release the concurrency slot that processRequest increments, causing slot
leaks on both success and error; update the isolated-job and shared-runner
dispatch code to call the same slot-release function used by the inline path
(e.g., releaseConcurrencySlot or decrementConcurrencySlot) immediately after
successful await dispatch and also in all error branches (including the
infra-absent branch that calls recordInfraAbsentRejection and before rethrowing
other JobSpawnerError kinds), so the slot is always released regardless of
outcome; modify the try/catch around spawnIsolatedJob and the
dispatchToSharedRunner call site to ensure the slot-release is invoked in
finally-like semantics.

---

Nitpick comments:
In `@src/k8s/job-entrypoint.ts`:
- Around line 81-94: The current validation enforces webhookSecret even though
App can be constructed without it for getInstallationOctokit(); update the check
to only require config.appId and config.privateKey, and when building the App
instance pass webhookSecret conditionally (include webhookSecret:
config.webhookSecret only if defined) so getInstallationOctokit still works
without a webhook secret; reference config.appId, config.privateKey,
config.webhookSecret, the App constructor, and getInstallationOctokit when
making this change and add a brief inline comment noting webhookSecret is only
needed for webhook verification.

In `@src/k8s/job-spawner.ts`:
- Around line 160-170: The init container "wait-for-docker" is using the
application image (image variable) just to run a shell loop; change it to a
minimal utility image (e.g., "busybox" or "docker:27-cli") instead of using
image to avoid pulling the large app image. Update the initContainers entry (the
one with name "wait-for-docker" in job-spawner.ts) to set image to the chosen
minimal image, keep the command and env (DOCKER_HOST) intact, and adjust
imagePullPolicy if needed for your registry.
- Around line 112-136: Replace the plain-text environment variable entries for
API credentials with secret references: instead of pushing objects with value
set from config.anthropicApiKey and config.claudeCodeOauthToken into providerEnv
via providerEnv.push(...), add entries using valueFrom.secretKeyRef that
reference a Kubernetes Secret (e.g., name "claude-credentials" with keys like
"anthropic-api-key" and "claude-code-oauth-token"); ensure the code paths that
currently check config.anthropicApiKey and config.claudeCodeOauthToken switch to
creating secret-based env entries and document/create the required Secret in the
namespace before Job creation so credentials are not embedded in the Job spec or
etcd.

🪄 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: 4af10b0f-f5e4-4ef2-87ce-dc8aa2edc2fe

📥 Commits

Reviewing files that changed from the base of the PR and between d0533eb and 4ec9b09.

📒 Files selected for processing (10)

• src/core/prompt-builder.ts
• src/k8s/classifier.ts
• src/k8s/job-entrypoint.ts
• src/k8s/job-spawner.ts
• src/k8s/shared-runner-dispatcher.ts
• src/webhook/router.ts
• test/contract/shared-runner-internal.test.ts
• test/core/prompt-builder.test.ts
• test/k8s/classifier.test.ts
• test/webhook/router.test.ts

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

@kubernetes/client-node 1.4.0 createNamespacedJob API signature

💡 Result:

The API signature for createNamespacedJob in @kubernetes/client-node version 1.4.0, on the BatchV1Api class, is: typescript createNamespacedJob(namespace: string, body: V1Job, options?: { dryRun?: string, fieldManager?: string, fieldValidation?: string, pretty?: string }): Promise In version 1.x (including 1.4.0), it accepts a single options object as the first parameter for positional calls like listNamespacedPod({ namespace: 'default' }), but for createNamespacedJob, sources show usage as createNamespacedJob(namespace, body) in older examples, with a change noted around v1.0 where it expects {namespace: "...", body: ...}. Example usage: const k8s = require('@kubernetes/client-node'); const kc = new k8s.KubeConfig; kc.loadFromDefault; const k8sBatchV1Api = kc.makeApiClient(k8s.BatchV1Api); const job = { apiVersion: 'batch/v1', kind: 'Job', metadata: { name: 'my-job' }, spec: { template: { spec: { containers: [...], restartPolicy: 'Never' } } } }; k8sBatchV1Api.createNamespacedJob('default', job) .then((response) => { console.log(response.body); }) .catch((err) => { console.error(err); }); Note: In some v1.0+ contexts, use createNamespacedJob({namespace: 'default', body: job}) to avoid "namespace was null" errors, as per recent usage reports. The library is generated from Kubernetes OpenAPI, matching the standard create namespaced resource pattern: POST /apis/batch/v1/namespaces/{namespace}/jobs with body V1Job.

Citations:

• 1: https://www.npmjs.com/package/@kubernetes/client-node
• 2: https://stackoverflow.com/questions/72659209/kubernetes-client-job-create
• 3: https://github.com/kubernetes-client/javascript
• 4: https://github.com/kubernetes-client/javascript/releases
• 5: https://stackoverflow.com/questions/79436997/createnamespacedjob-fails-with-namespace-was-null-or-undefined
• 6: https://stackoverflow.com/questions/56990265/how-can-i-generate-a-v1job-object-for-the-kubernetes-nodejs-api-client-from-a

---

Fix createNamespacedJob API call to use positional parameters.

The @kubernetes/client-node 1.4.0 BatchV1Api.createNamespacedJob method signature is:

createNamespacedJob(namespace: string, body: V1Job, options?: {...}): Promise<V1Job>

The code currently calls it with an object pattern { namespace: config.jobNamespace, body: jobSpec }, but the API expects positional parameters. Change to:

await client.batch.createNamespacedJob(config.jobNamespace, jobSpec);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/k8s/job-spawner.ts` around lines 227 - 242, The createNamespacedJob call
uses an object parameter but the BatchV1Api.createNamespacedJob signature
expects positional args; update the call in job-spawner.ts to pass
config.jobNamespace and jobSpec as positional parameters to
client.batch.createNamespacedJob (i.e.,
client.batch.createNamespacedJob(config.jobNamespace, jobSpec)) while leaving
the surrounding try/catch and JobSpawnerError handling (including status
extraction and message) intact.

[chrisleekr]

Thanks @coderabbitai. Your review was against commit 4ec9b09 (pre-fix). Status of each actionable item:

#	File	Finding	Status
1	job-spawner.ts:227	createNamespacedJob positional vs object param	False positive. @kubernetes/client-node@1.4.0 re-exports ObjectBatchV1Api as BatchV1Api (see dist/gen/index.d.ts). The installed consumer-facing class takes an object — the current object-form call is correct. Verified via dist/gen/types/ObjectParamAPI.d.ts:12699.
2	shared-runner-dispatcher.ts:203	isSuccessResponse missing status check	Already fixed in 85e93e5
3	router.ts:298-314	Concurrency leak on shared-runner / isolated-job	Already fixed in 85e93e5

Nitpicks:

• job-entrypoint.ts webhookSecret requirement — Addressed in 251ecdb (now conditional; webhookSecret is only needed for webhook verification, not installation-token issuance).
• job-spawner.ts init container image — Already fixed in 85e93e5 (now docker:27-cli).
• job-spawner.ts plain-text credential env vars — Valid security follow-up; acknowledged. Deferred to a dedicated Slice (secretKeyRef wiring needs a Secret-provisioning story and is out of scope for this MVP PR).

CI green on the latest commit. Proceeding to squash-merge per standing auth once checks clear.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@chrisleekr Thanks for the thorough disposition — all clear on my end:

• Finding #1 (false positive): Noted and appreciated. I'll record the @kubernetes/client-node@1.4.0 API shape to avoid flagging that pattern in future reviews.
• Findings #2 & #3 + all nitpicks: Confirmed fixed. The conditional webhookSecret approach in 251ecdb is the right call — it was indeed only needed for webhook verification, not installation-token issuance.
• Plain-text env vars: Reasonable MVP deferral. The secretKeyRef wiring is a meaningful security hardening task that warrants its own story and a proper Secret-provisioning design, so scoping it out here makes sense.

Good luck with the squash-merge! 🎉

---

✏️ Learnings added

Learnt from: chrisleekr
URL: https://github.com/chrisleekr/github-app-playground/pull/19

Timestamp: 2026-04-15T10:00:29.656Z
Learning: In the repository `chrisleekr/github-app-playground`, `kubernetes/client-node1.4.0` re-exports `ObjectBatchV1Api` as `BatchV1Api`. The consumer-facing `BatchV1Api.createNamespacedJob` takes a single object parameter (object-form call), not positional arguments. Flagging it as positional-param usage is a false positive. Verified via `dist/gen/types/ObjectParamAPI.d.ts:12699`.

---

Learnt from: chrisleekr
URL: https://github.com/chrisleekr/github-app-playground/pull/19

Timestamp: 2026-04-15T10:00:29.656Z
Learning: In `src/k8s/job-spawner.ts` (chrisleekr/github-app-playground), injecting provider credentials (e.g., `ANTHROPIC_API_KEY`, `CLAUDE_CODE_OAUTH_TOKEN`, AWS keys) as plain-text env vars in the Kubernetes Job spec is a known MVP shortcut. The proper fix — wiring `secretKeyRef` to a Kubernetes Secret — is intentionally deferred to a future Slice that includes a Secret-provisioning story, and should not be treated as a blocker for the current PR.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

[chrisleekr]

🎉 This PR is included in version 1.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀