feat(bot): PAT override + artifact sandbox + secret-exfil hardening

[chrisleekr]

Summary

Four independent bot fixes bundled into one branch:

1. Optional GitHub PAT override. New GITHUB_PERSONAL_ACCESS_TOKEN config bypasses the App installation token for every GitHub API/git operation — useful for solo/dev installs and to side-step App-token rate limits. Single-tenant only: ALLOWED_OWNERS must contain exactly one owner (mirrors the existing CLAUDE_CODE_OAUTH_TOKEN constraint), enforced at startup via assertPatRequiresAllowlist. Resolution centralised in src/core/github-token.ts:resolveGithubToken(). The PAT changes API + push identity, but commit author/committer metadata stays pinned to chrisleekr-bot[bot] by src/core/checkout.ts. Orchestrator skips getRepoInstallation + getInstallationOctokit when the PAT is set, saving two GitHub API calls per job offer.

2. Mermaid diagram now required for behaviour/flow changes in bot:implement PR descriptions. Previous prompt told the agent to skip the Diagram section "unless behaviour or flow actually changes" — too lax. Tightened to require a Mermaid block whenever code paths, state, external calls, error handling, or sequencing shift. Pure refactors / typo fixes / test-only changes still skip.

3. IMPLEMENT.md / REVIEW.md / RESOLVE.md no longer get committed to target repos. Pre-fix the agent wrote these summary files at the repo root inside the cloned checkout, so they ended up in PR commits. Pipeline now creates a sibling ${workDir}-artifacts directory outside the checkout, exposed to the agent as BOT_ARTIFACT_DIR. readCapturedFiles reads from there; both directories are removed in the same finally block. mkdirSync(artifactsDir) lives inside the try block so cleanup() still fires for workDir if mkdir throws.

4. Secret-exfiltration hardening (closes security(pipeline): Claude Code subprocess inherits all daemon secrets via process.env spread in buildProviderEnv #102). Four-layer defence against agent-side prompt-injection that tries to make the bot post secrets back to GitHub:

   • Layer 1 — subprocess env allowlist (buildProviderEnv in src/core/executor.ts): explicit allowlist + prefix patterns + deny-set replaces wildcard ...process.env. Banned daemon secrets (GITHUB_APP_PRIVATE_KEY, GITHUB_WEBHOOK_SECRET, DAEMON_AUTH_TOKEN[_PREVIOUS], DATABASE_URL, VALKEY_URL, REDIS_URL, CONTEXT7_API_KEY, GITHUB_PERSONAL_ACCESS_TOKEN) cannot reach the agent CLI. Sets CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1 so grandchildren inherit no creds.
   • Layer 2 — input spotlighting (src/utils/sanitize.ts + src/core/prompt-builder.ts): user-controlled fields wrapped in nonce-bracketed delimiters; triggerUsername rejected (not stripped) when it contains whitespace to block git commit-trailer forging.
   • Layer 3 — output regex chokepoint (src/utils/github-output-guard.ts:safePostToGitHub): every GitHub-bound write goes through redactSecrets(); matched bytes silently stripped (no [REDACTED_X] markers — markers leak probing signal). Skip-and-error when redaction empties the body.
   • Layer 4 — LLM output scanner (src/utils/llm-output-scanner.ts): catches encoded/obfuscated secrets the regex misses, with per-call nonce tag names to block tag-close injection. Default ON for source: "agent", fail-open on Bedrock outage. New H7 daemon-secret-leak heuristic in src/config.ts.

   Phase-1 wiring: tracking-comment.ts (create + update), both scoped daemon executors, both ship/scoped reply paths. Phase-2 callsites tracked in CLAUDE.md § Coverage status. Default scanner + triage models bumped to sonnet-4-6.

Diagram

flowchart TB
    classDef before fill:#7a1f1f,stroke:#3d0f0f,color:#ffffff
    classDef after  fill:#1f5a3a,stroke:#0f3b25,color:#ffffff
    classDef shared fill:#3d3d3d,stroke:#1f1f1f,color:#ffffff

    Webhook["webhook delivery"]:::shared --> Pipeline["pipeline.runPipeline"]:::shared

    Pipeline --> BeforeAuth["BEFORE<br/>octokit.auth type=installation<br/>at every callsite"]:::before
    Pipeline --> AfterAuth["AFTER<br/>resolveGithubToken octokit<br/>PAT short-circuit when set<br/>orchestrator skips installation lookup"]:::after

    Pipeline --> BeforeFs["BEFORE<br/>workDir=clone-tempdir<br/>agent writes IMPLEMENT.md<br/>at repo root inside checkout<br/>file lands in PR commit"]:::before
    Pipeline --> AfterFs["AFTER<br/>workDir + sibling artifacts dir<br/>BOT_ARTIFACT_DIR env exported<br/>agent writes summaries OUTSIDE<br/>checkout cannot pick up via git add"]:::after

    Pipeline --> BeforeMermaid["BEFORE<br/>bot:implement prompt<br/>skip Diagram unless behaviour changes<br/>agent often skipped anyway"]:::before
    Pipeline --> AfterMermaid["AFTER<br/>bot:implement prompt<br/>Mermaid required when behaviour/flow shifts<br/>only omit for pure refactor/typo/test-only"]:::after

    Pipeline --> BeforeSecrets["BEFORE<br/>subprocess inherits process.env<br/>agent sees GITHUB_APP_PRIVATE_KEY<br/>DB_URL VALKEY_URL etc.<br/>writes posted directly to GitHub"]:::before
    Pipeline --> AfterSecrets["AFTER<br/>buildProviderEnv allowlist+denyset<br/>safePostToGitHub regex+LLM strip<br/>nonce-tagged spotlight on user input<br/>4 layers gate every reply"]:::after

Loading

Changes

• Config (src/config.ts): githubPersonalAccessToken, assertPatRequiresAllowlist() guard, LLM scanner settings (llmOutputScanner.*), H7 daemon-secret-leak detector heuristic, startup warn when PAT override is active.
• Token resolver (src/core/github-token.ts, new): single source for GitHub credential — PAT short-circuit or installation-token mint.
• Call-site swaps: pipeline.ts and orchestrator connection-handler.ts mint sites go through resolveGithubToken(). Orchestrator avoids getRepoInstallation + getInstallationOctokit when PAT is set.
• Pipeline (src/core/pipeline.ts): sibling ${workDir}-artifacts dir created inside try block, passed to executor, cleaned up alongside workDir in finally. readCapturedFiles reads from BOT_ARTIFACT_DIR.
• Executor (src/core/executor.ts): buildProviderEnv takes artifactsDir, exports BOT_ARTIFACT_DIR, replaces wildcard env passthrough with allowlist + prefix patterns + deny-set; sets CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1.
• Output guard (src/utils/github-output-guard.ts, new): safePostToGitHub({ body, source, callsite, log, post }) chokepoint — regex pass + optional LLM scanner, fails open on Bedrock outage, skips post + logs error when redaction empties the body, helper applyLlmRedaction keeps cyclomatic depth bounded.
• LLM scanner (src/utils/llm-output-scanner.ts, new): Bedrock/Anthropic single-turn classifier with per-call nonce tag names, JSON-only response shape, single retry on malformed JSON, stub injector for tests.
• Sanitize (src/utils/sanitize.ts): dual-mode — input spotlighting nonce delimiters + output redactSecrets() (GitHub tokens, AWS access keys, DB URLs stopping at markdown delimiters, JWT-shaped strings ≥20-char segments, RSA private-key blocks). Stateful regex reuse bug fixed.
• Prompt builder (src/core/prompt-builder.ts): nonce-spotlighted user fields; rejects whitespace in triggerUsername (commit-trailer forging vector).
• Wiring: tracking-comment.ts (create + update), scoped-explain-thread-executor.ts, scoped-fix-thread-executor.ts, workflows/ship/scoped/explain-thread.ts, workflows/ship/scoped/fix-thread.ts route writes through safePostToGitHub. updateTrackingComment now throws on skip instead of silently no-oping.
• MCP servers (comment.ts, inline-comment.ts): inline redactSecrets() + console.error log (subprocess can't import the daemon-config-bound chokepoint).
• Workflow prompts: implement.ts / review.ts / resolve.ts summary writes point at $BOT_ARTIFACT_DIR/<file>.md with explicit "do NOT git add" warning; implement.ts Mermaid rule tightened.
• K8s spawner (src/k8s/ephemeral-daemon-spawner.ts): env propagation aligned with the new allowlist semantics.
• Model defaults: scanner + triage default model bumped from claude-3-5-haiku to sonnet-4-6 alias added to MODEL_MAP; snapshot test updated.
• Tests: test/config.test.ts (4 new PAT-allowlist cases), test/core/github-token.test.ts (3 cases), test/core/build-provider-env.test.ts (12 cases — banned-key denial, prefix-allow, BOT_ARTIFACT_DIR, blank-credential scrub), test/utils/github-output-guard.test.ts (6 cases — clean post, regex strip, empty-after-redact skip, LLM fail-open, LLM strip, system-source no-LLM), test/utils/sanitize.test.ts extended with output-redaction cases, test/preload.ts clears GITHUB_PERSONAL_ACCESS_TOKEN to prevent local .env leak.
• Docs: CLAUDE.md adds "Security invariants" section + Authentication options GitHub-credential subsection; docs/operate/configuration.md adds GITHUB_PERSONAL_ACCESS_TOKEN, LLM scanner env vars, "Subprocess env allowlist" subsection; docs/build/architecture.md updated; docs/use/workflows/{implement,review,resolve}.md Artifact rows reflect $BOT_ARTIFACT_DIR.
• Drift cleanup: removed tracked IMPLEMENT.md, REVIEW.md, RESOLVE.md from the repo root.
• .env.example: adds GITHUB_PERSONAL_ACCESS_TOKEN + LLM scanner vars.

Related Issues

• Closes security(pipeline): Claude Code subprocess inherits all daemon secrets via process.env spread in buildProviderEnv #102 (subprocess env hardening + output-side secret redaction)
• Bundle of independent fixes (PAT override, artifact sandbox, mermaid prompt) — no other tracked issue.

Test plan

• bun run typecheck clean
• bun run lint 0 errors in modified files
• bun test test/core/build-provider-env.test.ts test/utils/github-output-guard.test.ts — 21/21 pass
• Full core+utils+config suite: 296 pass, 0 fail across 18 files
• bun run docs:build + check-docs-versions.ts + check-docs-citations.ts all green
• Manual smoke for PAT mode: set GITHUB_PERSONAL_ACCESS_TOKEN=ghp_xxx + single-owner ALLOWED_OWNERS, fire a webhook, verify resulting commits show the PAT user as author and bot identity stays on commit metadata.
• Manual smoke for artifact sandbox: trigger @chrisleekr-bot-dev bot:implement on a sandbox repo; verify IMPLEMENT.md is NOT in the PR file list and orchestrator log shows capturedFiles read from ${workDir}-artifacts. Repeat for bot:review (REVIEW.md) and a resolve flow (RESOLVE.md).
• Manual smoke for output guard: post a comment containing a fake ghp_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa token via tracking-comment path; verify the resulting GitHub comment has the bytes stripped and a scanner: "regex" warn line appears in the daemon log without the original token bytes.

Summary by CodeRabbit

• New Features

  • Added GitHub Personal Access Token (PAT) as an alternative authentication method to GitHub App installation tokens
  • Workflow-generated reports (Implement, Resolve, Review) now write to a temporary directory instead of the repository root

• Bug Fixes

  • Strengthened daemon authentication security with constant-time token comparison

• Documentation

  • Updated configuration references and workflow documentation for new authentication and artifact handling features

• Tests

  • Added test coverage for PAT validation, GitHub token resolution, and artifact directory handling

[coderabbitai]

Warning

Rate limit exceeded

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

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6b023738-396a-4039-9032-f81d8ffb204a

📥 Commits

Reviewing files that changed from the base of the PR and between e3ecf64 and e936915.

📒 Files selected for processing (30)

• .env.example
• CLAUDE.md
• docs/build/architecture.md
• docs/operate/configuration.md
• src/ai/llm-client.ts
• src/app.ts
• src/config.ts
• src/core/executor.ts
• src/core/formatter.ts
• src/core/prompt-builder.ts
• src/core/tracking-comment.ts
• src/daemon/scoped-explain-thread-executor.ts
• src/daemon/scoped-fix-thread-executor.ts
• src/k8s/ephemeral-daemon-spawner.ts
• src/mcp/servers/comment.ts
• src/mcp/servers/inline-comment.ts
• src/orchestrator/connection-handler.ts
• src/utils/github-output-guard.ts
• src/utils/llm-output-scanner.ts
• src/utils/sanitize.ts
• src/workflows/ship/scoped/explain-thread.ts
• src/workflows/ship/scoped/fix-thread.ts
• test/ai/llm-client.test.ts
• test/core/build-provider-env.test.ts
• test/core/github-token.test.ts
• test/core/prompt-builder.test.ts
• test/orchestrator/connection-handler.test.ts
• test/preload.ts
• test/utils/github-output-guard.test.ts
• test/utils/sanitize.test.ts

📝 Walkthrough

Walkthrough

This PR introduces two independent architectural improvements: (1) externalize workflow-generated reports (IMPLEMENT.md, RESOLVE.md, REVIEW.md) into a sibling ${workDir}-artifacts directory to prevent unintended git staging, and (2) add optional GitHub Personal Access Token (PAT) authentication as an override to installation tokens, with single-owner allowlist enforcement and centralized token resolution.

Changes

Artifacts Directory & Workflow Report Isolation

Layer / File(s)	Summary
Architecture & Concepts docs/build/architecture.md, CLAUDE.md	Document "One request, one clone" expanded with sibling ${workDir}-artifacts directory for agent-generated reports, exposed via BOT_ARTIFACT_DIR env var, and pipeline cleanup in finally block.
Pipeline Infrastructure src/core/pipeline.ts	Pipeline creates artifactsDir recursively, passes it to executeAgent, reads captured files from artifactsDir instead of workDir, and cleans up both directories in finally.
Executor & Subprocess Environment src/core/executor.ts	buildProviderEnv exports BOT_ARTIFACT_DIR from optional artifactsDir parameter; ExecuteAgentParams gains artifactsDir field; executeAgent forwards it into env construction.
Workflow Handler Prompts src/workflows/handlers/implement.ts, src/workflows/handlers/resolve.ts, src/workflows/handlers/review.ts	Agent prompts updated to direct report writes to $BOT_ARTIFACT_DIR/IMPLEMENT.md, $BOT_ARTIFACT_DIR/RESOLVE.md, and $BOT_ARTIFACT_DIR/REVIEW.md respectively, with explicit "outside repo" warnings.
Workflow Documentation docs/use/workflows/implement.md, docs/use/workflows/resolve.md, docs/use/workflows/review.md	Artifact output paths updated from repo-root files to $BOT_ARTIFACT_DIR/ variants, documented as non-committed sibling directories.
PR Template & Tests .github/PULL_REQUEST_TEMPLATE/bot-implement.md, test/core/build-provider-env.test.ts	PR template diagram spacing adjusted; new test suite validates BOT_ARTIFACT_DIR env export and token forwarding.

GitHub PAT Authentication & Token Resolution

Layer / File(s)	Summary
Configuration & Validation src/config.ts, test/config.test.ts	Add githubPersonalAccessToken optional field with nonEmptyOptionalString coercion; new assertPatRequiresAllowlist guard enforces exactly one ALLOWED_OWNERS entry when PAT is set; loadConfig reads GITHUB_PERSONAL_ACCESS_TOKEN env and emits startup warning.
Core Token Resolution src/core/github-token.ts, test/core/github-token.test.ts	New exported async function resolveGithubToken(octokit, pat?) centralizes credential selection: returns provided PAT, falls back to config.githubPersonalAccessToken, or mints installation token via octokit.auth(); tests cover PAT pass-through, installation fallback, and empty-string handling.
Executor & Subprocess Environment src/core/executor.ts	buildProviderEnv signature updated to accept optional artifactsDir; already forwards credential env vars without change (token source is caller responsibility).
Pipeline Token Integration src/core/pipeline.ts	Installation token resolved via new resolveGithubToken(ctx.octokit) instead of inline octokit.auth() call.
Connection Handler Token Minting src/orchestrator/connection-handler.ts	handleAccept and handleScopedAccept updated: PAT short-circuits token lookup (return config.githubPersonalAccessToken directly), otherwise mint via resolveGithubToken(octokit) instead of direct octokit.auth().
Configuration Documentation docs/operate/configuration.md, CLAUDE.md	Updated GitHub App credentials table to document GITHUB_PERSONAL_ACCESS_TOKEN as optional with single-owner ALLOWED_OWNERS requirement; added "GitHub credential" subsection describing PAT override semantics, git auth behavior, and commit-author immutability.
Implementation Record IMPLEMENT.md	Documents issue #76 resolution (constant-time bearer-token auth for daemon WebSocket); includes test commands, CI results, and runbook updates; obsolete records RESOLVE.md and REVIEW.md removed.

Sequence Diagram(s)

sequenceDiagram
    participant Pipeline as Pipeline<br/>(runPipeline)
    participant Ctx as Context<br/>(octokit, config)
    participant Resolver as resolveGithubToken<br/>(core/github-token.ts)
    participant SDK as Claude SDK<br/>(executor)
    participant Subprocess as Agent Subprocess<br/>(Claude Code CLI)
    participant Artifacts as Artifacts Dir<br/>(${workDir}-artifacts)

    Pipeline->>Resolver: resolveGithubToken(octokit)
    alt PAT configured
        Resolver-->>Pipeline: config.githubPersonalAccessToken
    else No PAT
        Resolver->>Ctx: octokit.auth({ type: "installation" })
        Ctx-->>Resolver: installation token
        Resolver-->>Pipeline: token
    end

    Pipeline->>Pipeline: mkdir(${workDir}-artifacts)
    Pipeline->>SDK: executeAgent({ artifactsDir })
    SDK->>SDK: buildProviderEnv(token, artifactsDir)
    SDK->>Subprocess: spawn with env { GH_TOKEN, GITHUB_TOKEN, BOT_ARTIFACT_DIR }
    Subprocess->>Artifacts: write IMPLEMENT.md, RESOLVE.md, REVIEW.md
    Subprocess-->>SDK: completion
    SDK->>Artifacts: readCapturedFiles(artifactsDir)
    Artifacts-->>SDK: report contents
    SDK-->>Pipeline: captured reports
    Pipeline->>Pipeline: rm(${workDir}-artifacts, recursive)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

The changes span multiple files across config, core execution, handlers, and tests with mixed complexity: configuration validation logic, async token resolution with conditional minting, filesystem operations in pipeline cleanup, and straightforward subprocess environment variable forwarding. The two cohorts are logically independent despite touching common files; artifact isolation is largely documentation and env-var threading, while PAT support requires understanding credential resolution flow and allowlist constraints. Test coverage is comprehensive for both features. No single file contains dense logic, but the distributed nature and credential sensitivity demand careful review of each token path and env-var forwarding.

Possibly related PRs

• chrisleekr/github-app-playground#63: Modifies bot-implement PR template and implement workflow handler prompts in parallel with this PR's artifact-directory and handler-prompt updates.
• chrisleekr/github-app-playground#96: Modifies src/core/pipeline.ts for baseBranch handling, touching the same central runPipeline function and executor integration points.
• chrisleekr/github-app-playground#91: Updates review and resolve workflow handlers and docs/use/workflows paths in parallel with this PR's similar changes.

Suggested labels

type: feature ✨, type: security 🔒, type: docs 📋

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the three main changes: PAT override for GitHub authentication, artifact sandboxing to prevent accidental commits, and secret-exfiltration hardening measures.
Linked Issues check	✅ Passed	The PR successfully addresses all coding requirements from issue #102: explicit env allowlist in buildProviderEnv, CLAUDE_CODE_SUBPROCESS_ENV_SCRUB=1 flag, extended secret redaction patterns, and comprehensive unit tests validating banned keys are excluded.
Out of Scope Changes check	✅ Passed	All changes align with stated PR objectives: PAT override implementation, artifact sandboxing via BOT_ARTIFACT_DIR, Mermaid diagram tightening, and secret-exfiltration hardening. No unrelated changes detected.

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

---

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

This PR bundles three bot-focused improvements: (1) a GitHub credential resolver that can optionally override the App installation token with a PAT (guarded by a single-owner allowlist), (2) tighter bot:implement PR-body guidance requiring Mermaid diagrams when behavior/flow changes, and (3) moving agent-authored summary artifacts (IMPLEMENT/REVIEW/RESOLVE) outside the cloned checkout so they don’t end up committed into target repos.

Changes:

• Add GITHUB_PERSONAL_ACCESS_TOKEN support via resolveGithubToken() plus a startup guard (assertPatRequiresAllowlist) enforcing single-tenant operation.
• Create a sibling ${workDir}-artifacts directory, plumb it through execution as BOT_ARTIFACT_DIR, and read captured summary files from there.
• Update implement/review/resolve prompts + PR template to reference $BOT_ARTIFACT_DIR and require Mermaid diagrams when behavior/flow changes; add unit tests covering token/env wiring.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/core/github-token.ts	New centralized resolver for PAT-vs-installation token selection.
src/config.ts	Adds PAT config field + single-tenant startup assertion + warning when override active.
src/orchestrator/connection-handler.ts	Routes token acquisition through resolveGithubToken() for dispatched jobs.
src/core/pipeline.ts	Creates/uses sibling artifacts dir; captured-file reads now come from artifacts dir; cleans up both dirs.
src/core/executor.ts	Exports buildProviderEnv, adds optional artifacts dir -> BOT_ARTIFACT_DIR export.
src/workflows/handlers/implement.ts	Tightens Mermaid requirements; writes IMPLEMENT summary to $BOT_ARTIFACT_DIR.
src/workflows/handlers/review.ts	Writes REVIEW summary to $BOT_ARTIFACT_DIR.
src/workflows/handlers/resolve.ts	Writes RESOLVE summary to $BOT_ARTIFACT_DIR.
.github/PULL_REQUEST_TEMPLATE/bot-implement.md	Updates Diagram section instructions to require Mermaid on behavior/flow changes.
docs/build/architecture.md	Documents artifacts dir and credential resolution behavior.
docs/operate/configuration.md	Documents GITHUB_PERSONAL_ACCESS_TOKEN and updates ALLOWED_OWNERS guidance.
CLAUDE.md	Updates pipeline/auth documentation, including PAT override description.
test/core/github-token.test.ts	Adds unit tests for PAT short-circuit vs installation mint fallback.
test/core/build-provider-env.test.ts	Adds tests for GH token env wiring, BOT_ARTIFACT_DIR, and blank-credential scrubbing.
test/config.test.ts	Adds tests for assertPatRequiresAllowlist.
IMPLEMENT.md	Removed drift file from repo root.
REVIEW.md	Removed drift file from repo root.
RESOLVE.md	Removed drift file from repo root.

[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.

[Copilot]

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.

[coderabbitai]

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/build/architecture.md`:
- Line 49: Update the sentence in the architecture doc that currently reads “bot
identity becomes the PAT owner” to avoid implying commit metadata changes:
explicitly state that src/core/github-token.ts:resolveGithubToken() will return
a PAT so the API/git authentication actor will act as the PAT owner, but commit
author/committer metadata is still controlled by the checkout configuration
(bot-pinned) and does not change. Mention both consumers (git credential helper
/ GH_TOKEN env / MCP server) use the resolved token for authentication and
clarify that only auth actor identity differs, not commit metadata.

In `@src/core/executor.ts`:
- Around line 55-63: Add two unit tests in test/core/build-provider-env.test.ts:
one that calls the env-building code with artifactsDir undefined (and another
with artifactsDir === "") and asserts that BOT_ARTIFACT_DIR is not present in
the returned object (verify absence), and a second test that sets
config.provider = "bedrock" with a non-empty artifactsDir and asserts both
BOT_ARTIFACT_DIR is present with the provided value and CLAUDE_CODE_USE_BEDROCK
=== "1" are present; locate the logic behind artifactsDir and BOT_ARTIFACT_DIR
in src/core/executor.ts (the artifactEnv variable and the branch checking
config.provider === "bedrock") and use those symbols to construct assertions.

In `@src/orchestrator/connection-handler.ts`:
- Around line 818-833: Collapse the duplicate PAT check by changing the call
site to pass a lazy octokit factory into resolveGithubToken instead of
performing the config.githubPersonalAccessToken check inline: remove the local
if/else branch in connection-handler.ts and call resolveGithubToken with an
async getOctokit function that calls getOrCreateApp(), fetches the repo
installation via app.octokit.rest.apps.getRepoInstallation({ owner, repo }) and
returns app.getInstallationOctokit(installation.id); then update
resolveGithubToken to accept getOctokit: () => Promise<Pick<Octokit, "auth">>
and internally prefer the passed pat or config.githubPersonalAccessToken before
invoking getOctokit() to request an installation token.
- Around line 818-833: Two code paths miss the PAT short-circuit and must be
updated to avoid minting installation tokens: in postOrphanNotification, and in
the tickle scheduler/octokitFactory used by resumeShipIntent (session-runner),
check if config.githubPersonalAccessToken !== undefined and if so use that PAT
for all GitHub API calls (skip calling app.octokit.rest.apps.getRepoInstallation
and app.getInstallationOctokit) so comments, reactions and scheduler actions are
performed with the PAT user; otherwise retain the existing installation-token
logic (mirror the pattern used in handleJobOfferAccept).

In `@test/core/build-provider-env.test.ts`:
- Around line 5-9: Add explicit regression assertions to the existing test for
buildProviderEnv to verify that high-risk secret keys are never forwarded: call
buildProviderEnv(...) in the same test and assert the returned env object does
NOT contain DAEMON_AUTH_TOKEN, GITHUB_APP_PRIVATE_KEY, DATABASE_URL, and
VALKEY_URL. Place these negative assertions alongside the current positive
assertions that check keys the function sets so the test continues to validate
intended keys while preventing silent reintroduction of exfil paths.

In `@test/core/github-token.test.ts`:
- Around line 23-28: The test relies on the module-level config singleton
(config.githubPersonalAccessToken) which may be set by the environment; update
the suite to pin or mock that singleton so the case is deterministic: in the
"falls back..." test ensure config.githubPersonalAccessToken is undefined by
mocking the config module (e.g., use mock.module("../../src/config", () => ({
config: { githubPersonalAccessToken: undefined } })) or equivalent) or
save/restore process.env around the suite so resolveGithubToken reads the
intended value; target the resolveGithubToken call and the
config.githubPersonalAccessToken symbol when applying the change.

🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2e20a24f-17c5-4189-bbe7-760e44c119bb

📥 Commits

Reviewing files that changed from the base of the PR and between cae53bd and e3ecf64.

📒 Files selected for processing (21)

• .github/PULL_REQUEST_TEMPLATE/bot-implement.md
• CLAUDE.md
• IMPLEMENT.md
• RESOLVE.md
• REVIEW.md
• docs/build/architecture.md
• docs/operate/configuration.md
• docs/use/workflows/implement.md
• docs/use/workflows/resolve.md
• docs/use/workflows/review.md
• src/config.ts
• src/core/executor.ts
• src/core/github-token.ts
• src/core/pipeline.ts
• src/orchestrator/connection-handler.ts
• src/workflows/handlers/implement.ts
• src/workflows/handlers/resolve.ts
• src/workflows/handlers/review.ts
• test/config.test.ts
• test/core/build-provider-env.test.ts
• test/core/github-token.test.ts

💤 Files with no reviewable changes (3)

• RESOLVE.md
• REVIEW.md
• IMPLEMENT.md

[chrisleekr]

🎉 This PR is included in version 1.10.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀