fix(pr-review): a pull request session reads its own repository

apps/ai/src/chat/code-mode.ts

@@ -51,6 +51,7 @@ export const CODE_MODE_TOOLS: ReadonlySet<string> = new Set([
 	"list_integrations",
 	"list_security_audit_runs",
 	"list_security_findings",
+	"list_source_repositories",
 	"mine_log_patterns",
 	"query_data",
 	"run_sql",

apps/ai/src/chat/maple-toolkit.test.ts

@@ -58,7 +58,7 @@ describe("buildChatToolkit", () => {
 		])
 	})
 
-	it("leaves every other tool's arguments as the model sent them", async () => {
+	it("reads the session's repository whatever repository the model names", async () => {
 		const { executor, calls } = recordingExecutor()
 		const { handlers } = buildChatToolkit(
 			executor,
@@ -73,8 +73,60 @@ describe("buildChatToolkit", () => {
 		const read = { repository: "MapleTechLabs/other", ref: "main", path: "a.ts" }
 
 		assert.isDefined(handlers.sandbox_read_file)
+		assert.isDefined(handlers.search_source_code)
 		await Effect.runPromise(handlers.sandbox_read_file(read, {}))
+		await Effect.runPromise(
+			handlers.search_source_code({ repository: "octo/private", query: "token" }, {}),
+		)
+
+		assert.deepEqual(calls, [
+			{ name: "sandbox_read_file", params: { ...read, repository: PULL_REQUEST.repository } },
+			{ name: "search_source_code", params: { repository: PULL_REQUEST.repository, query: "token" } },
+		])
+	})
+
+	it("leaves a telemetry tool's arguments as the model sent them", async () => {
+		const { executor, calls } = recordingExecutor()
+		const { handlers } = buildChatToolkit(
+			executor,
+			tenantFor(human),
+			PR_REVIEW_RULESET,
+			"chat",
+			undefined,
+			undefined,
+			undefined,
+			PULL_REQUEST,
+		)
+
+		assert.isDefined(handlers.list_services)
+		await Effect.runPromise(handlers.list_services({ environment: "production" }, {}))
+
+		assert.deepEqual(calls, [{ name: "list_services", params: { environment: "production" } }])
+	})
+
+	it("offers no tool that names a repository when the pull request could not be read", () => {
+		const { executor } = recordingExecutor()
+		const { handlers } = buildChatToolkit(
+			executor,
+			tenantFor(human),
+			PR_REVIEW_RULESET,
+			"chat",
+			undefined,
+			undefined,
+			undefined,
+			"unreadable",
+		)
+
+		assert.isUndefined(handlers.pr_file_diff)
+		assert.isUndefined(handlers.sandbox_exec)
+		assert.isUndefined(handlers.read_source_file)
+		assert.isDefined(handlers.list_services)
+	})
+
+	it("does not let a review list the organization's other repositories", () => {
+		const { executor } = recordingExecutor()
+		const { handlers } = buildChatToolkit(executor, tenantFor(human), PR_REVIEW_RULESET, "chat")
 
-		assert.deepEqual(calls, [{ name: "sandbox_read_file", params: read }])
+		assert.isUndefined(handlers.list_source_repositories)
 	})
 })

apps/ai/src/chat/maple-toolkit.ts

@@ -10,14 +10,32 @@ import type { McpToolSurface } from "@maple/domain/mcp-manifest"
 import type { McpToolExecutorApi } from "../mcp/dispatcher"
 import { buildMapleToolkit, type ToolUiPayload, type UiSource } from "../mcp/tools/llm-tools"
 
-/** The pull request a review session is about, which its diff tools always read. */
+/** The pull request a review or reply session is about: the one its tools read, whatever the model names. */
 export interface SessionPullRequest {
 	readonly repository: string
 	readonly number: number
 }
 
+/**
+ * What a session about a pull request may read. `unreadable` is one whose pull request could not
+ * be looked up: it is offered no tool that names a repository, rather than every repository.
+ */
+export type SessionRepositoryScope = SessionPullRequest | "unreadable"
+
 const PULL_REQUEST_TOOLS: ReadonlySet<string> = new Set(["pr_changed_files", "pr_context", "pr_file_diff"])
 
+/** Tools that read whichever connected repository their `repository` argument names. */
+const REPOSITORY_TOOLS: ReadonlySet<string> = new Set([
+	"read_source_file",
+	"sandbox_exec",
+	"sandbox_grep",
+	"sandbox_list_files",
+	"sandbox_read_file",
+	"search_source_code",
+])
+
+const namesRepository = (tool: string): boolean => PULL_REQUEST_TOOLS.has(tool) || REPOSITORY_TOOLS.has(tool)
+
 /**
  * All Maple tools, with mutating ones gated.
  *
@@ -33,23 +51,27 @@ export const buildChatToolkit = (
 	sessionAttributes?: Readonly<Record<string, string>>,
 	onAnswer?: (tool: string, answer: string) => void,
 	onUi?: (toolCallId: string, ui: ToolUiPayload, call: UiSource) => void,
-	pullRequest?: SessionPullRequest,
+	pullRequest?: SessionRepositoryScope,
 ) =>
 	buildMapleToolkit(executor, tenant, {
 		surface,
 		...(sessionAttributes === undefined ? undefined : { sessionAttributes }),
 		...(onAnswer === undefined ? undefined : { onAnswer }),
 		...(onUi === undefined ? undefined : { onUi }),
-		...(pullRequest === undefined
+		...(pullRequest === undefined || pullRequest === "unreadable"
 			? undefined
 			: {
 					pinned: (name: string) =>
 						PULL_REQUEST_TOOLS.has(name)
 							? { repository: pullRequest.repository, number: pullRequest.number }
-							: undefined,
+							: REPOSITORY_TOOLS.has(name)
+								? { repository: pullRequest.repository }
+								: undefined,
 				}),
 		// `deny` means the model never sees the tool. That is a stronger guarantee than refusing the
 		// call afterwards, and it is free — an unoffered tool cannot be called.
-		include: (name) => evaluatePermission(ruleset, name) !== "deny",
+		include: (name) =>
+			evaluatePermission(ruleset, name) !== "deny" &&
+			!(pullRequest === "unreadable" && namesRepository(name)),
 		gate: (name) => evaluatePermission(ruleset, name) === "ask",
 	})

apps/ai/src/chat/permissions.ts

@@ -65,7 +65,6 @@ export const PR_REVIEW_TOOLS: ReadonlyArray<string> = [
 	"sandbox_list_files",
 	"sandbox_read_file",
 	"sandbox_exec",
-	"list_source_repositories",
 	"search_source_code",
 	"read_source_file",
 	"list_services",

apps/ai/src/chat/run.ts

@@ -45,7 +45,7 @@ import {
 } from "./events"
 import { buildReviewFanout } from "./review/fanout"
 import { buildCodeMode, CODE_MODE_GUIDANCE, CODE_MODE_TOOLS } from "./code-mode"
-import { buildChatToolkit, type SessionPullRequest } from "./maple-toolkit"
+import { buildChatToolkit, type SessionRepositoryScope } from "./maple-toolkit"
 import { accumulateUsage, type RunUsage } from "./usage"
 import type { MakeCompletion } from "./completion"
 import type { TenantContext } from "@maple/domain/http"
@@ -111,11 +111,10 @@ export interface ChatRunInput {
 	/** This run is an autonomous pass's close-out: a report it files is a partial. */
 	readonly closeOut?: boolean
 	/**
-	 * The pull request a review session is about. Its diff tools read this one whatever
-	 * repository or number the model passes: a wrong guess there failed the call, and enough of
-	 * those stopped the review.
+	 * The pull request a review or reply session is about. Its diff and repository tools read
+	 * this one whatever repository or number the model passes; `unreadable` withholds them.
 	 */
-	readonly pullRequest?: SessionPullRequest
+	readonly pullRequest?: SessionRepositoryScope
 	/** The agent to run as; defaults to the session's. The local review runner passes a variant. */
 	readonly agent?: AgentDefinition
 	/** Runs `run_code` programs for an agent that offers it; absent, the tool is not offered. */

apps/ai/src/chat/turn-runner.ts

@@ -54,6 +54,7 @@
 import { makeUnfinishedSettler, runAutonomousTurn, settleAbandonedTurn, settleRow } from "./autonomous-settle"
 import { AGENTS, isAutonomousTurn, sessionOf } from "./agents"
 import { profileForTurn } from "./profiles"
+import type { SessionRepositoryScope } from "./maple-toolkit"
 import { runChatTurn } from "./run"
 import { toTenantContext, turnTenant } from "./turn-actor"
 import { dataSourcesPromptSection } from "../mcp/lib/data-sources"
@@ -99,6 +100,7 @@
 /** What the row records when the pass and its close-out both ended in prose or on an error. */
 const NO_DIAGNOSIS_ERROR = "no_diagnosis: the agent ended its pass without submitting a diagnosis; retry"
 const NO_REPLY_ERROR = "no_reply: the agent ended its pass without submitting an answer"
+const PULL_REQUEST_UNREADABLE = "The pull request this session is about could not be read."
 
 /**
  * Meter what this turn spent into the org's AI usage.
@@ -361,7 +363,7 @@
 				input.session.endTurn(input.messageId, { reason: "error", error: refusal.value.message })
 				return yield* annotateTurn()
 			}
-			// Read before the run: the diff tools are pinned to it. An unreadable target reviews unpinned.
+			// Read before the run: the diff and repository tools are held to it.
 			const target =
 				prReviewId === undefined
 					? Option.none()
@@ -376,6 +378,39 @@
 								),
 							),
 						)
+			const replyTarget =
+				prReplyId === undefined
+					? Option.none()
+					: yield* conversations.replyTarget(tenant.orgId, prReplyId).pipe(
+							Effect.catch((error) =>
+								Effect.logInfo("Could not read the reply's target").pipe(
+									Effect.annotateLogs({
+										sessionId: input.sessionId,
+										error: error.message,
+									}),
+									Effect.as(Option.none()),
+								),
+							),
+						)
+			// A session about a pull request reads that repository, or none when it cannot be named.
+			const pullRequest: SessionRepositoryScope | undefined =
+				prReviewId === undefined && prReplyId === undefined
+					? undefined
+					: Option.match(
+							Option.orElse(target, () => replyTarget),
+							{
+								onNone: (): SessionRepositoryScope => "unreadable",
+								onSome: ({ repository, number }) => ({ repository, number }),
+							},
+						)
+			// An unattended pass that cannot name its pull request has nothing it may read.
+			if (pullRequest === "unreadable" && autonomous) {
+				observability.outcome = "error"
+				yield* failUnfinished("start_failed", PULL_REQUEST_UNREADABLE)
+				observability.failureReason = "PullRequestUnreadable"
+				input.session.endTurn(input.messageId, { reason: "error", error: PULL_REQUEST_UNREADABLE })
+				return yield* annotateTurn()
+			}
 			// Clone the commit while the model reads the diff, rather than when its first source tool
 			// asks and waits on it. A child of the turn (no request Scope here, so `forkRequestScoped`
 			// parents it on this fiber): a clone still running when the turn ends carries on in the container.
@@ -564,10 +599,7 @@
 					...(turn.closeOut === true ? { closeOut: true } : undefined),
 					...(turn.agent === undefined ? undefined : { agent: turn.agent }),
 					...(codeExecutor === undefined ? undefined : { codeExecutor }),
-					...Option.match(target, {
-						onNone: () => undefined,
-						onSome: ({ repository, number }) => ({ pullRequest: { repository, number } }),
-					}),
+					...(pullRequest === undefined ? undefined : { pullRequest }),
 					text: turn.text,
 					history: turn.history,
 					usage,

apps/ai/src/mcp/tools/source-code.test.ts

@@ -131,6 +131,26 @@ describe("the source-code tools", () => {
 		}),
 	)
 
+	it.effect("refuses a search path that would add a qualifier of its own", () =>
+		Effect.gen(function* () {
+			const searched: Array<string> = []
+			const error = yield* Effect.flip(
+				call(
+					"search_source_code",
+					{ repository: "octo/shop", query: "three", path: "src repo:octo/other" },
+					{
+						searchCode: (_org, repo) => {
+							searched.push(repo)
+							return Effect.succeed([])
+						},
+					},
+				),
+			)
+			assert.strictEqual(error._tag, "@maple/mcp/errors/McpInvalidInputError")
+			assert.deepStrictEqual(searched, [])
+		}),
+	)
+
 	it("splits search paths and finds match lines", () => {
 		assert.deepStrictEqual(splitSearchPath("apps/api/"), { dir: "apps/api" })
 		assert.deepStrictEqual(splitSearchPath("README.md"), { filename: "README.md" })

apps/ai/src/mcp/tools/source-code.ts

@@ -133,9 +133,10 @@ export function registerSourceCodeTools(server: McpToolRegistrar) {
 					parameter: "query",
 				})
 			}
-			if (path !== undefined && unsafePath(path)) {
+			// The path becomes search qualifiers, so a space in it would start a qualifier of its own.
+			if (path !== undefined && (unsafePath(path) || /\s/.test(Str.trim(path)))) {
 				return yield* new McpInvalidInputError({
-					message: "path must be repository-relative",
+					message: "path must be repository-relative and contain no whitespace",
 					parameter: "path",
 				})
 			}

packages/backend/src/services/pr-review/PrReviewConversationService.test.ts

@@ -365,6 +365,18 @@ describe("PrReviewConversationService", () => {
 			assert.deepEqual(recorded.posted, [
 				{ body: "Because two writers race on the cursor.", threadRootId: "800" },
 			])
+			// The session's tools are held to this pull request, whatever its comments name.
+			assert.deepEqual(
+				yield* conversations.replyTarget(orgId, outcome.replyId!),
+				Option.some({
+					repository: "octo/repo",
+					number: 612,
+				}),
+			)
+			assert.deepEqual(
+				yield* conversations.replyTarget(asOrgId("org_other"), outcome.replyId!),
+				Option.none(),
+			)
 			// Nothing staged on an ask, and a second submission posts nothing.
 			assert.include(
 				yield* conversations.stageEdit(orgId, outcome.replyId!, {

packages/backend/src/services/pr-review/PrReviewConversationService.ts

@@ -87,6 +87,14 @@ export interface PrReviewConversationServiceApi {
 	 * pull request's head is still the run's, its review starts as a trusted author's would. Never fails.
 	 */
 	readonly onCiApproved: (orgId: OrgId, job: PullRequestCiApprovedJob) => Effect.Effect<void>
+	/** The pull request a reply answers on, which its session's tools are held to. */
+	readonly replyTarget: (
+		orgId: OrgId,
+		replyId: PrReviewReplyId,
+	) => Effect.Effect<
+		Option.Option<{ readonly repository: string; readonly number: number }>,
+		PrReviewPersistenceError
+	>
 	/** Post the agent's answer, committing a `fix`'s staged edits first. */
 	readonly submitReply: (
 		orgId: OrgId,
@@ -303,6 +311,22 @@ export class PrReviewConversationService extends Context.Service<
 				),
 			)
 
+		const replyTarget: PrReviewConversationServiceApi["replyTarget"] = Effect.fn(
+			"PrReviewConversationService.replyTarget",
+		)(function* (orgId, replyId) {
+			const row = yield* getReply(orgId, replyId).pipe(
+				Effect.asSome,
+				Effect.catchTag("@maple/http/pr-review/PrReviewReplyNotFoundError", () =>
+					Effect.succeed(Option.none<PrReviewReplyRow>()),
+				),
+			)
+			if (Option.isNone(row)) return Option.none()
+			const repo = yield* repositories
+				.getRepositoryById(orgId, row.value.repositoryId)
+				.pipe(Effect.mapError(toPersistence))
+			return Option.map(repo, ({ fullName }) => ({ repository: fullName, number: row.value.number }))
+		})
+
 		/** Post on the pull request where the row says; a failure is logged, never thrown into the turn. */
 		const post = Effect.fn("PrReviewConversationService.post")(function* (
 			orgId: OrgId,
@@ -840,6 +864,7 @@ export class PrReviewConversationService extends Context.Service<
 		return {
 			onPullRequestComment,
 			onCiApproved,
+			replyTarget,
 			submitReply,
 			stageEdit,
 			failReply,