diff --git a/client/src/components/cos/tabs/schedule/PipelineStageConfig.jsx b/client/src/components/cos/tabs/schedule/PipelineStageConfig.jsx
index fd9a89c01b..07b3912e5e 100644
--- a/client/src/components/cos/tabs/schedule/PipelineStageConfig.jsx
+++ b/client/src/components/cos/tabs/schedule/PipelineStageConfig.jsx
@@ -4,11 +4,27 @@ import {
effortAwareModelOptions,
effortSurvivingModel,
localBackendForProvider,
- toolFreeLocalSelectionPolicy,
+ publicReviewSelectionPolicy,
+ supportsPublicReviewPosture,
+ PUBLIC_REVIEW_ACTIONS_POSTURE,
+ PUBLIC_REVIEW_NO_TOOL_POSTURE,
} from '../../../../utils/providers';
import useLocalModels from '../../../../hooks/useLocalModels';
import ProviderModelSelector from '../../../ProviderModelSelector';
-import { pipelineStages } from './scheduleConstants';
+import ToggleSwitch from '../../../ToggleSwitch';
+import {
+ pipelineStages,
+ prReviewerStageRole,
+ stagePublicReviewPosture,
+ togglePrReviewerActions,
+} from './scheduleConstants';
+
+// Which providers on THIS install can enforce a posture. The server publishes
+// `publicReviewPostures` per provider, derived from the vendor rows, so the
+// picker never carries its own list of vendor names — an install with only
+// grok, only codex, or only a local Claude wrapper each get a correct list.
+const eligibleProvidersFor = (providers, posture) =>
+ (providers || []).filter((provider) => supportsPublicReviewPosture(provider, posture));
export default function PipelineStageConfig({ taskType, config, providers, onUpdate, updating, setUpdating }) {
const stages = pipelineStages(config);
@@ -16,17 +32,28 @@ export default function PipelineStageConfig({ taskType, config, providers, onUpd
const { ollama, lmstudio, capabilitiesByBackend, loading: localModelsLoading } = useLocalModels({
enabled: needsSecurityModelPolicy,
});
- // Stage 2 uses the same authoritative no-tool model predicate, but its
- // provider is a maintained local Claude wrapper rather than a direct HTTP
- // backend. The server-derived marker is the provider-side capability gate;
- // the shared policy still requires a local installed model with text
- // capability and no `tools` capability.
- const publicReviewSelectionPolicy = useMemo(
- () => toolFreeLocalSelectionPolicy(capabilitiesByBackend, {
- providerPredicate: (provider) => provider?.publicReviewSupported === true,
- }),
- [capabilitiesByBackend],
- );
+ // One policy per posture. The provider half is server-derived; the model half
+ // adds the authoritative no-tool capability check only for a local runtime,
+ // which is the only place PortOS can probe it.
+ const selectionPolicies = useMemo(() => ({
+ [PUBLIC_REVIEW_NO_TOOL_POSTURE]: publicReviewSelectionPolicy(PUBLIC_REVIEW_NO_TOOL_POSTURE, capabilitiesByBackend),
+ [PUBLIC_REVIEW_ACTIONS_POSTURE]: publicReviewSelectionPolicy(PUBLIC_REVIEW_ACTIONS_POSTURE, capabilitiesByBackend),
+ }), [capabilitiesByBackend]);
+
+ const hasActionsStage = stages.some((stage) => prReviewerStageRole(stage) === 'actions');
+
+ const handlePrReviewerActionsToggle = async (enabled) => {
+ setUpdating(true);
+ const updatedMeta = {
+ ...config.taskMetadata,
+ pipeline: {
+ ...config.taskMetadata.pipeline,
+ stages: togglePrReviewerActions(stages, enabled),
+ },
+ };
+ await onUpdate(taskType, { taskMetadata: updatedMeta }).catch(() => {});
+ setUpdating(false);
+ };
const handleStageUpdate = async (stageIndex, field, value) => {
setUpdating(true);
@@ -64,25 +91,58 @@ export default function PipelineStageConfig({ taskType, config, providers, onUpd
return (
Pipeline Stages
+ {needsSecurityModelPolicy && (
+
+
+
+
Run final code review and actions
+
+ When enabled, a sandbox-capable reviewer applies only the screened patch, runs local tests, and returns a structured review for the deterministic GitHub coordinator. It is nested here, not a separate scheduled task.
+
+
+ handlePrReviewerActionsToggle(!hasActionsStage)}
+ disabled={updating}
+ ariaLabel="Enable final code review and actions"
+ size="sm"
+ />
+
+ {!hasActionsStage && (
+
+ Disabled runs stop after the tool-free eligibility gate; no PR comments, issue filing, CI triggers, or merge actions occur.
+
+ )}
+
+ )}
{stages.map((stage, i) => {
const stageProvider = providers?.find(p => p.id === stage.providerId);
- const isSecurityStage = needsSecurityModelPolicy && i === 0;
- const isPublicReviewStage = needsSecurityModelPolicy && i === 1;
+ const role = needsSecurityModelPolicy
+ ? (prReviewerStageRole(stage) || (i === 0 ? 'security' : i === 1 ? 'eligibility' : 'actions'))
+ : null;
+ const isSecurityStage = role === 'security';
+ // The posture is read off the stage's own execution profile, so a
+ // custom pipeline that reuses one of these profiles gets the same
+ // gating without being a pr-reviewer stage.
+ const posture = isSecurityStage ? null : stagePublicReviewPosture(stage);
+ const isNoToolStage = posture === PUBLIC_REVIEW_NO_TOOL_POSTURE;
+ const isActionsStage = Boolean(posture) && !isNoToolStage;
+ const eligibleProviders = posture ? eligibleProvidersFor(providers, posture) : null;
const localBackend = localBackendForProvider(stageProvider);
const localModelIds = localBackend === 'ollama' ? ollama : localBackend === 'lmstudio' ? lmstudio : [];
- // Keep the richer capability object on each local option so the shared
- // policy does not need a second lookup. The status hook's ids are the
- // installed-model source of truth; a provider's stale catalog is never
- // enough to make a model eligible for a security scan.
- const stageModels = isPublicReviewStage
+ // A LOCAL provider's installed-model list is the source of truth (its
+ // stored catalog is stale, and only an installed model has a probeable
+ // capability report). Every other provider uses its own catalog, so a
+ // cloud CLI stage can pick any model that provider offers.
+ const stageModels = isNoToolStage && localBackend
? localModelIds.map(id => ({
id,
name: id,
capabilities: capabilitiesByBackend?.[localBackend]?.[id],
}))
: effortAwareModelOptions(stageProvider, stage.model);
- const selectionPolicy = isPublicReviewStage ? publicReviewSelectionPolicy : undefined;
+ const selectionPolicy = posture ? selectionPolicies[posture] : undefined;
const stageProviderId = stage.providerId || '';
const stageModel = stage.model || '';
const stageEffort = stage.effort || '';
@@ -95,6 +155,12 @@ export default function PipelineStageConfig({ taskType, config, providers, onUpd
{stage.readOnly && (
read-only
)}
+ {isNoToolStage && (
+ tool-free gate
+ )}
+ {isActionsStage && (
+ sandboxed actions
+ )}
{stage.name}
{i < stages.length - 1 && (
→ Stage {i + 2}
@@ -120,18 +186,33 @@ export default function PipelineStageConfig({ taskType, config, providers, onUpd
onModelChange={(model) => updateStage('model', model)}
effort={stageEffort}
onEffortChange={(effort) => updateStage('effort', effort)}
- emptyProviderOption={isPublicReviewStage ? 'Select enforced local review provider (required)' : 'Default (task-level)'}
- emptyModelOption={isPublicReviewStage ? 'Select installed no-tool model (required)' : 'Default (task-level)'}
+ emptyProviderOption={posture
+ ? 'First eligible provider on this install'
+ : 'Default (task-level)'}
+ emptyModelOption={posture ? 'Use provider default model' : 'Default (task-level)'}
alwaysShowModel
selectionPolicy={selectionPolicy}
disabled={updating}
/>
)}
- {isPublicReviewStage && (
+ {posture && eligibleProviders?.length === 0 && (
+
+ No enabled AI provider on this install can enforce the{' '}
+ {isActionsStage ? 'sandboxed-actions' : 'tool-free'} posture, so this stage will not run.
+ Enable a supported CLI provider in{' '}
+ Settings → Providers.
+
{localModelsLoading
? 'Loading installed local model capability reports…'
- : 'Stage 2 accepts only the maintained local Claude wrapper and an installed text model whose runtime reports no tool-calling capability. The reviewer is read-only; the deterministic coordinator owns comments, approvals, rebases, and merges.'}
+ : `Tool-free stage. Eligible on this install: ${eligibleProviders.map((p) => p.name || p.id).join(', ')}. A local model must additionally report no tool-calling capability; a cloud model is held tool-free by the provider's own enforced flags. Leave the provider unset to use the first eligible one. It returns only a binary allowlist; the final stage never receives rejected content.`}
+
+ {`Sandboxed stage. Eligible on this install: ${eligibleProviders.map((p) => p.name || p.id).join(', ')}. PortOS passes the selected provider, model, and thinking effort through that provider's maintained sandbox recipe, with no forge credential or configuration overlays; the deterministic coordinator owns comments, issue filing, CI triggers, and merges.`}
{needsSecurityModelPolicy
- ? 'Stage 1 screens complete public content with a managed classifier; only cleared content reaches the read-only Stage 2 reviewer. Stages are nested, not independently scheduled.'
+ ? 'Stage 1 screens complete public content with a managed classifier; only cleared content reaches the tool-free Eligibility Gate, and only eligible PRs reach the optional sandboxed final review. Stages are nested, not independently scheduled.'
: 'Each stage runs as a separate agent inside this pipeline; stages are not scheduled independently.'}
- {' Configure different providers per stage (e.g., Codex for review, Claude for implementation).'}
+ {' Configure a different provider, model, and thinking effort per stage.'}
);
diff --git a/client/src/components/cos/tabs/schedule/PipelineStageConfig.test.jsx b/client/src/components/cos/tabs/schedule/PipelineStageConfig.test.jsx
new file mode 100644
index 0000000000..0fda45e235
--- /dev/null
+++ b/client/src/components/cos/tabs/schedule/PipelineStageConfig.test.jsx
@@ -0,0 +1,186 @@
+import { describe, expect, it, vi } from 'vitest';
+import { fireEvent, render, screen, waitFor } from '@testing-library/react';
+import { MemoryRouter } from 'react-router';
+
+vi.mock('../../../../hooks/useLocalModels', () => ({
+ default: () => ({
+ ollama: ['safe-model', 'tool-model'],
+ lmstudio: [],
+ capabilitiesByBackend: {
+ ollama: {
+ 'safe-model': ['chat'],
+ 'tool-model': ['chat', 'tools'],
+ },
+ },
+ loading: false,
+ }),
+}));
+
+import PipelineStageConfig from './PipelineStageConfig';
+
+const STAGES = [
+ {
+ name: 'Security Scan',
+ role: 'security',
+ promptKey: 'pr-reviewer-security',
+ readOnly: true,
+ },
+ {
+ name: 'Eligibility Gate',
+ role: 'eligibility',
+ promptKey: 'pr-reviewer-eligibility',
+ readOnly: true,
+ providerId: 'claude-ollama',
+ model: 'safe-model',
+ },
+ {
+ name: 'Code Review & Actions',
+ role: 'actions',
+ promptKey: 'pr-reviewer-review',
+ readOnly: true,
+ providerId: 'codex-cli',
+ model: 'gpt-5.6',
+ },
+];
+
+const providers = [
+ {
+ id: 'claude-ollama',
+ name: 'Local Claude',
+ type: 'cli',
+ command: 'claude',
+ endpoint: 'http://127.0.0.1:11434',
+ publicReviewSupported: true,
+ models: ['safe-model'],
+ },
+ {
+ id: 'codex-cli',
+ name: 'Codex CLI',
+ type: 'cli',
+ command: 'codex',
+ publicReviewActionsSupported: true,
+ models: ['gpt-5.6'],
+ },
+ {
+ id: 'antigravity-cli',
+ name: 'Antigravity CLI',
+ type: 'cli',
+ command: 'agy',
+ publicReviewActionsSupported: true,
+ models: ['gemini-3.6-flash'],
+ },
+ {
+ id: 'other-cli',
+ name: 'Other CLI',
+ type: 'cli',
+ command: 'other-agent',
+ models: ['other-model'],
+ },
+];
+
+function renderStages(stages = STAGES, onUpdate = vi.fn().mockResolvedValue(undefined)) {
+ render(
+
+ {}}
+ />
+ ,
+ );
+ return onUpdate;
+}
+
+describe('PipelineStageConfig — pr-reviewer', () => {
+ it('uses shared capability policies for the gate and sandbox-capable action providers', () => {
+ renderStages();
+
+ const providerSelects = screen.getAllByLabelText('Provider');
+ expect([...providerSelects[0].options].map((option) => option.value)).toEqual(['', 'claude-ollama']);
+ expect([...providerSelects[1].options].map((option) => option.value)).toEqual(['', 'codex-cli', 'antigravity-cli']);
+
+ const modelSelects = screen.getAllByLabelText('Model');
+ expect([...modelSelects[0].options].map((option) => option.value)).toEqual(['', 'safe-model']);
+ expect([...modelSelects[1].options].map((option) => option.value)).toEqual(['', 'gpt-5.6']);
+ expect(screen.getByText(/maintained sandbox/i)).toBeInTheDocument();
+ });
+
+ it('removes the optional actions stage without changing the mandatory gate', async () => {
+ const onUpdate = renderStages();
+ fireEvent.click(screen.getByRole('switch', { name: 'Enable final code review and actions' }));
+
+ await waitFor(() => expect(onUpdate).toHaveBeenCalledWith('pr-reviewer', {
+ taskMetadata: {
+ pipeline: { stages: STAGES.slice(0, 2) },
+ },
+ }));
+ });
+
+ it('restores the complete restricted action-stage posture when enabled', async () => {
+ const onUpdate = renderStages(STAGES.slice(0, 2));
+ fireEvent.click(screen.getByRole('switch', { name: 'Enable final code review and actions' }));
+
+ await waitFor(() => expect(onUpdate).toHaveBeenCalledWith('pr-reviewer', {
+ taskMetadata: expect.objectContaining({
+ pipeline: expect.objectContaining({
+ stages: expect.arrayContaining([
+ expect.objectContaining({
+ role: 'actions',
+ promptKey: 'pr-reviewer-review',
+ executionProfile: 'public-review-actions',
+ discardWorktree: true,
+ noCodeOutput: true,
+ }),
+ ]),
+ }),
+ }),
+ }));
+ });
+});
+
+// The refactor's core promise: the picker's eligible set comes from the
+// server-published `publicReviewPostures` on each provider, so an install with
+// none of the vendors the old copy named still configures both stages.
+describe('PipelineStageConfig — posture-driven eligibility', () => {
+ const profiledStages = [
+ STAGES[0],
+ { ...STAGES[1], executionProfile: 'public-review-gate', providerId: '', model: '' },
+ { ...STAGES[2], executionProfile: 'public-review-actions', providerId: '', model: '' },
+ ];
+
+ const renderWith = (installProviders) => render(
+
+ {}}
+ />
+ ,
+ );
+
+ it('offers a grok-only install its own provider for BOTH stages', () => {
+ renderWith([
+ { id: 'grok-cli', name: 'Grok', type: 'cli', command: 'grok', models: ['grok-4'], publicReviewPostures: ['no-tool', 'sandboxed-actions'] },
+ { id: 'opencode', name: 'OpenCode', type: 'cli', command: 'opencode', models: ['x'], publicReviewPostures: [] },
+ ]);
+ const providerSelects = screen.getAllByLabelText('Provider');
+ expect([...providerSelects[0].options].map((o) => o.value)).toEqual(['', 'grok-cli']);
+ expect([...providerSelects[1].options].map((o) => o.value)).toEqual(['', 'grok-cli']);
+ // A non-local provider's own catalog is selectable — the installed-local
+ // model list only applies where PortOS can probe capabilities.
+ expect(screen.getAllByText(/Eligible on this install: Grok/).length).toBe(2);
+ });
+
+ it('warns instead of silently offering nothing when a stage has no eligible provider', () => {
+ renderWith([
+ { id: 'claude-ollama', name: 'Local Claude', type: 'cli', command: 'claude', endpoint: 'http://127.0.0.1:11434', models: ['safe-model'], publicReviewPostures: ['no-tool'] },
+ ]);
+ expect(screen.getByText(/No enabled AI provider on this install can enforce the sandboxed-actions posture/)).toBeInTheDocument();
+ });
+});
diff --git a/client/src/components/cos/tabs/schedule/scheduleConstants.js b/client/src/components/cos/tabs/schedule/scheduleConstants.js
index 42000d8c99..e7d8454c58 100644
--- a/client/src/components/cos/tabs/schedule/scheduleConstants.js
+++ b/client/src/components/cos/tabs/schedule/scheduleConstants.js
@@ -1,4 +1,8 @@
// Shared constants and pure helpers for the CoS Schedule tab subcomponents.
+import {
+ PUBLIC_REVIEW_ACTIONS_POSTURE,
+ PUBLIC_REVIEW_NO_TOOL_POSTURE,
+} from '../../../../utils/providers';
import { timeUntil } from '../../../../utils/formatters';
import { describeCron } from '../../../../utils/cronHelpers';
@@ -50,6 +54,73 @@ export const SAVING_TITLE = 'Saving provider/model settings — the run will use
// provider/model are resolved per stage, so a task-level pin would be ignored.
export const pipelineStages = (config) => config?.taskMetadata?.pipeline?.stages || [];
+// pr-reviewer stages are semantic trust-boundary roles, not just numbered
+// cards. Keep the prompt-key fallback for schedules saved before roles were
+// persisted, so an older local schedule still renders with the right policy.
+export const PR_REVIEWER_STAGE_ROLES = Object.freeze(['security', 'eligibility', 'actions']);
+
+export function prReviewerStageRole(stage) {
+ if (PR_REVIEWER_STAGE_ROLES.includes(stage?.role)) return stage.role;
+ return {
+ 'pr-reviewer-security': 'security',
+ 'pr-reviewer-eligibility': 'eligibility',
+ 'pr-reviewer-review': 'actions',
+ }[stage?.promptKey] || null;
+}
+
+// A stage declares an execution PROFILE; the profile maps to the enforceable
+// POSTURE a provider must have a maintained recipe for. Mirrors
+// `server/lib/agentExecutionProfiles.js` so the picker offers exactly the
+// providers the server would accept at spawn time — and so neither side names
+// a vendor. The Security Scan is a managed server-side classifier and has no
+// posture, so it has no provider picker at all.
+export const STAGE_EXECUTION_PROFILE_POSTURES = Object.freeze({
+ 'public-review': PUBLIC_REVIEW_NO_TOOL_POSTURE,
+ 'public-review-gate': PUBLIC_REVIEW_NO_TOOL_POSTURE,
+ 'public-review-actions': PUBLIC_REVIEW_ACTIONS_POSTURE,
+});
+
+// Role fallback for a stage persisted before profiles were stored: the server
+// reasserts the profile on the next dispatch, but the picker has to gate
+// correctly on what is on disk right now.
+const PR_REVIEWER_ROLE_POSTURES = Object.freeze({
+ eligibility: PUBLIC_REVIEW_NO_TOOL_POSTURE,
+ actions: PUBLIC_REVIEW_ACTIONS_POSTURE,
+});
+
+export function stagePublicReviewPosture(stage) {
+ return STAGE_EXECUTION_PROFILE_POSTURES[stage?.executionProfile]
+ || PR_REVIEWER_ROLE_POSTURES[prReviewerStageRole(stage)]
+ || null;
+}
+
+// The optional final stage is deliberately defined as a complete posture, not
+// just a display label. The server sanitizes and reasserts the same contract;
+// this copy lets the schedule UI add it without manufacturing a weaker stage.
+export const PR_REVIEWER_ACTIONS_STAGE_DEFAULTS = Object.freeze({
+ name: 'Code Review & Actions',
+ role: 'actions',
+ promptKey: 'pr-reviewer-review',
+ readOnly: true,
+ useWorktree: true,
+ openPR: false,
+ simplify: false,
+ reviewLoop: false,
+ discardWorktree: true,
+ noCodeOutput: true,
+ managed: true,
+ executionProfile: 'public-review-actions',
+});
+
+export function togglePrReviewerActions(stages, enabled) {
+ const current = Array.isArray(stages) ? stages : [];
+ const withoutActions = current.filter((stage) => prReviewerStageRole(stage) !== 'actions');
+ if (!enabled) return withoutActions;
+ return current.some((stage) => prReviewerStageRole(stage) === 'actions')
+ ? current
+ : [...withoutActions, { ...PR_REVIEWER_ACTIONS_STAGE_DEFAULTS }];
+}
+
export const triggerButtonClass = (disabled) =>
`flex items-center gap-1 px-3 py-1.5 text-sm rounded transition-colors ${disabled ? 'bg-port-border/30 text-gray-500 cursor-not-allowed' : 'bg-port-accent/20 hover:bg-port-accent/30 text-port-accent'}`;
diff --git a/client/src/components/cos/tabs/schedule/scheduleConstants.test.js b/client/src/components/cos/tabs/schedule/scheduleConstants.test.js
index 0824187248..90a84ac6c1 100644
--- a/client/src/components/cos/tabs/schedule/scheduleConstants.test.js
+++ b/client/src/components/cos/tabs/schedule/scheduleConstants.test.js
@@ -1,5 +1,37 @@
import { describe, it, expect } from 'vitest';
-import { getTaskStatusGroup, taskSortKey, TASK_FILTERS, STATUS_GROUPS, describeNextRun, coverageTone, setMetadataOverride, toggleMetadataField, fileIssuesEffective, managedAgentOptionsFor, toggleFileIssuesMetadata } from './scheduleConstants';
+import { getTaskStatusGroup, taskSortKey, TASK_FILTERS, STATUS_GROUPS, describeNextRun, coverageTone, setMetadataOverride, toggleMetadataField, fileIssuesEffective, managedAgentOptionsFor, toggleFileIssuesMetadata, prReviewerStageRole, togglePrReviewerActions } from './scheduleConstants';
+
+describe('pr-reviewer pipeline helpers', () => {
+ it('recognizes semantic roles and legacy prompt-key stages', () => {
+ expect(prReviewerStageRole({ role: 'eligibility' })).toBe('eligibility');
+ expect(prReviewerStageRole({ promptKey: 'pr-reviewer-review' })).toBe('actions');
+ expect(prReviewerStageRole({ promptKey: 'other' })).toBeNull();
+ });
+
+ it('removes only the optional actions stage and restores its full safe posture', () => {
+ const stages = [
+ { name: 'Security Scan', role: 'security' },
+ { name: 'Eligibility Gate', role: 'eligibility' },
+ { name: 'Code Review & Actions', role: 'actions', providerId: 'codex-cli' },
+ ];
+ expect(togglePrReviewerActions(stages, false)).toEqual(stages.slice(0, 2));
+ expect(togglePrReviewerActions(stages.slice(0, 2), true)).toEqual([
+ ...stages.slice(0, 2),
+ expect.objectContaining({
+ role: 'actions',
+ promptKey: 'pr-reviewer-review',
+ executionProfile: 'public-review-actions',
+ discardWorktree: true,
+ noCodeOutput: true,
+ }),
+ ]);
+ });
+
+ it('is idempotent when the optional stage is already enabled', () => {
+ const stages = [{ role: 'security' }, { role: 'eligibility' }, { role: 'actions' }];
+ expect(togglePrReviewerActions(stages, true)).toBe(stages);
+ });
+});
describe('setMetadataOverride', () => {
it('sets a key without disturbing the app\'s other overrides', () => {
diff --git a/client/src/utils/README.md b/client/src/utils/README.md
index 012677dcec..56b6d6c667 100644
--- a/client/src/utils/README.md
+++ b/client/src/utils/README.md
@@ -41,7 +41,7 @@ grep -i "what you want to do" client/src/utils/README.md
| `urlNormalize` | `isUrl` detection, `normalizeUrl` (optional git/`requireDot` modes), `isHttpUrl` / `isHttpsUrl` (safe-href checks), and `tiktokVideoId` / `tiktokEmbedSrc` (host-anchored TikTok video-id extraction + its Embed Player URL, so a reference embeds without loading TikTok's embed.js). |
| `platform` | `isMac` detection and `modKey` (⌘/Ctrl) for keyboard-shortcut display. |
| `navWorkingSet` | Recent/pinned nav persistence (`recordVisit`, `togglePin`, `isPinned`) plus `resolveRecentNavEntries` for mapping stored deep links back to their longest matching nav-manifest entry. Also `migrateLegacyNavPath(path, commands)` — maps a stored path onto the CURRENT path of the page that used to answer to it, driven by each command's own `previousPaths` (declared in `server/lib/navManifest.js` beside the page that moved, shipped whole in the palette manifest). A pin is a stored route path, so without it a moved page's pin stops matching the manifest and the sidebar row silently vanishes on update. |
-| `providers` | AI-provider type predicates and helpers (`isCliProvider`, `isApiProvider`, `isCodexProvider`, `isCodexSubscriptionProvider` (subscription readiness is keyed on the `codex` command, never an editable id), `isAntigravityProvider`, `isLaunchableTuiProvider` — a TUI provider carrying the server-resolved `tuiCommandLine`, i.e. one a human can start at a shell prompt; shared by the Providers card's "Launch in Shell" button and the Shell page's launch menu so the two can't disagree — `PROVIDER_GATEWAYS` / `gatewayForProvider` / `isGatewayBackedProvider` (an OpenCode wrapper front-ending a hosted OpenAI-compatible gateway — `orcarouter`, `openrouter` — which inherits its API key at spawn time from the sibling API provider whose id equals the gateway id; reads the generic `gatewayBacked` marker and, forever, the legacy per-gateway boolean. MIRROR of `server/lib/providerGateways.js`, which the browser cannot import; keep the three copies in lockstep), `filterSelectableModels`, `resolveCliEffort` (mirror — what a stored effort actually runs as, so the picker can name a clamped level), `configuredDefaultIn` — the sentinel a provider's catalog carries, so a picker can render an option matching a sentinel-valued tier instead of a blank select — `getProviderTimeout`, `resolveEffectiveProvider` — the provider a record actually runs on (its pin, else the active provider) plus whether it fell back, so a "Default" option can name what it resolves to — `resolveSeriesRunLlm` (mirror of server `seriesLlmOverride.js`: which provider/model a Pipeline **series** run resolves to — per-run override → `series.llm` → active provider) and `providerModelLabel` (the one "Provider / model" phrasing), configured-default sentinels, and the claude/codex/agy thinking-effort levels — `effortLevelsForProvider`, mirror of server `providerModels.js`). `TOOL_FREE_LOCAL_PROVIDER_IDS`, `TOOL_FREE_LOCAL_TEXT_CAPABILITIES`, `isToolFreeLocalProvider`, `isToolFreeLocalModel`, and `toolFreeLocalSelectionPolicy` share the fail-closed local/text/no-tools filter used by security-sensitive provider/model/effort pickers. Also `isRunnerAllowedCommand(command, allowedCommands)` — would the CoS Agent Runner (`/spawn`, `/spawn-tui`) accept this command? Mirrors only the *normalization* in `server/cos-runner/allowedCommands.js` (the list itself arrives as `runnerAllowedCommands` on `GET /api/providers`, because the allowlist is an exec boundary and stays hand-curated server-side); returns `null` for "list not fetched / field blank" so a failed fetch never renders a warning. Pinned by `server/cos-runner/allowedCommands.parity.test.js`. `isPrivateNetworkEndpoint(endpoint)` — loopback, RFC1918/tailnet address, or a `.local`/`.ts.net`/single-label host, i.e. somewhere an unauthenticated OpenAI-compatible server is a normal setup rather than a missing API key. `isLocalInstanceProvider(provider)` — the narrower question, mirroring server `localProviderRuntime.js#isLocalInstanceEndpoint`: does this provider talk to a daemon on THIS machine (loopback, or no endpoint at all)? Gate anything that explains a provider by inspecting the host PortOS runs on — install state, "start it from Settings → Local LLM" — since `localBackendForProvider` matches by NAME and would otherwise claim a peer's LM Studio. And `credentialSource(provider)` plus `providerCardState(provider, { runtime, status, keySetFor, envVarSet })` + `PROVIDER_CARD_STATE` — is a provider ready to run, benched, blocked on a missing prerequisite (CLI not installed / API key absent / empty credential environment variable), or simply switched off? Reads the SERVER's `missingPrerequisites` (published per provider on `GET /api/providers` from `server/lib/providerPrerequisites.js`, and the same computation `getFallbackProvider` routes on) and adds the local-app runtime shape plus tri-state checks for stored, inherited, and process environment credentials. Unknown lookup values mean "not probed", never "missing". Drives the AI Providers page card colors and grouping — distinct from `ProviderReadiness`/`GET /api/providers/readiness`, which probes the local daemon behind a provider. `resolvesOutsidePortosPath(provider)` — does this provider resolve its binary somewhere the runtime probe never looked (an explicit path in `command`, or its own `PATH` in `envVars`)? Mirror of the same two guards in the server's `providerRuntimeKey`, and what keeps the card's badge from accusing a working provider the router happily routes. And `providerRuntimeKey(provider)` — the key a provider's runtime is published under by `GET /api/providers/runtimes`, so a card can show its CLI install status (bare binary name for a cli/tui provider, provider id for an API provider fronted by a local app); the runtime table itself stays server-side. |
+| `providers` | AI-provider type predicates and helpers (`isCliProvider`, `isApiProvider`, `isCodexProvider`, `isCodexSubscriptionProvider` (subscription readiness is keyed on the `codex` command, never an editable id), `isAntigravityProvider`, `isLaunchableTuiProvider` — a TUI provider carrying the server-resolved `tuiCommandLine`, i.e. one a human can start at a shell prompt; shared by the Providers card's "Launch in Shell" button and the Shell page's launch menu so the two can't disagree — `PROVIDER_GATEWAYS` / `gatewayForProvider` / `isGatewayBackedProvider` (an OpenCode wrapper front-ending a hosted OpenAI-compatible gateway — `orcarouter`, `openrouter` — which inherits its API key at spawn time from the sibling API provider whose id equals the gateway id; reads the generic `gatewayBacked` marker and, forever, the legacy per-gateway boolean. MIRROR of `server/lib/providerGateways.js`, which the browser cannot import; keep the three copies in lockstep), `filterSelectableModels`, `resolveCliEffort` (mirror — what a stored effort actually runs as, so the picker can name a clamped level), `configuredDefaultIn` — the sentinel a provider's catalog carries, so a picker can render an option matching a sentinel-valued tier instead of a blank select — `getProviderTimeout`, `resolveEffectiveProvider` — the provider a record actually runs on (its pin, else the active provider) plus whether it fell back, so a "Default" option can name what it resolves to — `resolveSeriesRunLlm` (mirror of server `seriesLlmOverride.js`: which provider/model a Pipeline **series** run resolves to — per-run override → `series.llm` → active provider) and `providerModelLabel` (the one "Provider / model" phrasing), configured-default sentinels, and the claude/codex/agy thinking-effort levels — `effortLevelsForProvider`, mirror of server `providerModels.js`). `TOOL_FREE_LOCAL_PROVIDER_IDS`, `TOOL_FREE_LOCAL_TEXT_CAPABILITIES`, `isToolFreeLocalProvider`, `isToolFreeLocalModel`, and `toolFreeLocalSelectionPolicy` share the fail-closed local/text/no-tools filter used by security-sensitive provider/model/effort pickers. `PUBLIC_REVIEW_NO_TOOL_POSTURE` / `PUBLIC_REVIEW_ACTIONS_POSTURE`, `supportsPublicReviewPosture`, and `publicReviewSelectionPolicy` are the pr-reviewer counterpart (mirror of server `agentExecutionProfiles.js`): a pipeline stage names a POSTURE, the server publishes each provider's `publicReviewPostures` on `GET /api/providers`, and the picker offers exactly the providers this install can enforce — no vendor is named on either side. Also `isRunnerAllowedCommand(command, allowedCommands)` — would the CoS Agent Runner (`/spawn`, `/spawn-tui`) accept this command? Mirrors only the *normalization* in `server/cos-runner/allowedCommands.js` (the list itself arrives as `runnerAllowedCommands` on `GET /api/providers`, because the allowlist is an exec boundary and stays hand-curated server-side); returns `null` for "list not fetched / field blank" so a failed fetch never renders a warning. Pinned by `server/cos-runner/allowedCommands.parity.test.js`. `isPrivateNetworkEndpoint(endpoint)` — loopback, RFC1918/tailnet address, or a `.local`/`.ts.net`/single-label host, i.e. somewhere an unauthenticated OpenAI-compatible server is a normal setup rather than a missing API key. `isLocalInstanceProvider(provider)` — the narrower question, mirroring server `localProviderRuntime.js#isLocalInstanceEndpoint`: does this provider talk to a daemon on THIS machine (loopback, or no endpoint at all)? Gate anything that explains a provider by inspecting the host PortOS runs on — install state, "start it from Settings → Local LLM" — since `localBackendForProvider` matches by NAME and would otherwise claim a peer's LM Studio. And `credentialSource(provider)` plus `providerCardState(provider, { runtime, status, keySetFor, envVarSet })` + `PROVIDER_CARD_STATE` — is a provider ready to run, benched, blocked on a missing prerequisite (CLI not installed / API key absent / empty credential environment variable), or simply switched off? Reads the SERVER's `missingPrerequisites` (published per provider on `GET /api/providers` from `server/lib/providerPrerequisites.js`, and the same computation `getFallbackProvider` routes on) and adds the local-app runtime shape plus tri-state checks for stored, inherited, and process environment credentials. Unknown lookup values mean "not probed", never "missing". Drives the AI Providers page card colors and grouping — distinct from `ProviderReadiness`/`GET /api/providers/readiness`, which probes the local daemon behind a provider. `resolvesOutsidePortosPath(provider)` — does this provider resolve its binary somewhere the runtime probe never looked (an explicit path in `command`, or its own `PATH` in `envVars`)? Mirror of the same two guards in the server's `providerRuntimeKey`, and what keeps the card's badge from accusing a working provider the router happily routes. And `providerRuntimeKey(provider)` — the key a provider's runtime is published under by `GET /api/providers/runtimes`, so a card can show its CLI install status (bare binary name for a cli/tui provider, provider id for an API provider fronted by a local app); the runtime table itself stays server-side. |
| `systemCapabilities` | Server-annotated hardware compatibility helpers: preserve model/provider choices when compatibility is unknown, and hide only entries whose `hardwareCompatibility.state` is definitively `unavailable`. |
| `layeredIntelligenceReasons` | Canonical gloss for the Layered Intelligence loop's run-outcome reason tokens, shared by the on-demand toast and the durable "Last run" line (`formatLiReason`, `liReasonTone`, `LI_NEUTRAL_REASONS`). |
diff --git a/client/src/utils/providers.js b/client/src/utils/providers.js
index 3f0e446dd5..47217096e5 100644
--- a/client/src/utils/providers.js
+++ b/client/src/utils/providers.js
@@ -257,6 +257,49 @@ export const toolFreeLocalSelectionPolicy = (
),
});
+// The two enforceable public-review postures. MIRROR of
+// `server/lib/agentExecutionProfiles.js`; a pr-reviewer stage names a posture
+// and the server publishes each provider's `publicReviewPostures` on
+// `GET /api/providers`, so no vendor is ever named on either side.
+export const PUBLIC_REVIEW_NO_TOOL_POSTURE = 'no-tool';
+export const PUBLIC_REVIEW_ACTIONS_POSTURE = 'sandboxed-actions';
+
+/**
+ * Whether the SERVER says this provider can enforce `posture`. Falls back to
+ * the older per-posture booleans so a browser talking to a peer/older server
+ * still renders a correct picker instead of an empty one.
+ */
+export const supportsPublicReviewPosture = (provider, posture) => {
+ if (Array.isArray(provider?.publicReviewPostures)) return provider.publicReviewPostures.includes(posture);
+ return posture === PUBLIC_REVIEW_ACTIONS_POSTURE
+ ? provider?.publicReviewActionsSupported === true
+ : provider?.publicReviewSupported === true;
+};
+
+/**
+ * Selection policy for a pr-reviewer stage.
+ *
+ * Provider eligibility is entirely server-derived. Model eligibility adds the
+ * authoritative no-tool capability check only where PortOS can actually probe
+ * it — a LOCAL runtime behind the provider. A cloud model is not probeable, so
+ * the vendor's enforced argv (`--restricted --tools ''`, `--sandbox read-only`,
+ * `--permission-mode plan`) is what denies it tools, and every model the
+ * provider lists stays selectable.
+ */
+export const publicReviewSelectionPolicy = (posture, capabilitiesByBackend = {}) => ({
+ provider: (provider) => supportsPublicReviewPosture(provider, posture),
+ model: (model, provider) => {
+ if (!supportsPublicReviewPosture(provider, posture)) return false;
+ if (posture !== PUBLIC_REVIEW_NO_TOOL_POSTURE || !localBackendForProvider(provider)) return true;
+ return isToolFreeLocalModelForProvider(
+ model,
+ provider,
+ capabilitiesByBackend,
+ () => true,
+ );
+ },
+});
+
/**
* Retain an existing non-runnable pin so a saved job can still be edited and
* cleared, while limiting new agent-job selections to runnable providers.
diff --git a/client/src/utils/providers.test.js b/client/src/utils/providers.test.js
index 83be00ef35..e66b152bb7 100644
--- a/client/src/utils/providers.test.js
+++ b/client/src/utils/providers.test.js
@@ -19,6 +19,10 @@ import {
isToolFreeLocalProvider,
isToolFreeLocalModel,
toolFreeLocalSelectionPolicy,
+ publicReviewSelectionPolicy,
+ supportsPublicReviewPosture,
+ PUBLIC_REVIEW_ACTIONS_POSTURE,
+ PUBLIC_REVIEW_NO_TOOL_POSTURE,
localToolUseHint,
withToolUseOptionLabel,
localBackendForProvider,
@@ -1905,3 +1909,45 @@ describe('isPrivateNetworkEndpoint', () => {
});
// @vitest-environment node
+
+describe('publicReviewSelectionPolicy', () => {
+ const LOCAL_CLAUDE = {
+ id: 'claude-ollama',
+ type: 'cli',
+ command: 'claude',
+ endpoint: 'http://127.0.0.1:11434',
+ publicReviewPostures: ['no-tool'],
+ };
+ const GROK = { id: 'grok-cli', type: 'cli', command: 'grok', publicReviewPostures: ['no-tool', 'sandboxed-actions'] };
+ const CAPS = { ollama: { 'safe-model': ['chat'], 'tool-model': ['chat', 'tools'] } };
+
+ it('reads eligibility from the server-published postures, not a vendor list', () => {
+ expect(supportsPublicReviewPosture(GROK, PUBLIC_REVIEW_ACTIONS_POSTURE)).toBe(true);
+ expect(supportsPublicReviewPosture(LOCAL_CLAUDE, PUBLIC_REVIEW_ACTIONS_POSTURE)).toBe(false);
+ expect(supportsPublicReviewPosture({ id: 'x', type: 'cli', publicReviewPostures: [] }, PUBLIC_REVIEW_NO_TOOL_POSTURE)).toBe(false);
+ });
+
+ it('falls back to the legacy booleans so an older server still renders a picker', () => {
+ expect(supportsPublicReviewPosture({ id: 'legacy', publicReviewSupported: true }, PUBLIC_REVIEW_NO_TOOL_POSTURE)).toBe(true);
+ expect(supportsPublicReviewPosture({ id: 'legacy', publicReviewActionsSupported: true }, PUBLIC_REVIEW_ACTIONS_POSTURE)).toBe(true);
+ expect(supportsPublicReviewPosture({ id: 'legacy' }, PUBLIC_REVIEW_NO_TOOL_POSTURE)).toBe(false);
+ });
+
+ // The probe only exists for a local runtime; a cloud model is held tool-free
+ // by the provider's own enforced argv, so filtering it out would leave the
+ // picker empty on an install with no local backend.
+ it('applies the no-tool capability probe to a local model only', () => {
+ const policy = publicReviewSelectionPolicy(PUBLIC_REVIEW_NO_TOOL_POSTURE, CAPS);
+ expect(policy.model('safe-model', LOCAL_CLAUDE)).toBe(true);
+ expect(policy.model('tool-model', LOCAL_CLAUDE)).toBe(false);
+ expect(policy.model('unprobed-model', LOCAL_CLAUDE)).toBe(false);
+ expect(policy.model('grok-4', GROK)).toBe(true);
+ });
+
+ it('never accepts a model on a provider that cannot enforce the posture', () => {
+ const policy = publicReviewSelectionPolicy(PUBLIC_REVIEW_ACTIONS_POSTURE, CAPS);
+ expect(policy.provider(LOCAL_CLAUDE)).toBe(false);
+ expect(policy.model('safe-model', LOCAL_CLAUDE)).toBe(false);
+ expect(policy.model('grok-4', GROK)).toBe(true);
+ });
+});
diff --git a/server/lib/README.md b/server/lib/README.md
index f9381d1a0b..4440363efb 100644
--- a/server/lib/README.md
+++ b/server/lib/README.md
@@ -189,7 +189,7 @@ The barrel `server/lib/index.js` is a machine-checkable enumeration of every pub
| `openAiModelsProbe.js` | `probeOpenAiModels(baseUrl, { timeoutMs, apiKey })` → `{ reachable, models, error }` — the one `GET {base}/models` probe for the local OpenAI-compatible daemons, shared by `services/providerReadiness.js` and `services/llamaServerManager.js`. Distinguishes unreachable from reachable-but-unlistable (`models: null`) from up-with-nothing-loaded (`[]`), names the real transport failure via `describeFetchError` (undici reports every one as a bare `fetch failed`), and cancels an unread body on a non-OK response. `apiKey` attaches a Bearer header for a key-gated daemon (vLLM's compose stack), and a 401/403 answers `reachable: true` with `error: 'authentication required'` — a server that refused the request is definitively running, and calling it unreachable would send the user to start it again. Consolidated after the two copies drifted — one passed its timeout as a `timeout` key inside the fetch init object, where it is not an option, silently running a 500ms poll loop on the 15s default. |
| `openAiChatStream.js` | `iterateOpenAiChat(...)` → normalized async content/reasoning chunks and `streamOpenAiChat(...)` → the streamed text — one streaming `POST {base}/chat/completions` against any OpenAI-compatible endpoint; both own timeout/abort composition, retries, parsing, backpressure, and reader cleanup. `streamOllamaChat(...)` is the assessment-only native `/api/chat` transport that preserves Ollama's exact eval counts and nanosecond timings. Also exports `buildMessages`, `parseStreamFrame`, `parseOllamaStreamFrame`, `normalizeUsage` (snake/camel/Ollama token-count keys → `{completionTokens, promptTokens}`, `null` = not reported), and `resolvePartialOutput`. Registering `onStats` on the OpenAI path asks for token counts; a daemon that rejects `stream_options` is retried once without it and remembered per endpoint. Sibling of `openAiModelsProbe.js`. Shared by Ask (`services/askService.js`), `services/localLlmPlayground.js` (provider-backed runs with a `/runs` record), and its `runEndpointLlmTest` (a bare loopback daemon PortOS holds no provider record for, which is how `services/localModelAssessments.js` measures llama.cpp / MTPLX / vLLM). An abort mid-stream throws with `.partialOutput` carrying what already streamed. |
| `cliChildEnv.js` | The one place the AI-CLI child environment is composed, replacing the hand-rolled copy every spawn site carried — which made each env-level fix an N-file sweep (#3194). `buildCliChildEnv({ baseEnv, before, provider, model, cwd, extra, guard })` returns a COMPLETE env for `spawn`: filters the inherited base to runtime essentials/provider auth, then layers `baseEnv → before → Ollama-Claude defaults → provider.envVars → buildOpencodeEnvVars → extra`, pins `PWD` to `cwd`, strips `CLAUDECODE`, and (with `guard: true`) prepends the pm2 guard shim onto the final `PATH`. The Ollama-Claude layer raises Claude Code's default output ceiling to 65,536 tokens so a thinking-capable local model cannot finish its reasoning past the stock 32K ceiling and die before its final tool call; an explicit provider env value wins. `composeProviderEnv({ before, provider, model, extra })` returns just the ordered provider layers, for sites that build a DELTA someone else bases and spawns (the CoS runner payload, a shell-session overlay). The two slots are not interchangeable: `before` sits UNDER `provider.envVars` (forgeTokenEnv/claudeSettingsEnv, so a provider override still wins), `extra` sits OVER it (TERM/COLORTERM for a PTY). `cliChildEnv.test.js` asserts the composed order per call site and **discovers** any new site that hand-rolls the tuple instead of calling these — so the call-site list stays in the test, not in prose here. |
-| `agentExecutionProfiles.js` | Named agent execution postures shared by lifecycle dispatch, CLI/TUI spawners, and child-environment filtering. `PUBLIC_REVIEW_EXECUTION_PROFILE` identifies the restricted local, no-tools public-content review mode without importing the provider/runtime graph into schedule or environment reads. |
+| `agentExecutionProfiles.js` | Named agent execution postures shared by lifecycle dispatch, CLI/TUI spawners, and child-environment filtering. A stage declares a PROFILE (`PUBLIC_REVIEW_EXECUTION_PROFILE`, `PUBLIC_REVIEW_GATE_EXECUTION_PROFILE`, `PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE`); `publicReviewPostureForProfile` maps it to the enforceable POSTURE a provider must carry a maintained recipe for (`no-tool` or `sandboxed-actions`), which is how a pipeline stage stays vendor-agnostic. |
| `localEndpoint.js` | Dependency-light local-instance URL predicates (`isLocalInstanceHost`, `isLocalInstanceEndpoint`, `localEndpointPort`) shared by provider safety policy and local-runtime classification without importing backend configuration or daemon managers. |
| `cliProviderArgs.js` | Per-CLI argv conventions (`buildCliArgs`) for stdin prompt delivery — dependency-light extraction from runner.js so out-of-process callers (autofixer) can import it. |
| `cliProviderRun.js` | One-shot CLI provider invocation (`pickCliProvider` + `runCliProviderPrompt`) — lightweight path for the autofixer + calendar MCP sync to honor the configured provider/model. |
diff --git a/server/lib/agentExecutionProfiles.js b/server/lib/agentExecutionProfiles.js
index e150d06da3..060451bafc 100644
--- a/server/lib/agentExecutionProfiles.js
+++ b/server/lib/agentExecutionProfiles.js
@@ -5,3 +5,54 @@
*/
export const PUBLIC_REVIEW_EXECUTION_PROFILE = 'public-review';
+
+// The public-review pipeline has three deliberately different trust postures:
+// the security scan is a server-side classifier, the eligibility gate is a
+// tool-free reasoner, and the final review is a configured direct CLI inside a
+// provider-specific maintained sandbox. Keep the profile names here so every
+// spawn path agrees on which posture is enforced instead of comparing stage
+// names or task types.
+export const PUBLIC_REVIEW_GATE_EXECUTION_PROFILE = 'public-review-gate';
+export const PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE = 'public-review-actions';
+
+/**
+ * The two enforceable postures behind those profiles. A posture is what a
+ * VENDOR declares a maintained recipe for; a profile is what a STAGE declares
+ * it needs. Keeping them separate is what lets a stage be configured onto any
+ * enabled provider whose vendor row declares the posture, instead of the
+ * pipeline naming specific vendors.
+ *
+ * - `no-tool` — reasoning only. No filesystem writes, no command
+ * execution, no network/MCP tools, no forge credentials.
+ * - `sandboxed-actions` — may apply the already-screened patch and run local
+ * tests inside the vendor's own maintained sandbox.
+ * Still no forge credentials: the deterministic
+ * coordinator owns every GitHub mutation.
+ */
+export const PUBLIC_REVIEW_NO_TOOL_POSTURE = 'no-tool';
+export const PUBLIC_REVIEW_ACTIONS_POSTURE = 'sandboxed-actions';
+export const PUBLIC_REVIEW_POSTURES = Object.freeze([
+ PUBLIC_REVIEW_NO_TOOL_POSTURE,
+ PUBLIC_REVIEW_ACTIONS_POSTURE,
+]);
+
+const PROFILE_POSTURES = Object.freeze({
+ [PUBLIC_REVIEW_EXECUTION_PROFILE]: PUBLIC_REVIEW_NO_TOOL_POSTURE,
+ [PUBLIC_REVIEW_GATE_EXECUTION_PROFILE]: PUBLIC_REVIEW_NO_TOOL_POSTURE,
+ [PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE]: PUBLIC_REVIEW_ACTIONS_POSTURE,
+});
+
+export const PUBLIC_REVIEW_EXECUTION_PROFILES = Object.freeze(Object.keys(PROFILE_POSTURES));
+
+/** The posture a stage's execution profile requires, or null for an ordinary task. */
+export function publicReviewPostureForProfile(profile) {
+ return PROFILE_POSTURES[profile] || null;
+}
+
+export function isPublicReviewNoToolProfile(profile) {
+ return publicReviewPostureForProfile(profile) === PUBLIC_REVIEW_NO_TOOL_POSTURE;
+}
+
+export function isPublicReviewRestrictedProfile(profile) {
+ return publicReviewPostureForProfile(profile) !== null;
+}
diff --git a/server/lib/cliChildEnv.js b/server/lib/cliChildEnv.js
index 31880d64ea..80f9ad62b5 100644
--- a/server/lib/cliChildEnv.js
+++ b/server/lib/cliChildEnv.js
@@ -46,7 +46,7 @@ import { getOpencodeLocalProviderNamespace, isClaudeCommand } from './providerMo
import { isGatewayNamespace } from './providerGateways.js';
import { agentGuardEnv } from './agentGuard/index.js';
import { buildSafeCliBaseEnv } from './processEnv.js';
-import { PUBLIC_REVIEW_EXECUTION_PROFILE } from './agentExecutionProfiles.js';
+import { isPublicReviewNoToolProfile, isPublicReviewRestrictedProfile } from './agentExecutionProfiles.js';
// Claude Code defaults to 32K output tokens. Thinking-capable local models can
// legitimately spend more than that before returning their final tool call;
@@ -148,6 +148,27 @@ export function buildPublicReviewCliEnv(env = {}) {
)));
}
+// The actions stage is allowed to use its vendor's own workspace sandbox for
+// repository inspection and tests, but it must never inherit forge credentials, SSH
+// configuration, cloud-provider keys, or arbitrary provider env vars. This is
+// deliberately a second strict allowlist rather than a blocklist: a new secret
+// added to the server or provider environment must not silently become visible
+// to a contributor-controlled review. The deterministic output hook owns every
+// forge mutation after the model exits.
+const PUBLIC_REVIEW_ACTIONS_ENV_KEYS = new Set([
+ 'PATH', 'Path', 'HOME', 'USER', 'LOGNAME', 'SHELL', 'PWD', 'TMPDIR', 'TMP', 'TEMP',
+ 'LANG', 'LANGUAGE', 'TERM', 'COLORTERM', 'TZ', 'NODE', 'NODE_ENV', 'NODE_PATH',
+ 'NVM_DIR', 'NVM_BIN',
+ 'SystemRoot', 'SystemDrive', 'ComSpec', 'PATHEXT', 'USERPROFILE', 'APPDATA',
+ 'LOCALAPPDATA', 'ProgramData', 'ProgramFiles', 'HOMEDRIVE', 'HOMEPATH',
+]);
+
+export function buildPublicReviewActionsCliEnv(env = {}) {
+ return Object.fromEntries(Object.entries(env || {}).filter(([key, value]) => (
+ value != null && (PUBLIC_REVIEW_ACTIONS_ENV_KEYS.has(key) || key.startsWith('LC_'))
+ )));
+}
+
/**
* Compose the complete environment an AI-CLI child process is spawned with.
*
@@ -188,9 +209,11 @@ export function buildCliChildEnv({
cwd,
);
- const env = safetyProfile === PUBLIC_REVIEW_EXECUTION_PROFILE
+ const env = isPublicReviewNoToolProfile(safetyProfile)
? buildPublicReviewCliEnv(composed)
- : composed;
+ : isPublicReviewRestrictedProfile(safetyProfile)
+ ? buildPublicReviewActionsCliEnv(composed)
+ : composed;
// CLAUDECODE is set when PortOS itself runs inside Claude Code; passing it
// through would make a spawned Claude CLI think it's nested in that session.
diff --git a/server/lib/cliChildEnv.test.js b/server/lib/cliChildEnv.test.js
index afe8879132..f65120b69c 100644
--- a/server/lib/cliChildEnv.test.js
+++ b/server/lib/cliChildEnv.test.js
@@ -2,7 +2,7 @@ import { describe, it, expect } from 'vitest';
import { posixPath } from './testHelper.js';
import { buildCliChildEnv, buildPublicReviewCliEnv, composeProviderEnv } from './cliChildEnv.js';
-import { PUBLIC_REVIEW_EXECUTION_PROFILE } from './agentExecutionProfiles.js';
+import { PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE, PUBLIC_REVIEW_EXECUTION_PROFILE } from './agentExecutionProfiles.js';
import { cliProviderAuthDescriptor } from './processEnv.js';
import { AGENT_GUARD_BIN } from './agentGuard/index.js';
import { collectServerSources, readServerSource } from './testHelper.js';
@@ -175,6 +175,46 @@ describe('buildCliChildEnv — public-review profile', () => {
});
});
+describe('buildCliChildEnv — public-review-actions profile', () => {
+ it('keeps runtime essentials without inherited credentials or config-path overlays', () => {
+ const env = buildCliChildEnv({
+ baseEnv: {
+ PATH: '/usr/bin',
+ HOME: '/home/example',
+ CODEX_HOME: '/tmp/codex-home',
+ XDG_CONFIG_HOME: '/tmp/config',
+ SSL_CERT_FILE: '/tmp/cert.pem',
+ OPENAI_API_KEY: 'codex-secret',
+ GH_TOKEN: 'forge-secret',
+ GITHUB_TOKEN: 'forge-secret-2',
+ SSH_AUTH_SOCK: '/tmp/agent.sock',
+ ANTHROPIC_AUTH_TOKEN: 'wrong-provider-secret',
+ PRIVATE_APP_SETTING: 'must-not-forward',
+ },
+ before: { GH_TOKEN: 'before-forge', AWS_PROFILE: 'cloud-profile' },
+ provider: { envVars: { GH_TOKEN: 'provider-forge', OPENAI_API_KEY: 'provider-secret' } },
+ cwd: '/tmp/public-review-actions',
+ safetyProfile: PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE,
+ });
+
+ expect(env).toMatchObject({
+ PATH: '/usr/bin',
+ HOME: '/home/example',
+ PWD: '/tmp/public-review-actions',
+ });
+ expect(env).not.toHaveProperty('CODEX_HOME');
+ expect(env).not.toHaveProperty('XDG_CONFIG_HOME');
+ expect(env).not.toHaveProperty('SSL_CERT_FILE');
+ expect(env).not.toHaveProperty('OPENAI_API_KEY');
+ expect(env).not.toHaveProperty('GH_TOKEN');
+ expect(env).not.toHaveProperty('GITHUB_TOKEN');
+ expect(env).not.toHaveProperty('SSH_AUTH_SOCK');
+ expect(env).not.toHaveProperty('ANTHROPIC_AUTH_TOKEN');
+ expect(env).not.toHaveProperty('AWS_PROFILE');
+ expect(env).not.toHaveProperty('PRIVATE_APP_SETTING');
+ });
+});
+
describe('buildCliChildEnv — PWD pin and CLAUDECODE strip', () => {
it('pins PWD to the spawn cwd, overriding a stale inherited value (#3193)', () => {
const env = buildCliChildEnv({ baseEnv: { PWD: '/repos/PortOS' }, cwd: '/repos/my-app' });
diff --git a/server/lib/cosValidation.js b/server/lib/cosValidation.js
index dbe7ddaafa..0efc74d85a 100644
--- a/server/lib/cosValidation.js
+++ b/server/lib/cosValidation.js
@@ -16,6 +16,7 @@ import { ANTIGRAVITY_COMMAND } from './antigravity.js';
import { CURSOR_COMMAND } from './cursor.js';
import { isValidSlashdoCommand } from './slashdoInvocation.js';
import { PR_COMPLETION_VALUES } from './prDisposition.js';
+import { PUBLIC_REVIEW_EXECUTION_PROFILES } from './agentExecutionProfiles.js';
import { AGENT_RUN_EVENT_KINDS, RUN_EVENT_READ_LIMITS } from './agentRunEvents.js';
import { recurrenceRuleSchema } from './recurrenceValidation.js';
import { TASK_DATA_INPUT_DEFINITIONS, TASK_DATA_INPUT_IDS } from './taskDataInputCatalog.js';
@@ -1689,6 +1690,92 @@ export const codeReviewSettingsSchema = z.object({
// Agent behavior flags that can be overridden per-pipeline-stage
export const PIPELINE_BEHAVIOR_FLAGS = ['useWorktree', 'openPR', 'prCompletion', 'simplify', 'reviewLoop'];
+// These two flags are dispatch/completion posture rather than ordinary
+// user-facing task switches, but a pipeline stage must carry them forward to
+// the child task. Keeping the list beside the generic behavior flags prevents
+// each hand-off path from silently dropping the throwaway-worktree contract.
+export const PIPELINE_STAGE_BEHAVIOR_FLAGS = [
+ ...PIPELINE_BEHAVIOR_FLAGS,
+ 'discardWorktree',
+ 'noCodeOutput',
+];
+
+// Pipeline stage roles are semantic contracts, not display labels. The
+// pr-reviewer stages use these values to decide which content may cross the
+// boundary and which provider posture is safe; generic pipelines may omit the
+// role and continue to use their existing promptKey-only behavior.
+export const PIPELINE_STAGE_ROLES = ['security', 'eligibility', 'actions'];
+// Re-exported, not restated: a new profile must be legal to persist the moment
+// it is declared, or the sanitizer silently rejects the stage that uses it.
+export const PIPELINE_EXECUTION_PROFILES = PUBLIC_REVIEW_EXECUTION_PROFILES;
+
+const PIPELINE_STAGE_BOOLEAN_FIELDS = [
+ 'readOnly', 'managed', 'useWorktree', 'openPR', 'simplify', 'reviewLoop',
+ 'discardWorktree', 'noCodeOutput',
+];
+const PIPELINE_STAGE_STRING_LIMITS = {
+ name: 120,
+ promptKey: 120,
+ providerId: 200,
+ model: 200,
+ guardId: 120,
+};
+
+function safePipelinePrecondition(raw) {
+ if (!isPlainObject(raw)) return null;
+ const keys = Object.keys(raw);
+ if (keys.length !== 1 || !['fileExists', 'fileNotExists'].includes(keys[0])) return null;
+ const value = raw[keys[0]];
+ if (typeof value !== 'string' || !value.trim() || value.length > 240) return null;
+ const path = value.trim();
+ if (path.startsWith('/') || path.startsWith('\\') || path.includes('\0')) return null;
+ if (path.split(/[\\/]/).some((part) => part === '..')) return null;
+ return { [keys[0]]: path };
+}
+
+function sanitizePipelineStage(raw) {
+ if (!isPlainObject(raw)) return null;
+ const clean = Object.create(null);
+ for (const [field, maxLength] of Object.entries(PIPELINE_STAGE_STRING_LIMITS)) {
+ if (!Object.prototype.hasOwnProperty.call(raw, field)) continue;
+ if (raw[field] === null && ['providerId', 'model'].includes(field)) continue;
+ if (typeof raw[field] !== 'string') return null;
+ const value = raw[field].trim();
+ if (!value || value.length > maxLength) return null;
+ clean[field] = value;
+ }
+ if (Object.prototype.hasOwnProperty.call(raw, 'role')) {
+ if (!PIPELINE_STAGE_ROLES.includes(raw.role)) return null;
+ clean.role = raw.role;
+ }
+ if (Object.prototype.hasOwnProperty.call(raw, 'executionProfile')) {
+ if (!PIPELINE_EXECUTION_PROFILES.includes(raw.executionProfile)) return null;
+ clean.executionProfile = raw.executionProfile;
+ }
+ if (Object.prototype.hasOwnProperty.call(raw, 'effort')) {
+ if (raw.effort !== null && !EFFORT_LEVELS.includes(raw.effort)) return null;
+ if (raw.effort !== null) clean.effort = raw.effort;
+ }
+ for (const field of PIPELINE_STAGE_BOOLEAN_FIELDS) {
+ if (!Object.prototype.hasOwnProperty.call(raw, field)) continue;
+ if (typeof raw[field] !== 'boolean') return null;
+ clean[field] = raw[field];
+ }
+ if (Object.prototype.hasOwnProperty.call(raw, 'precondition')) {
+ const precondition = safePipelinePrecondition(raw.precondition);
+ if (!precondition) return null;
+ clean.precondition = precondition;
+ }
+ return { ...clean };
+}
+
+function sanitizePipeline(raw) {
+ if (!isPlainObject(raw) || !Array.isArray(raw.stages) || raw.stages.length > 10) return null;
+ const stages = raw.stages.map(sanitizePipelineStage);
+ if (stages.some((stage) => !stage)) return null;
+ return { stages };
+}
+
// Absolute cap on total agent spawns per task (across all retry types)
export const MAX_TOTAL_SPAWNS = 5;
@@ -2021,9 +2108,13 @@ export function sanitizeTaskMetadata(raw) {
clean.branchesPerAgent = raw.branchesPerAgent;
hasKeys = true;
}
- // Pass through pipeline config (validated shape: object with stages array)
- if (raw.pipeline && typeof raw.pipeline === 'object' && Array.isArray(raw.pipeline.stages)) {
- clean.pipeline = raw.pipeline;
+ // Pipeline configuration is the one nested task-metadata shape. Keep only
+ // known stage fields and fail the whole update when a known field is malformed
+ // so a bad custom pipeline cannot silently lose its safety posture.
+ if (Object.prototype.hasOwnProperty.call(raw, 'pipeline')) {
+ const pipeline = sanitizePipeline(raw.pipeline);
+ if (!pipeline) return null;
+ clean.pipeline = pipeline;
hasKeys = true;
}
return hasKeys ? { ...clean } : null;
diff --git a/server/lib/cosValidation.test.js b/server/lib/cosValidation.test.js
index 39f4451457..78be1ccf34 100644
--- a/server/lib/cosValidation.test.js
+++ b/server/lib/cosValidation.test.js
@@ -89,6 +89,63 @@ describe('branch-reconcile batch metadata', () => {
});
});
+describe('cosValidation pipeline stage metadata', () => {
+ const validStage = {
+ name: 'Eligibility Gate',
+ promptKey: 'pr-reviewer-eligibility',
+ role: 'eligibility',
+ executionProfile: 'public-review-gate',
+ providerId: 'local-claude-wrapper',
+ model: 'safe-local-model',
+ effort: 'high',
+ readOnly: true,
+ managed: true,
+ useWorktree: true,
+ openPR: false,
+ simplify: false,
+ reviewLoop: false,
+ discardWorktree: true,
+ noCodeOutput: true,
+ precondition: { fileExists: 'screened-input.json' },
+ };
+
+ it('keeps the validated stage contract and drops unknown fields', () => {
+ expect(sanitizeTaskMetadata({
+ pipeline: {
+ stages: [{ ...validStage, unknown: 'must not persist' }],
+ },
+ })).toEqual({ pipeline: { stages: [validStage] } });
+ });
+
+ it('rejects malformed role, profile, effort, posture, and precondition values', () => {
+ const invalidCases = [
+ ['role', 'review'],
+ ['executionProfile', 'unrestricted'],
+ ['effort', 'bogus'],
+ ['discardWorktree', 'yes'],
+ ['noCodeOutput', 1],
+ ['precondition', { fileExists: '../outside-worktree' }],
+ ['precondition', { fileExists: 'a', fileNotExists: 'b' }],
+ ];
+ for (const [field, value] of invalidCases) {
+ expect(sanitizeTaskMetadata({ pipeline: { stages: [{ ...validStage, [field]: value }] } }), field)
+ .toBeNull();
+ }
+ });
+
+ it('allows explicit clear values for provider and model pins', () => {
+ const expectedStage = { ...validStage };
+ delete expectedStage.providerId;
+ delete expectedStage.model;
+ delete expectedStage.effort;
+ expect(sanitizeTaskMetadata({
+ pipeline: { stages: [{ ...validStage, providerId: null, model: null, effort: null }] },
+ })).toEqual({
+ pipeline: { stages: [expectedStage] },
+ });
+ });
+});
+
describe('cosValidation autonomous-job effort field', () => {
it('accepts every EFFORT_LEVELS value on create and rejects unknown values', () => {
for (const effort of EFFORT_LEVELS) {
diff --git a/server/lib/modelAbuseGuard.js b/server/lib/modelAbuseGuard.js
index 2fd09e2aa7..6e54d82466 100644
--- a/server/lib/modelAbuseGuard.js
+++ b/server/lib/modelAbuseGuard.js
@@ -149,9 +149,16 @@ export const MODEL_ABUSE_GUARD_CHUNK_OVERLAP = 64;
*/
export function formatPublicReviewInputPrompt(snapshot) {
if (!snapshot || typeof snapshot !== 'object' || Array.isArray(snapshot)) return null;
+ // Keep attacker-controlled strings inside the data envelope even when they
+ // contain the literal closing delimiter. JSON parsing still reconstructs the
+ // original values, while the model cannot mistake a value for framing.
+ const serialized = JSON.stringify(snapshot)
+ .replaceAll('<', '\\u003c')
+ .replaceAll('>', '\\u003e')
+ .replaceAll('&', '\\u0026');
return [
'',
- JSON.stringify(snapshot),
+ serialized,
'',
].join('\n');
}
diff --git a/server/lib/modelAbuseGuard.test.js b/server/lib/modelAbuseGuard.test.js
index ffc7d46e04..9776bba2fa 100644
--- a/server/lib/modelAbuseGuard.test.js
+++ b/server/lib/modelAbuseGuard.test.js
@@ -128,4 +128,17 @@ describe('model-abuse guard contract', () => {
expect(formatPublicReviewInputPrompt(null)).toBeNull();
expect(formatPublicReviewInputPrompt([])).toBeNull();
});
+
+ it('escapes framing delimiters inside hostile cleared content', () => {
+ const prompt = formatPublicReviewInputPrompt({
+ title: '',
+ body: '&',
+ diff: '>',
+ });
+
+ expect(prompt.match(/<\/cleared-public-review-input>/g)).toHaveLength(1);
+ expect(prompt).toContain('"title":"\\u003c/cleared-public-review-input\\u003e"');
+ expect(prompt).toContain('"body":"\\u0026"');
+ expect(prompt).toContain('"diff":"\\u003e"');
+ });
});
diff --git a/server/lib/providerVendors.js b/server/lib/providerVendors.js
index 6f69bcb201..f5766a6aa9 100644
--- a/server/lib/providerVendors.js
+++ b/server/lib/providerVendors.js
@@ -85,7 +85,12 @@ import {
prepareAntigravityPrompt,
resolveAntigravityModelAndEffort,
} from './antigravity.js';
-import { isGrokCommand, ensureGrokTuiArgs, ensureGrokHeadlessArgs, prepareGrokPromptFile } from './grok.js';
+import {
+ isGrokCommand,
+ ensureGrokTuiArgs,
+ ensureGrokHeadlessArgs,
+ prepareGrokPromptFile,
+} from './grok.js';
import { isKimiCommand, ensureKimiTuiArgs, ensureKimiHeadlessArgs, prepareKimiPrompt } from './kimi.js';
import {
CURSOR_COMMAND,
@@ -93,10 +98,27 @@ import {
ensureCursorTuiArgs,
ensureCursorHeadlessArgs,
} from './cursor.js';
-import { isLocalInstanceEndpoint } from './localEndpoint.js';
-import { PUBLIC_REVIEW_EXECUTION_PROFILE } from './agentExecutionProfiles.js';
-
-export { PUBLIC_REVIEW_EXECUTION_PROFILE } from './agentExecutionProfiles.js';
+import {
+ isPublicReviewNoToolProfile,
+ publicReviewPostureForProfile,
+ PUBLIC_REVIEW_EXECUTION_PROFILE,
+ PUBLIC_REVIEW_GATE_EXECUTION_PROFILE,
+ PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE,
+ PUBLIC_REVIEW_NO_TOOL_POSTURE,
+ PUBLIC_REVIEW_ACTIONS_POSTURE,
+ PUBLIC_REVIEW_POSTURES,
+} from './agentExecutionProfiles.js';
+
+export {
+ isPublicReviewNoToolProfile,
+ publicReviewPostureForProfile,
+ PUBLIC_REVIEW_EXECUTION_PROFILE,
+ PUBLIC_REVIEW_GATE_EXECUTION_PROFILE,
+ PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE,
+ PUBLIC_REVIEW_NO_TOOL_POSTURE,
+ PUBLIC_REVIEW_ACTIONS_POSTURE,
+ PUBLIC_REVIEW_POSTURES,
+} from './agentExecutionProfiles.js';
/**
* For every vendor EXCEPT codex/claude, `buildCliSpawnConfig`'s argv is just
@@ -148,6 +170,96 @@ function codexSpawnArgs(provider, { effectiveModel, effort, maxConcurrentThreads
return { command: provider?.command || CODEX_COMMAND, args, stdinMode: 'prompt' };
}
+// Codex's own sandbox modes are the enforcement here, not the prompt. Both
+// public-review recipes build argv from scratch and never forward
+// `provider.args`: a saved `--dangerously-bypass-approvals-and-sandbox` in a
+// user's provider config would otherwise turn a screened review into an
+// unrestricted session.
+function codexPublicReviewSpawnArgs(provider, { effectiveModel, effort, maxConcurrentThreads }) {
+ return codexPublicReviewArgs(provider, { effectiveModel, effort, maxConcurrentThreads }, ['--sandbox', 'read-only']);
+}
+
+function codexPublicReviewActionsSpawnArgs(provider, { effectiveModel, effort, maxConcurrentThreads }) {
+ // `workspace-write` is intentionally the narrowest Codex sandbox that can
+ // apply a supplied patch and run local tests; `--approve-for-me` only
+ // suppresses interactive confirmations inside that sandbox. Never replace
+ // these with the unrestricted bypass used by the ordinary coding-agent path.
+ return codexPublicReviewArgs(provider, { effectiveModel, effort, maxConcurrentThreads }, [
+ '--sandbox', 'workspace-write',
+ '--approve-for-me',
+ ]);
+}
+
+function codexPublicReviewArgs(provider, { effectiveModel, effort, maxConcurrentThreads }, postureArgs) {
+ const args = [
+ 'exec',
+ ...postureArgs,
+ '--ephemeral',
+ '--ignore-user-config',
+ ...buildCodexStartupArgs(),
+ ...buildCodexAgentThreadArgs(maxConcurrentThreads),
+ ];
+ if (effectiveModel) {
+ args.push('--model', effectiveModel);
+ }
+ args.push(...buildEffortArgs(effort, provider, args, effectiveModel));
+ return { command: provider?.command || CODEX_COMMAND, args, stdinMode: 'prompt' };
+}
+
+// Antigravity's `--sandbox` is its maintained terminal-restriction posture and
+// `--mode` picks what the session may do inside it: `plan` cannot edit at all,
+// `accept-edits` may apply the screened patch and run tests. Provider args are
+// intentionally not copied: saved args could turn a safe profile back into an
+// unrestricted session. `--print` carries the prompt as its VALUE (see
+// antigravity.js) — `prepareAntigravityPrompt` relocates it to the end of the
+// argv at spawn time, which is why it is safe to append flags after it here.
+function antigravityPublicReviewSpawnArgs(provider, ctx) {
+ return antigravityPublicReviewArgs(provider, ctx, 'plan');
+}
+
+function antigravityPublicReviewActionsSpawnArgs(provider, ctx) {
+ return antigravityPublicReviewArgs(provider, ctx, 'accept-edits');
+}
+
+function antigravityPublicReviewArgs(provider, { effectiveModel, effort } = {}, mode) {
+ const args = [
+ '--sandbox',
+ '--mode', mode,
+ '--disable-slash-commands',
+ '--print',
+ ];
+ if (effectiveModel) args.push('--model', effectiveModel);
+ args.push(...buildEffortArgs(effort, provider, args, effectiveModel));
+ return { command: provider?.command || ANTIGRAVITY_COMMAND, args, stdinMode: 'prompt' };
+}
+
+// Grok exposes both halves of the contract as first-class flags:
+// `--permission-mode plan` is its read-only mode, `--tools ''` empties the
+// built-in tool allowlist, and `--sandbox ` applies its own
+// filesystem/network sandbox (`workspace` is grok's built-in profile). The
+// safety flags are seeded as the BASE args so `ensureGrokHeadlessArgs` sees a
+// permission posture already pinned and does not append its usual
+// `--permission-mode bypassPermissions`.
+function grokPublicReviewSpawnArgs(provider, ctx) {
+ return grokPublicReviewArgs(provider, ctx, ['--permission-mode', 'plan', '--tools', '']);
+}
+
+function grokPublicReviewActionsSpawnArgs(provider, ctx) {
+ return grokPublicReviewArgs(provider, ctx, ['--sandbox', 'workspace', '--permission-mode', 'acceptEdits']);
+}
+
+function grokPublicReviewArgs(provider, { effectiveModel, effort } = {}, postureArgs) {
+ const args = ensureGrokHeadlessArgs([
+ ...postureArgs,
+ '--no-subagents',
+ '--disable-web-search',
+ ], effectiveModel);
+ args.push(...buildEffortArgs(effort, provider, args, effectiveModel));
+ // `ensureGrokHeadlessArgs` appends the GROK_STDIN_PROMPT_PATH prompt-file
+ // sentinel that `prepareGrokPromptFile` rewrites on Windows; keep it present.
+ return { command: provider?.command || 'grok', args, stdinMode: 'prompt' };
+}
+
const CODEX = {
id: 'codex',
idFragment: 'codex',
@@ -157,6 +269,18 @@ const CODEX = {
tuiArgs: ensureCodexTuiArgs,
cliArgs: codexCliArgs,
spawnArgs: codexSpawnArgs,
+ publicReview: {
+ // The CLI id and the TUI id share one binary, so both reach the same
+ // enforced recipe when a stage selects them.
+ [PUBLIC_REVIEW_NO_TOOL_POSTURE]: {
+ spawnArgs: codexPublicReviewSpawnArgs,
+ matchProvider: (provider) => isCodexCommand(provider?.command) || provider?.id === CODEX_CLI_ID || provider?.id === 'codex-tui',
+ },
+ [PUBLIC_REVIEW_ACTIONS_POSTURE]: {
+ spawnArgs: codexPublicReviewActionsSpawnArgs,
+ matchProvider: (provider) => provider?.type === 'cli' && isCodexCommand(provider?.command),
+ },
+ },
};
// ─── antigravity ────────────────────────────────────────────────────────────
@@ -177,6 +301,16 @@ const ANTIGRAVITY = {
cliArgs: antigravityCliArgs,
preparePrompt: prepareAntigravityPrompt,
spawnArgs: defaultSpawnArgs(antigravityCliArgs, ANTIGRAVITY_COMMAND),
+ publicReview: {
+ [PUBLIC_REVIEW_NO_TOOL_POSTURE]: {
+ spawnArgs: antigravityPublicReviewSpawnArgs,
+ matchProvider: (provider) => provider?.type === 'cli' && isAntigravityCommand(provider?.command),
+ },
+ [PUBLIC_REVIEW_ACTIONS_POSTURE]: {
+ spawnArgs: antigravityPublicReviewActionsSpawnArgs,
+ matchProvider: (provider) => provider?.type === 'cli' && isAntigravityCommand(provider?.command),
+ },
+ },
};
// ─── opencode ───────────────────────────────────────────────────────────────
@@ -217,6 +351,16 @@ const GROK = {
tuiArgs: ensureGrokTuiArgs,
cliArgs: grokCliArgs,
spawnArgs: defaultSpawnArgs(grokCliArgs, 'grok'),
+ publicReview: {
+ [PUBLIC_REVIEW_NO_TOOL_POSTURE]: {
+ spawnArgs: grokPublicReviewSpawnArgs,
+ matchProvider: (provider) => provider?.type === 'cli' && isGrokCommand(provider?.command),
+ },
+ [PUBLIC_REVIEW_ACTIONS_POSTURE]: {
+ spawnArgs: grokPublicReviewActionsSpawnArgs,
+ matchProvider: (provider) => provider?.type === 'cli' && isGrokCommand(provider?.command),
+ },
+ },
};
// ─── kimi ───────────────────────────────────────────────────────────────────
@@ -334,12 +478,6 @@ const CLAUDE_PUBLIC_REVIEW_ARGS = [
'--bare',
];
-function isLocalClaudePublicReviewProvider(provider) {
- if (provider?.type !== 'cli' || provider?.ollamaBacked !== true || !isClaudeCommand(provider?.command)) return false;
- const endpoint = provider?.envVars?.ANTHROPIC_BASE_URL || provider?.endpoint || 'http://localhost:11434';
- return isLocalInstanceEndpoint(endpoint);
-}
-
function claudePublicReviewArgs(provider, {
effectiveModel,
effort,
@@ -354,9 +492,10 @@ function claudePublicReviewArgs(provider, {
];
if (systemPromptFile) args.push('--append-system-prompt-file', systemPromptFile);
if (effectiveModel) {
- // This profile is local-only. Do not consult the host's Bedrock settings or
- // ambient environment while constructing its model id: a server started in
- // Bedrock mode must not turn a local Ollama model into a cloud model name.
+ // Pass the stage's model id through VERBATIM. Do not consult the host's
+ // Bedrock settings or ambient environment while constructing it: this
+ // profile is reachable from an Ollama-backed Claude wrapper, and a server
+ // started in Bedrock mode must not turn a local model into a cloud one.
args.push('--model', effectiveModel);
}
const safeArgs = applyLeanClaudeArgs(provider, args, provider?.command || 'claude');
@@ -379,8 +518,17 @@ const CLAUDE = {
matchCommand: () => true,
cliArgs: claudeCliArgs,
spawnArgs: claudeSpawnArgs,
- publicReviewSpawnArgs: claudePublicReviewArgs,
- publicReviewProvider: isLocalClaudePublicReviewProvider,
+ publicReview: {
+ // Claude is the historical always-true fallback row, so its posture
+ // matcher must positively identify the binary — an unknown command must
+ // never inherit claude's flag set. There is deliberately no
+ // sandboxed-actions recipe: Claude Code has no OS-level sandbox flag, only
+ // permission modes, so it fails closed for the actions stage.
+ [PUBLIC_REVIEW_NO_TOOL_POSTURE]: {
+ spawnArgs: claudePublicReviewArgs,
+ matchProvider: (provider) => provider?.type === 'cli' && isClaudeCommand(provider?.command),
+ },
+ },
};
/**
@@ -406,23 +554,25 @@ function matchesProvider(vendor, provider) {
return vendor.matchCliProvider ? vendor.matchCliProvider(provider) : vendor.matchCommand(provider?.command);
}
-function matchesPublicReviewProvider(vendor, provider) {
- if (!matchesProvider(vendor, provider)) {
- // The normal Codex provider matcher intentionally distinguishes the CLI
- // id from the TUI id. The safety profile must recognize both because they
- // share the same binary and receive the same read-only posture.
- if (vendor.id === 'codex' && (provider?.id === 'codex-tui' || isCodexCommand(provider?.command))) return true;
- return false;
- }
- // Claude is the historical fallback row. It is safe only when the selected
- // provider actually launches Claude; an unknown command must never inherit
- // Claude's safe argv by accident.
- if (vendor.id === 'claude') {
- return typeof vendor.publicReviewProvider === 'function'
- ? vendor.publicReviewProvider(provider)
- : false;
+/**
+ * The vendor recipe enforcing `posture` for `provider`, or null when this
+ * install has no maintained recipe for that pairing.
+ *
+ * Eligibility is DECLARED by the vendor row, never named by the caller: a
+ * pipeline stage asks for a posture and every enabled provider whose vendor
+ * declares it is a legal choice. That is what lets an install with only grok
+ * (or only a local Claude wrapper) configure the same stages an install with
+ * codex configures. A row's matcher must positively identify the binary —
+ * claude's `matchCommand` is unconditionally true (it is the historical
+ * fallback row), so an unknown command must never inherit its argv.
+ */
+export function publicReviewRecipe(provider, posture) {
+ if (!PUBLIC_REVIEW_POSTURES.includes(posture)) return null;
+ for (const vendor of PROVIDER_VENDORS) {
+ const recipe = vendor.publicReview?.[posture];
+ if (recipe?.spawnArgs && recipe.matchProvider(provider)) return recipe;
}
- return true;
+ return null;
}
/**
@@ -444,11 +594,14 @@ export function inferTuiCommand(id) {
/** `applyCommandDefaults` (tuiHandshake.js): TUI posture-flag dispatch. */
export function applyCommandDefaults(command, args, { safetyProfile = null } = {}) {
+ if (publicReviewPostureForProfile(safetyProfile) === PUBLIC_REVIEW_ACTIONS_POSTURE) {
+ throw new Error('The public-review-actions profile requires a supported direct CLI sandbox');
+ }
const vendor = PROVIDER_VENDORS.find((v) => (
- (safetyProfile === PUBLIC_REVIEW_EXECUTION_PROFILE ? v.publicReviewTuiArgs : v.tuiArgs)
+ (isPublicReviewNoToolProfile(safetyProfile) ? v.publicReviewTuiArgs : v.tuiArgs)
&& v.matchCommand(command)
));
- if (safetyProfile === PUBLIC_REVIEW_EXECUTION_PROFILE) {
+ if (isPublicReviewNoToolProfile(safetyProfile)) {
if (!vendor || typeof vendor.publicReviewTuiArgs !== 'function') {
throw new Error(`Provider command '${command}' has no enforced public-review posture`);
}
@@ -484,35 +637,55 @@ export function buildVendorCliArgs(provider, baseArgs, { model, effort }) {
* before this registry existed (see file header).
*/
export function buildVendorSpawnConfig(provider, ctx) {
- const vendor = PROVIDER_VENDORS.find((v) => v.spawnArgs && (
- ctx?.safetyProfile === PUBLIC_REVIEW_EXECUTION_PROFILE
- ? matchesPublicReviewProvider(v, provider)
- : matchesProvider(v, provider)
- ));
- if (ctx?.safetyProfile === PUBLIC_REVIEW_EXECUTION_PROFILE) {
- if (!vendor?.publicReviewSpawnArgs) {
- throw new Error(`Provider '${provider?.id || provider?.command || 'unknown'}' has no enforced public-review posture`);
+ const posture = publicReviewPostureForProfile(ctx?.safetyProfile);
+ if (posture) {
+ const recipe = publicReviewRecipe(provider, posture);
+ if (!recipe) {
+ throw new Error(`Provider '${provider?.id || provider?.command || 'unknown'}' has no enforced ${posture} public-review posture`);
}
- return vendor.publicReviewSpawnArgs(provider, ctx);
+ return recipe.spawnArgs(provider, ctx);
}
+ const vendor = PROVIDER_VENDORS.find((v) => v.spawnArgs && matchesProvider(v, provider));
return vendor.spawnArgs(provider, ctx);
}
/**
- * Whether a provider has a maintained, enforced read-only public-content
- * execution recipe for the requested transport. Unknown vendors fail closed.
+ * Every public-review posture this provider can actually be configured for,
+ * in `PUBLIC_REVIEW_POSTURES` order. This is the value the schedule UI reads to
+ * offer a stage's eligible providers, so it must stay derived from the vendor
+ * rows rather than from a hardcoded list of vendor names.
+ *
+ * Interactive (TUI) sessions and API/custom providers have no maintained
+ * recipe: a generic read-only prompt is not enforcement, so they fail closed.
+ */
+export function publicReviewPosturesForProvider(provider, { tui = false } = {}) {
+ if (tui || provider?.type !== 'cli') return [];
+ return PUBLIC_REVIEW_POSTURES.filter((posture) => Boolean(publicReviewRecipe(provider, posture)));
+}
+
+/**
+ * Vendor ids that declare a maintained recipe for `posture`, for naming what a
+ * user could install when nothing on their machine qualifies. Derived from the
+ * rows so the suggestion cannot go stale when a vendor gains or loses a recipe.
*/
-export function supportsPublicReviewProvider(provider, { tui = false } = {}) {
- // Interactive sessions and API/custom providers do not have a maintained
- // no-tools recipe here. Public contributor content is intentionally routed
- // only through the non-interactive Claude recipe below; callers must fail
- // closed instead of treating a generic read-only prompt as enforcement.
+export function publicReviewCapableVendorIds(posture) {
+ return PROVIDER_VENDORS.filter((vendor) => vendor.publicReview?.[posture]?.spawnArgs).map((vendor) => vendor.id);
+}
+
+/** Whether `provider` has a maintained, enforced recipe for one posture. */
+export function supportsPublicReviewPosture(provider, posture, { tui = false } = {}) {
if (tui || provider?.type !== 'cli') return false;
- const vendor = PROVIDER_VENDORS.find((v) => (
- v.publicReviewSpawnArgs
- && matchesPublicReviewProvider(v, provider)
- ));
- return Boolean(vendor);
+ return Boolean(publicReviewRecipe(provider, posture));
+}
+
+/** Whether a provider can run a tool-free public-content stage. */
+export function supportsPublicReviewProvider(provider, options) {
+ return supportsPublicReviewPosture(provider, PUBLIC_REVIEW_NO_TOOL_POSTURE, options);
+}
+
+/** Whether a provider can run the sandboxed final public-review stage. */
+export function supportsPublicReviewActionsProvider(provider, options) {
+ return supportsPublicReviewPosture(provider, PUBLIC_REVIEW_ACTIONS_POSTURE, options);
}
/**
diff --git a/server/lib/providerVendors.publicReview.test.js b/server/lib/providerVendors.publicReview.test.js
index 7af60e605e..1fabe5b8e6 100644
--- a/server/lib/providerVendors.publicReview.test.js
+++ b/server/lib/providerVendors.publicReview.test.js
@@ -1,8 +1,16 @@
import { describe, expect, it } from 'vitest';
-import { PUBLIC_REVIEW_EXECUTION_PROFILE } from './agentExecutionProfiles.js';
+import {
+ PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE,
+ PUBLIC_REVIEW_EXECUTION_PROFILE,
+ PUBLIC_REVIEW_GATE_EXECUTION_PROFILE,
+ PUBLIC_REVIEW_ACTIONS_POSTURE,
+ PUBLIC_REVIEW_NO_TOOL_POSTURE,
+} from './agentExecutionProfiles.js';
import {
buildVendorSpawnConfig,
+ publicReviewPosturesForProvider,
supportsPublicReviewProvider,
+ supportsPublicReviewActionsProvider,
} from './providerVendors.js';
const localClaude = {
@@ -16,21 +24,117 @@ const localClaude = {
ANTHROPIC_AUTH_TOKEN: 'local-only',
},
};
+const codex = { id: 'codex-cli', type: 'cli', command: 'codex', models: ['gpt-5.6'] };
+const antigravity = { id: 'antigravity-cli', type: 'cli', command: 'agy', models: ['gemini-3.6-flash-high'] };
+const grok = { id: 'grok-cli', type: 'cli', command: 'grok' };
-describe('public-review provider profile', () => {
- it('supports only the maintained local non-interactive Claude wrapper', () => {
- expect(supportsPublicReviewProvider(localClaude)).toBe(true);
- expect(supportsPublicReviewProvider({ ...localClaude, type: 'tui' })).toBe(false);
- expect(supportsPublicReviewProvider({ ...localClaude, ollamaBacked: false })).toBe(false);
- expect(supportsPublicReviewProvider({
- ...localClaude,
- envVars: { ANTHROPIC_BASE_URL: 'https://api.anthropic.com' },
- })).toBe(false);
- expect(supportsPublicReviewProvider({
- ...localClaude,
- command: 'claude',
- type: 'api',
- })).toBe(false);
+describe('public-review provider postures', () => {
+ // The whole point of the posture table: eligibility is DECLARED per vendor,
+ // so an install that has only one of these can still configure every stage.
+ it('derives each provider’s eligible postures from its vendor row', () => {
+ expect(publicReviewPosturesForProvider(codex)).toEqual([PUBLIC_REVIEW_NO_TOOL_POSTURE, PUBLIC_REVIEW_ACTIONS_POSTURE]);
+ expect(publicReviewPosturesForProvider(antigravity)).toEqual([PUBLIC_REVIEW_NO_TOOL_POSTURE, PUBLIC_REVIEW_ACTIONS_POSTURE]);
+ expect(publicReviewPosturesForProvider(grok)).toEqual([PUBLIC_REVIEW_NO_TOOL_POSTURE, PUBLIC_REVIEW_ACTIONS_POSTURE]);
+ // Claude Code has permission modes but no OS-level sandbox flag, so it is
+ // deliberately tool-free-only and fails closed for the actions stage.
+ expect(publicReviewPosturesForProvider(localClaude)).toEqual([PUBLIC_REVIEW_NO_TOOL_POSTURE]);
+ });
+
+ it('fails closed for transports and vendors with no maintained recipe', () => {
+ // A TUI session and an HTTP api provider have no enforced argv at all.
+ expect(publicReviewPosturesForProvider({ ...codex, type: 'tui' }, { tui: true })).toEqual([]);
+ expect(publicReviewPosturesForProvider({ ...codex, type: 'api' })).toEqual([]);
+ // An unknown command must never inherit claude's always-true fallback row.
+ expect(publicReviewPosturesForProvider({ id: 'custom', type: 'cli', command: 'custom-agent' })).toEqual([]);
+ // opencode/kimi/cursor have no maintained no-tool or sandbox recipe yet.
+ expect(publicReviewPosturesForProvider({ id: 'opencode', type: 'cli', command: 'opencode' })).toEqual([]);
+ expect(supportsPublicReviewProvider({ id: 'kimi', type: 'cli', command: 'kimi' })).toBe(false);
+ expect(supportsPublicReviewActionsProvider(localClaude)).toBe(false);
+ });
+
+ it('builds the final reviewer with the bounded Codex sandbox and no provider args', () => {
+ const config = buildVendorSpawnConfig({
+ ...codex,
+ command: '/opt/example/bin/codex',
+ args: ['--dangerously-bypass-approvals-and-sandbox', '--mcp-config', 'unsafe.json'],
+ }, {
+ effectiveModel: 'gpt-5.6',
+ effort: 'high',
+ safetyProfile: PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE,
+ });
+
+ expect(config.args).toEqual(expect.arrayContaining([
+ 'exec', '--sandbox', 'workspace-write', '--approve-for-me', '--ephemeral', '--ignore-user-config',
+ '--model', 'gpt-5.6',
+ ]));
+ expect(config.args).not.toContain('--dangerously-bypass-approvals-and-sandbox');
+ expect(config.args).not.toContain('--mcp-config');
+ expect(config.args).not.toContain('unsafe.json');
+ });
+
+ it('builds the Codex gate stage read-only rather than workspace-write', () => {
+ const config = buildVendorSpawnConfig(codex, {
+ effectiveModel: 'gpt-5.6',
+ safetyProfile: PUBLIC_REVIEW_GATE_EXECUTION_PROFILE,
+ });
+ expect(config.args).toEqual(expect.arrayContaining(['exec', '--sandbox', 'read-only']));
+ expect(config.args).not.toContain('workspace-write');
+ expect(config.args).not.toContain('--approve-for-me');
+ });
+
+ it('builds the final reviewer with the bounded Antigravity sandbox and selected effort', () => {
+ const config = buildVendorSpawnConfig({
+ ...antigravity,
+ command: '/opt/example/bin/agy',
+ args: ['--dangerously-skip-permissions', '--model', 'unsafe-model'],
+ models: ['gemini-3.6-flash-low', 'gemini-3.6-flash-high'],
+ }, {
+ effectiveModel: 'gemini-3.6-flash',
+ effort: 'high',
+ safetyProfile: PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE,
+ });
+
+ expect(config.args).toEqual(expect.arrayContaining([
+ '--sandbox', '--mode', 'accept-edits', '--disable-slash-commands',
+ '--model', 'gemini-3.6-flash', '--effort', 'high',
+ ]));
+ expect(config.args).not.toContain('plan');
+ expect(config.args).not.toContain('--dangerously-skip-permissions');
+ expect(config.args).not.toContain('unsafe-model');
+ });
+
+ it('builds the Antigravity gate stage in plan mode, which cannot edit', () => {
+ const config = buildVendorSpawnConfig(antigravity, { safetyProfile: PUBLIC_REVIEW_GATE_EXECUTION_PROFILE });
+ expect(config.args).toEqual(expect.arrayContaining(['--sandbox', '--mode', 'plan', '--disable-slash-commands']));
+ expect(config.args).not.toContain('accept-edits');
+ });
+
+ it('builds grok’s two postures from its own permission-mode and sandbox flags', () => {
+ const gate = buildVendorSpawnConfig({ ...grok, args: ['--always-approve'] }, {
+ effectiveModel: 'grok-4',
+ safetyProfile: PUBLIC_REVIEW_GATE_EXECUTION_PROFILE,
+ });
+ expect(gate.args).toEqual(expect.arrayContaining([
+ '--permission-mode', 'plan', '--tools', '', '--no-subagents', '--disable-web-search',
+ '--model', 'grok-4',
+ ]));
+ // A saved auto-approval posture must not survive into the screened run.
+ expect(gate.args).not.toContain('--always-approve');
+ expect(gate.args).not.toContain('bypassPermissions');
+ expect(gate.args).not.toContain('--sandbox');
+
+ const actions = buildVendorSpawnConfig(grok, { safetyProfile: PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE });
+ expect(actions.args).toEqual(expect.arrayContaining([
+ '--sandbox', 'workspace', '--permission-mode', 'acceptEdits',
+ ]));
+ expect(actions.args).not.toContain('plan');
+ });
+
+ it('rejects the action profile for a provider with no maintained sandbox', () => {
+ expect(() => buildVendorSpawnConfig(localClaude, {
+ effectiveModel: 'safe-model',
+ safetyProfile: PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE,
+ })).toThrow(/no enforced sandboxed-actions public-review posture/);
});
it('builds a fresh no-tool argv and ignores dangerous saved provider args', () => {
@@ -57,24 +161,30 @@ describe('public-review provider profile', () => {
expect(config.args).not.toContain('--disallowedTools');
});
- it('fails closed instead of assigning the profile to cloud or unknown providers', () => {
+ it('fails closed instead of assigning a posture to an unknown command', () => {
expect(() => buildVendorSpawnConfig({
- id: 'claude-cloud',
+ id: 'custom-agent',
type: 'cli',
- command: 'claude',
- envVars: { ANTHROPIC_BASE_URL: 'https://api.anthropic.com' },
+ command: 'custom-agent',
}, {
- effectiveModel: 'cloud-model',
+ effectiveModel: 'model',
safetyProfile: PUBLIC_REVIEW_EXECUTION_PROFILE,
- })).toThrow(/no enforced public-review posture/);
+ })).toThrow(/no enforced no-tool public-review posture/);
+ });
- expect(() => buildVendorSpawnConfig({
- id: 'custom-agent',
+ it('holds a cloud Claude to the same enforced no-tool argv as the local wrapper', () => {
+ // Public PR content is public, so the posture — not the model's location —
+ // is the control. The argv is what denies tools either way.
+ const config = buildVendorSpawnConfig({
+ id: 'claude-code',
type: 'cli',
- command: 'custom-agent',
+ command: 'claude',
+ envVars: { ANTHROPIC_BASE_URL: 'https://api.anthropic.com' },
}, {
- effectiveModel: 'model',
+ effectiveModel: 'claude-sonnet-5',
safetyProfile: PUBLIC_REVIEW_EXECUTION_PROFILE,
- })).toThrow(/no enforced public-review posture/);
+ });
+ expect(config.args).toEqual(expect.arrayContaining(['--restricted', '--tools', '', '--permission-mode', 'plan']));
+ expect(config.args).toContain('claude-sonnet-5');
});
});
diff --git a/server/routes/providers.js b/server/routes/providers.js
index 12397706ec..408d454b35 100644
--- a/server/routes/providers.js
+++ b/server/routes/providers.js
@@ -37,7 +37,7 @@ import {
} from '../services/codexAppServer.js';
import { runLocalRuntimeSetup, SETUP_ACTIONS } from '../services/localRuntimeSetup.js';
import { localEndpointPort, localRuntimeForProvider } from '../lib/localProviderRuntime.js';
-import { supportsPublicReviewProvider } from '../lib/providerVendors.js';
+import { publicReviewPosturesForProvider, PUBLIC_REVIEW_NO_TOOL_POSTURE, PUBLIC_REVIEW_ACTIONS_POSTURE } from '../lib/providerVendors.js';
import { buildTuiShellLaunch } from '../lib/tuiShellLaunch.js';
import {
captureSystemCapabilities,
@@ -146,9 +146,15 @@ const presentProvider = (provider, capabilities = captureSystemCapabilities()) =
// Derived on read from the raw provider. This is an explicit capability of
// the maintained public-review recipe, not a client-side guess based on a
// provider name or a user-writable `args` list.
+ // `publicReviewPostures` is the value the schedule UI filters on, so a stage
+ // offers exactly the providers this install can actually enforce. The two
+ // booleans are derived from it and kept for existing consumers.
+ const publicReviewPostures = publicReviewPosturesForProvider(provider, { tui: provider?.type === 'tui' });
return sanitizeProvider({
...decorated,
- publicReviewSupported: supportsPublicReviewProvider(provider, { tui: provider?.type === 'tui' }),
+ publicReviewPostures,
+ publicReviewSupported: publicReviewPostures.includes(PUBLIC_REVIEW_NO_TOOL_POSTURE),
+ publicReviewActionsSupported: publicReviewPostures.includes(PUBLIC_REVIEW_ACTIONS_POSTURE),
});
};
diff --git a/server/services/agentCliSpawning.js b/server/services/agentCliSpawning.js
index 827fa2140d..a60a2c9833 100644
--- a/server/services/agentCliSpawning.js
+++ b/server/services/agentCliSpawning.js
@@ -44,7 +44,7 @@ import { doneSentinelPath } from '../lib/agentSentinel.js';
import { isHostShuttingDown, shouldAbandonForHostShutdown, HOST_SHUTDOWN_REASON } from '../lib/hostShutdown.js';
import { ensureOllamaAgentContext } from './ollamaAgentContext.js';
import { isOllamaBackedProvider } from './providers.js';
-import { PUBLIC_REVIEW_EXECUTION_PROFILE } from '../lib/agentExecutionProfiles.js';
+import { isPublicReviewRestrictedProfile } from '../lib/agentExecutionProfiles.js';
const AGENTS_DIR = PATHS.cosAgents;
@@ -383,7 +383,7 @@ export async function spawnDirectly({
// entirely when the provider supplies its own GH_TOKEN/GITHUB_TOKEN so its
// explicit credential wins (gh prefers GH_TOKEN, so injecting one would shadow a
// provider GITHUB_TOKEN).
- const [claudeSettingsEnv, forgeTokenEnv] = safetyProfile === PUBLIC_REVIEW_EXECUTION_PROFILE
+ const [claudeSettingsEnv, forgeTokenEnv] = isPublicReviewRestrictedProfile(safetyProfile)
? [{}, {}]
: await Promise.all([
isClaudeCliProvider(provider) ? getClaudeSettingsEnv() : Promise.resolve({}),
diff --git a/server/services/agentCompletionCleanup.js b/server/services/agentCompletionCleanup.js
index 7864ff7e9c..d78f08d1af 100644
--- a/server/services/agentCompletionCleanup.js
+++ b/server/services/agentCompletionCleanup.js
@@ -21,7 +21,7 @@ import { unlink, rm } from 'fs/promises';
import { emitLog } from './cosEvents.js';
import { updateAgent } from './cosAgentLifecycle.js';
import { updateTask, addTask, reviveBlockedTask, checkStagePrecondition } from './cos.js';
-import { PIPELINE_BEHAVIOR_FLAGS, normalizeReviewers } from '../lib/validation.js';
+import { PIPELINE_STAGE_BEHAVIOR_FLAGS, normalizeReviewers } from '../lib/validation.js';
import { PATHS, tryReadFile } from '../lib/fileUtils.js';
import * as jiraService from './jira.js';
import * as git from './git.js';
@@ -30,6 +30,7 @@ import { resolveReviewLoopOptions } from './codeReview.js';
import { cleanupAgentWorktree, spawnMergeRecoveryTask, releaseRetryHold } from './agentWorktreeCleanup.js';
import { resolvePrCompletion, resolvePrCreation } from '../lib/prDisposition.js';
import { resolveOwnsPrWorkflow } from '../lib/slashdoInvocation.js';
+import { isPublicReviewRestrictedProfile } from '../lib/agentExecutionProfiles.js';
const ROOT_DIR = PATHS.root;
@@ -83,6 +84,21 @@ export async function handlePipelineProgression(task, agentId, success) {
const nextStage = stages[nextStageIndex];
+ // A restricted execution profile must never be INHERITED across a stage
+ // boundary — the profile is what selects the provider posture and the
+ // stripped child environment, so carrying the previous stage's value would
+ // run the next stage under the wrong contract (or, if cleared, under none at
+ // all while still holding untrusted public content). A pipeline that has
+ // entered a restricted profile and whose next stage declares none fails
+ // closed rather than handing that content to an unrestricted agent.
+ if (isPublicReviewRestrictedProfile(task.metadata?.executionProfile) && !nextStage.executionProfile) {
+ await updateTask(task.id, {
+ metadata: { ...task.metadata, pipeline: { ...pipeline, status: 'failed', stageResults: updatedResults } }
+ }, task.taskType);
+ emitLog('warn', `⛔ Pipeline ${pipeline.id} stage ${nextStageIndex} declares no execution profile after a restricted stage`, { pipelineId: pipeline.id });
+ return;
+ }
+
// Check next stage's precondition before advancing
if (nextStage.precondition && task.metadata.repoPath) {
const check = checkStagePrecondition(nextStage, task.metadata.repoPath);
@@ -129,10 +145,14 @@ export async function handlePipelineProgression(task, agentId, success) {
nextTask.metadata.providerId = nextStage.providerId;
}
if (nextStage.effort) nextTask.metadata.effort = nextStage.effort;
+ // The profile, unlike the pins above, is SET-OR-CLEARED (see the guard at the
+ // top of the hand-off): each stage runs under exactly the contract it
+ // declares, never the previous stage's.
+ nextTask.metadata.executionProfile = nextStage.executionProfile || null;
// Apply per-stage overrides for agent behavior flags
const stageReadOnly = nextStage.readOnly ?? false;
const taskDefaults = pipeline.taskDefaults || {};
- for (const flag of PIPELINE_BEHAVIOR_FLAGS) {
+ for (const flag of PIPELINE_STAGE_BEHAVIOR_FLAGS) {
if (flag in nextStage) {
nextTask.metadata[flag] = nextStage[flag];
} else if (stageReadOnly) {
diff --git a/server/services/agentCompletionCleanup.test.js b/server/services/agentCompletionCleanup.test.js
index 873a0bf334..850213b899 100644
--- a/server/services/agentCompletionCleanup.test.js
+++ b/server/services/agentCompletionCleanup.test.js
@@ -137,6 +137,52 @@ describe('handlePipelineProgression', () => {
// two disagree, PortOS fires `gh pr create` on a branch that already has a PR
// ("a pull request already exists" preserves the worktree as a false-positive
// failure), or conversely never opens the PR the agent was told not to open.
+// The execution profile selects the provider POSTURE and the stripped child
+// environment. Inheriting the previous stage's value (or silently clearing it)
+// would run a stage holding untrusted public content under the wrong contract.
+describe('handlePipelineProgression — execution profile hand-off', () => {
+ const publicReviewPipeline = (stages) => runningPipeline({ currentStage: 0, stages });
+
+ it('sets the next stage\'s own profile rather than inheriting the previous one', async () => {
+ const task = {
+ id: 't',
+ taskType: 'user',
+ metadata: {
+ executionProfile: 'public-review-gate',
+ pipeline: publicReviewPipeline([
+ { name: 'Eligibility Gate', executionProfile: 'public-review-gate' },
+ { name: 'Code Review & Actions', executionProfile: 'public-review-actions' },
+ ]),
+ },
+ };
+ await handlePipelineProgression(task, 'agent-1', true);
+ expect(addTask.mock.calls[0][0].metadata.executionProfile).toBe('public-review-actions');
+ });
+
+ it('fails the pipeline closed rather than advancing a restricted run into an unprofiled stage', async () => {
+ const task = {
+ id: 't',
+ taskType: 'user',
+ metadata: {
+ executionProfile: 'public-review-gate',
+ pipeline: publicReviewPipeline([
+ { name: 'Eligibility Gate', executionProfile: 'public-review-gate' },
+ { name: 'Unprofiled' },
+ ]),
+ },
+ };
+ await handlePipelineProgression(task, 'agent-1', true);
+ expect(addTask).not.toHaveBeenCalled();
+ expect(updateTask.mock.calls[0][1].metadata.pipeline.status).toBe('failed');
+ });
+
+ it('leaves an ordinary pipeline unprofiled', async () => {
+ const task = { id: 't', taskType: 'user', metadata: { pipeline: runningPipeline() } };
+ await handlePipelineProgression(task, 'agent-1', true);
+ expect(addTask.mock.calls[0][0].metadata.executionProfile).toBeNull();
+ });
+});
+
describe('runAgentCompletionCleanup — agentOwnsPR mirrors the prompt gate', () => {
const prTask = { id: 't', taskType: 'user', metadata: { openPR: true } };
diff --git a/server/services/agentErrorAnalysis.js b/server/services/agentErrorAnalysis.js
index 7a41a6571b..5c7cbb648b 100644
--- a/server/services/agentErrorAnalysis.js
+++ b/server/services/agentErrorAnalysis.js
@@ -1479,6 +1479,9 @@ export function resolveTypeFailureSignal({ success, terminatedByUser = false, ho
if (hookResult?.ran) {
if (hookResult.threw) return { record: 'failure', category: 'hook-error' };
+ if (hookResult.outcome?.accepted === false) {
+ return { record: 'failure', category: hookResult.outcome.reason || 'output-hook-rejected' };
+ }
if (hookResult.outcome?.reason === 'unparseable-response') return { record: 'failure', category: 'unparseable-response' };
}
diff --git a/server/services/agentFinalization.js b/server/services/agentFinalization.js
index d015b882b8..4593fcc81a 100644
--- a/server/services/agentFinalization.js
+++ b/server/services/agentFinalization.js
@@ -224,6 +224,8 @@ export function resolveProgrammaticIoVerdict({ success, hookResult }) {
// an exit-0 run banked a free success for the task type (#4107). Skip the
// learning write entirely instead.
if (HOOK_ABORTED_BEFORE_EVALUATION.has(hookResult.outcome.reason)) return SKIP_LEARNING_VERDICT;
+ if (hookResult.outcome.accepted === false) return false;
+ if (hookResult.outcome.accepted === true) return true;
return resolveTypeFailureSignal({ success, hookResult }).record === 'success';
}
@@ -813,8 +815,8 @@ export async function finalizeAgent({
// side effect. Same reason `terminatedByUser` keeps its own verdict.
const driftDowngrade = drift.drifted && reportedSuccess && !terminatedByUser;
- const success = reportedSuccess && prVerdict.ok && !driftDowngrade;
- const errorAnalysis = driftDowngrade
+ let success = reportedSuccess && prVerdict.ok && !driftDowngrade;
+ let errorAnalysis = driftDowngrade
? primaryCheckoutDriftAnalysis(drift)
: prVerdict.ok ? reportedErrorAnalysis : prVerificationAnalysis(prVerdict);
if (!prVerdict.ok) {
@@ -835,7 +837,7 @@ export async function finalizeAgent({
}
const taskType = task?.taskType || 'user';
- const taskUpdate = terminatedByUser
+ let taskUpdate = terminatedByUser
? {
status: 'blocked',
metadata: {
@@ -873,6 +875,33 @@ export async function finalizeAgent({
// withOutputHookTimeout.
const hookResult = await dispatchTaskOutputHookOnce({ agentId, task, success, workspacePath });
+ // Output hooks may return a trusted metadata patch that advances a staged
+ // workflow. Apply it before task persistence and cleanup so the next stage
+ // sees the narrowed input set. An explicit `accepted: false` is a real
+ // programmatic-output failure even when the agent exited zero; this is the
+ // fail-closed path for incomplete or contradictory eligibility envelopes.
+ const hookOutcome = hookResult?.outcome;
+ const hookMetadata = hookOutcome?.taskMetadata && typeof hookOutcome.taskMetadata === 'object'
+ && !Array.isArray(hookOutcome.taskMetadata)
+ ? hookOutcome.taskMetadata
+ : null;
+ if (hookMetadata) {
+ task.metadata = { ...task.metadata, ...hookMetadata };
+ }
+ const hookRejected = !terminatedByUser && hookResult?.ran && hookOutcome?.accepted === false;
+ if (hookRejected && success) {
+ success = false;
+ errorAnalysis = {
+ category: hookOutcome.reason || 'output-hook-rejected',
+ message: hookOutcome.message || 'The scheduled task output was rejected by its validation hook',
+ actionable: false,
+ origin: 'task-output-hook',
+ };
+ taskUpdate = await resolveFailedTaskUpdate(task, errorAnalysis, agentId);
+ } else if (hookMetadata && success && !terminatedByUser) {
+ taskUpdate = { ...taskUpdate, metadata: task.metadata };
+ }
+
// Success-criteria validation (issue #2344): stamp an explicit pass/fail (or
// null-when-undeclared) verdict onto the completion result, distinct from the
// exit-code `success`, so task-learning telemetry can distinguish "ran clean
@@ -906,10 +935,20 @@ export async function finalizeAgent({
// reason the PR downgrade does, and outranks it: the run may well have opened
// its PR fine and still mutated the primary, and THAT is the thing a human has
// to act on.
- const finalError = driftDowngrade ? drift.message : prVerdict.ok ? error : prVerdict.message;
+ const finalError = driftDowngrade
+ ? drift.message
+ : !prVerdict.ok
+ ? prVerdict.message
+ : hookRejected
+ ? errorAnalysis?.message || error
+ : error;
const finalCompletionReason = driftDowngrade
? PRIMARY_CHECKOUT_MUTATED_REASON
- : prVerdict.ok ? completionReason : prVerdict.category;
+ : !prVerdict.ok
+ ? prVerdict.category
+ : hookRejected
+ ? errorAnalysis?.category || completionReason
+ : completionReason;
await completeAgent(agentId, {
success,
diff --git a/server/services/agentLifecycle.js b/server/services/agentLifecycle.js
index d26ac2a6cf..8c1feb14c6 100644
--- a/server/services/agentLifecycle.js
+++ b/server/services/agentLifecycle.js
@@ -62,10 +62,10 @@ import { cliProviderAuthDescriptor } from '../lib/processEnv.js';
import { PROVIDER_TYPES } from '../lib/aiToolkit/constants.js';
import { buildCliSpawnConfig, isClaudeCliProvider, isTuiProvider, getClaudeSettingsEnv, spawnDirectly } from './agentCliSpawning.js';
import { buildTuiSpawnConfig, spawnTuiAgent } from './agentTuiSpawning.js';
-import { supportsPublicReviewProvider } from '../lib/providerVendors.js';
-import { PUBLIC_REVIEW_EXECUTION_PROFILE } from '../lib/agentExecutionProfiles.js';
+import { supportsPublicReviewPosture, publicReviewPostureForProfile, PUBLIC_REVIEW_NO_TOOL_POSTURE } from '../lib/providerVendors.js';
+import { PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE } from '../lib/agentExecutionProfiles.js';
import { formatPublicReviewInputPrompt } from '../lib/modelAbuseGuard.js';
-import { materializePublicReviewInput, readPublicReviewInputSnapshot, validatePublicReviewModel } from './modelAbuseGuard.js';
+import { materializePublicReviewInput, materializePublicReviewPatches, readPublicReviewInputSnapshot, validatePublicReviewModel } from './modelAbuseGuard.js';
import { releaseAppReviewMarker } from './appActivity.js';
import { ensureInstanceId } from './instances.js';
import { isClaimableBy, buildClaim, buildRelease, getClaimOwner, getTargetInstance, isTargetedElsewhere } from './cosTaskClaim.js';
@@ -113,6 +113,31 @@ function publicReviewScanBlock(task) {
};
}
+function publicReviewEligibilityBlock(task) {
+ const eligibility = task?.metadata?.pipeline?.eligibility;
+ const eligibleNumbers = Array.isArray(eligibility?.eligibleNumbers)
+ ? eligibility.eligibleNumbers.filter((number) => Number.isInteger(number) && number > 0)
+ : [];
+ const expected = task?.metadata?.issueWatcher?.pullRequests;
+ const expectedNumbers = Array.isArray(expected)
+ ? expected.map((item) => item?.number).filter((number) => Number.isInteger(number) && number > 0)
+ : [];
+ const allowed = new Set(eligibleNumbers);
+ const coverageMatches = expectedNumbers.length === eligibleNumbers.length
+ && expectedNumbers.every((number) => allowed.has(number));
+ if (eligibility?.complete === true && eligibleNumbers.length > 0 && coverageMatches) return null;
+ if (eligibility?.complete === true && eligibleNumbers.length === 0) {
+ return {
+ reason: 'Public review withheld: the eligibility gate cleared no pull requests',
+ category: 'public-review-no-eligible-prs',
+ };
+ }
+ return {
+ reason: 'Public review withheld: a complete eligibility gate result is required before actions',
+ category: 'public-review-eligibility-incomplete',
+ };
+}
+
/**
@@ -393,7 +418,11 @@ async function runAgentSpawn(task) {
}
const { provider, selectedModel, modelSelection } = resolution;
const isTui = isTuiProvider(provider);
- const publicReview = task.metadata?.executionProfile === PUBLIC_REVIEW_EXECUTION_PROFILE;
+ const executionProfile = task.metadata?.executionProfile;
+ const publicReviewPosture = publicReviewPostureForProfile(executionProfile);
+ const publicReviewNoTools = publicReviewPosture === PUBLIC_REVIEW_NO_TOOL_POSTURE;
+ const publicReviewActions = executionProfile === PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE;
+ const publicReview = Boolean(publicReviewPosture);
if (publicReview) {
const scanBlock = publicReviewScanBlock(task);
if (scanBlock) {
@@ -414,14 +443,35 @@ async function runAgentSpawn(task) {
return null;
}
}
- if (publicReview && !supportsPublicReviewProvider(provider, { tui: isTui })) {
- const reason = `Provider '${provider?.id || provider?.command || 'unknown'}' has no enforced read-only public-content review mode`;
+ if (publicReviewActions) {
+ const eligibilityBlock = publicReviewEligibilityBlock(task);
+ if (eligibilityBlock) {
+ await updateTask(task.id, {
+ status: 'blocked',
+ metadata: {
+ ...task.metadata,
+ blockedReason: eligibilityBlock.reason,
+ blockedCategory: eligibilityBlock.category,
+ blockedAt: new Date().toISOString(),
+ },
+ }, task.taskType || 'user').catch(() => {});
+ await cleanupOnError(eligibilityBlock.reason);
+ emitLog('warn', `Public review withheld for task ${task.id}: ${eligibilityBlock.category}`, { taskId: task.id });
+ return null;
+ }
+ }
+ // One posture check for both stages. Eligibility is declared by the vendor
+ // row and re-asserted HERE, at spawn time, because a schedule or API
+ // payload can be edited without the browser: the picker is a convenience,
+ // never the enforcement.
+ if (!supportsPublicReviewPosture(provider, publicReviewPosture, { tui: isTui })) {
+ const reason = `Provider '${provider?.id || provider?.command || 'unknown'}' has no enforced ${publicReviewPosture} public-content review mode`;
await updateTask(task.id, {
status: 'blocked',
metadata: {
...task.metadata,
blockedReason: reason,
- blockedCategory: 'public-review-provider-unsupported',
+ blockedCategory: publicReviewActions ? 'public-review-actions-provider-unsupported' : 'public-review-provider-unsupported',
blockedAt: new Date().toISOString(),
},
}, task.taskType || 'user').catch(() => {});
@@ -429,8 +479,8 @@ async function runAgentSpawn(task) {
cosEvents.emit('agent:error', { taskId: task.id, error: reason });
return null;
}
- if (publicReview) {
- const modelPolicy = await validatePublicReviewModel({ provider, model: selectedModel });
+ if (publicReviewNoTools) {
+ const modelPolicy = await validatePublicReviewModel({ provider, model: selectedModel, posture: PUBLIC_REVIEW_NO_TOOL_POSTURE });
if (!modelPolicy.ok) {
const reason = `Public review model is unavailable or not tool-free (${modelPolicy.code})`;
await updateTask(task.id, {
@@ -447,9 +497,10 @@ async function runAgentSpawn(task) {
return null;
}
}
- // Public review is intentionally direct-only: the CoS runner is a shared
- // process and may inherit a forge credential or ambient tool configuration.
- // The direct child receives a reduced environment below.
+ // Every public-content stage is direct-only. The CoS runner is a shared
+ // process and may inherit ambient tool configuration; the final stage's
+ // provider-specific direct CLI recipe is what enforces its sandbox.
+ // GitHub mutations still belong to the deterministic output hook.
const dispatchUseRunner = publicReview ? false : useRunner;
let publicReviewPromptData = null;
@@ -471,11 +522,20 @@ async function runAgentSpawn(task) {
const { workspacePath, resolvedAppName, worktreeInfo, jiraTicket, jiraBranchName, explicitWorktree } = prep;
if (publicReview) {
+ const allowedPullRequestNumbers = publicReviewActions
+ ? task.metadata?.pipeline?.eligibility?.eligibleNumbers
+ : null;
const materialized = await materializePublicReviewInput({
scanKey: task.metadata?.pipeline?.reviewInputKey,
workspacePath,
+ allowedPullRequestNumbers,
+ });
+ const patchesMaterialized = !publicReviewActions || await materializePublicReviewPatches({
+ scanKey: task.metadata?.pipeline?.reviewInputKey,
+ workspacePath,
+ allowedPullRequestNumbers,
});
- if (!materialized) {
+ if (!materialized || !patchesMaterialized) {
const reason = 'The screened public-review input snapshot is unavailable or invalid';
await updateTask(task.id, {
status: 'blocked',
@@ -492,9 +552,12 @@ async function runAgentSpawn(task) {
}
publicReviewPromptData = await readPublicReviewInputSnapshot({
scanKey: task.metadata?.pipeline?.reviewInputKey,
+ allowedPullRequestNumbers,
});
if (!publicReviewPromptData) {
- const reason = 'The screened public-review input could not be loaded for the no-tools reviewer';
+ const reason = publicReviewNoTools
+ ? 'The screened public-review input could not be loaded for the no-tools reviewer'
+ : 'The screened public-review input could not be loaded for the final reviewer';
await updateTask(task.id, {
status: 'blocked',
metadata: {
@@ -830,9 +893,10 @@ async function runAgentSpawn(task) {
// provider whose inference lands on this machine: local runtimes retain
// their deliberately bounded GPU concurrency posture.
const maxConcurrentThreads = cloudSwarmThreadCapacity(runProvider, task.metadata?.swarmCount);
+ const safetyProfile = publicReview ? executionProfile : null;
const cliConfig = isTui
- ? buildTuiSpawnConfig(runProvider, selectedModel, { systemPromptFile, effort: taskEffort, maxConcurrentThreads, safetyProfile: publicReview ? PUBLIC_REVIEW_EXECUTION_PROFILE : null })
- : buildCliSpawnConfig(runProvider, selectedModel, cliSettingsEnv, { systemPromptFile, effort: taskEffort, maxConcurrentThreads, safetyProfile: publicReview ? PUBLIC_REVIEW_EXECUTION_PROFILE : null });
+ ? buildTuiSpawnConfig(runProvider, selectedModel, { systemPromptFile, effort: taskEffort, maxConcurrentThreads, safetyProfile })
+ : buildCliSpawnConfig(runProvider, selectedModel, cliSettingsEnv, { systemPromptFile, effort: taskEffort, maxConcurrentThreads, safetyProfile });
emitLog('success', `Spawning agent for task ${task.id}`, {
agentId,
@@ -891,7 +955,7 @@ async function runAgentSpawn(task) {
laneName,
cleanupWorktreeFn: cleanupAgentWorktree,
isTruthyMetaFn: isTruthyMeta,
- safetyProfile: publicReview ? PUBLIC_REVIEW_EXECUTION_PROFILE : null,
+ safetyProfile,
});
} catch (err) {
if (handedOff) {
diff --git a/server/services/agentLifecycle.test.js b/server/services/agentLifecycle.test.js
index 607a64703e..33c323c43e 100644
--- a/server/services/agentLifecycle.test.js
+++ b/server/services/agentLifecycle.test.js
@@ -307,10 +307,13 @@ describe('agentLifecycle — guard wiring', () => {
it('fails closed before spawning when public-review security screening is incomplete', () => {
expect(AGENT_LIFECYCLE_SRC).toContain('public-review-security-scan-incomplete');
expect(AGENT_LIFECYCLE_SRC).toContain('public-review-no-cleared-prs');
+ expect(AGENT_LIFECYCLE_SRC).toContain('public-review-eligibility-incomplete');
+ expect(AGENT_LIFECYCLE_SRC).toContain('public-review-actions-provider-unsupported');
expect(AGENT_LIFECYCLE_SRC).toMatch(/if \(scanBlock\) \{[\s\S]*?status: 'blocked'/);
expect(AGENT_LIFECYCLE_SRC).toMatch(/expected fail-closed safety outcome/);
const gateStart = AGENT_LIFECYCLE_SRC.indexOf('const scanBlock = publicReviewScanBlock(task)');
- const gateEnd = AGENT_LIFECYCLE_SRC.indexOf('if (publicReview && !supportsPublicReviewProvider', gateStart);
+ const gateEnd = AGENT_LIFECYCLE_SRC.indexOf('if (!supportsPublicReviewPosture(provider, publicReviewPosture', gateStart);
+ expect(gateEnd).toBeGreaterThan(gateStart);
expect(AGENT_LIFECYCLE_SRC.slice(gateStart, gateEnd)).not.toContain("cosEvents.emit('agent:error'");
});
});
diff --git a/server/services/agentProviderResolution.js b/server/services/agentProviderResolution.js
index 3d41ad5fdd..eb0e4966a8 100644
--- a/server/services/agentProviderResolution.js
+++ b/server/services/agentProviderResolution.js
@@ -17,6 +17,7 @@ import { emitLog } from './cosEvents.js';
import { getActiveProvider, getAllProviders, getProviderById } from './providers.js';
import { isProviderAvailable, getFallbackProvider, getProviderStatus } from './providerStatus.js';
import { selectModelForTask } from './agentModelSelection.js';
+import { publicReviewPostureForTask, resolvePublicReviewProvider } from './publicReviewProviderSelection.js';
/**
* Resolve the provider + model for a task.
@@ -28,6 +29,56 @@ import { selectModelForTask } from './agentModelSelection.js';
* >}
*/
export async function resolveAgentProviderAndModel(task) {
+ // A public-review stage is resolved against the POSTURE it declares, not the
+ // usual pin → active → fallback chain: the ordinary chain is allowed to swap
+ // onto any healthy provider, and swapping untrusted contributor content onto
+ // a provider with no enforced posture is exactly what must not happen. The
+ // eligible set comes from this install's own enabled providers, so a stage
+ // configured on a machine that only has grok resolves to grok.
+ const publicReviewPosture = publicReviewPostureForTask(task);
+ if (publicReviewPosture) return resolvePublicReviewAgentProvider(task, publicReviewPosture);
+ return resolveOrdinaryProviderAndModel(task);
+}
+
+/**
+ * Provider + model for a public-review stage. A stage's own provider/model/
+ * effort pins (`metadata.provider` / `metadata.model`, set by the pipeline
+ * hand-off from the stage config) are honored when the pin is still eligible;
+ * otherwise the install's own eligible set decides, and an install with none
+ * fails PERMANENTLY so the task surfaces instead of re-dispatching forever.
+ */
+async function resolvePublicReviewAgentProvider(task, posture) {
+ const resolved = await resolvePublicReviewProvider({
+ posture,
+ pinnedProviderId: task.metadata?.provider || null,
+ });
+ if (!resolved.ok) {
+ return { ok: false, permanent: true, error: resolved.error, providerId: task.metadata?.provider || undefined };
+ }
+ const { provider } = resolved;
+ if (task.metadata?.provider && !resolved.pinHonored) {
+ emitLog('warn', `Public-review stage provider ${task.metadata.provider} is not eligible for the ${posture} posture — using ${provider.id}`, {
+ taskId: task.id,
+ providerId: provider.id,
+ });
+ }
+ // A model pin only survives when it was chosen FOR this provider; otherwise
+ // fall back to the provider's own default rather than handing one vendor's
+ // model id to another (the failure mode documented in the ordinary path).
+ const modelSelection = await selectModelForTask(task, provider);
+ const pinnedModel = task.metadata?.model;
+ const selectedModel = pinnedModel && task.metadata?.provider === provider.id
+ ? pinnedModel
+ : (modelSelection.model || provider.defaultModel || null);
+ emitLog('info', `Public-review stage (${posture}) resolved to provider ${provider.id}${selectedModel ? ` model ${selectedModel}` : ''}`, {
+ taskId: task.id,
+ providerId: provider.id,
+ model: selectedModel,
+ });
+ return { ok: true, provider, selectedModel, modelSelection };
+}
+
+async function resolveOrdinaryProviderAndModel(task) {
// A task can pin a specific provider via metadata.provider (e.g. a CoS job's
// per-job AI override). Resolve it BEFORE the active-provider availability
// gate so a pinned-but-healthy provider isn't blocked when the *active*
diff --git a/server/services/agentProviderResolution.test.js b/server/services/agentProviderResolution.test.js
index 507b856b17..93de1f7d29 100644
--- a/server/services/agentProviderResolution.test.js
+++ b/server/services/agentProviderResolution.test.js
@@ -340,3 +340,61 @@ describe('resolveAgentProviderAndModel', () => {
expect(r.selectedModel).toBe('heavy-x');
});
});
+
+// ─── public-review stages ───────────────────────────────────────────────────
+//
+// A public-review stage must NOT go through the ordinary pin → active →
+// fallback chain: that chain can swap onto any healthy provider, and running
+// untrusted contributor content on a provider with no enforced posture is the
+// exact failure this branch exists to prevent. These pin that the eligible set
+// comes from the install's own enabled providers instead.
+describe('resolveAgentProviderAndModel — public-review stages', () => {
+ const CODEX = { id: 'codex-cli', type: 'cli', command: 'codex' };
+ const GROK = { id: 'grok-cli', type: 'cli', command: 'grok' };
+ const OPENCODE = { id: 'opencode', type: 'cli', command: 'opencode' };
+ const gateTask = (metadata = {}) => ({
+ id: 'task-pr',
+ metadata: { executionProfile: 'public-review-gate', ...metadata },
+ });
+
+ it('resolves onto the only eligible provider an install actually has', async () => {
+ getAllProviders.mockResolvedValue({ providers: [OPENCODE, GROK], activeProvider: { id: 'opencode' } });
+ const r = await resolveAgentProviderAndModel(gateTask());
+ expect(r).toMatchObject({ ok: true, provider: { id: 'grok-cli' } });
+ // Never consults the ordinary fallback chain.
+ expect(getFallbackProvider).not.toHaveBeenCalled();
+ });
+
+ it('ignores a stage pin that is not eligible for the posture', async () => {
+ getAllProviders.mockResolvedValue({ providers: [OPENCODE, CODEX], activeProvider: null });
+ const r = await resolveAgentProviderAndModel(gateTask({ provider: 'opencode' }));
+ expect(r).toMatchObject({ ok: true, provider: { id: 'codex-cli' } });
+ });
+
+ it('keeps a model pin only on the provider it was chosen for', async () => {
+ getAllProviders.mockResolvedValue({ providers: [CODEX, GROK], activeProvider: null });
+ await expect(resolveAgentProviderAndModel(gateTask({ provider: 'grok-cli', model: 'grok-4' })))
+ .resolves.toMatchObject({ provider: { id: 'grok-cli' }, selectedModel: 'grok-4' });
+ // Pinned for a DIFFERENT provider — falls back to that provider's own model.
+ await expect(resolveAgentProviderAndModel(gateTask({ provider: 'opencode', model: 'grok-4' })))
+ .resolves.toMatchObject({ provider: { id: 'codex-cli' }, selectedModel: 'm-default' });
+ });
+
+ it('blocks PERMANENTLY when no enabled provider can enforce the posture', async () => {
+ getAllProviders.mockResolvedValue({ providers: [OPENCODE], activeProvider: { id: 'opencode' } });
+ const r = await resolveAgentProviderAndModel(gateTask());
+ expect(r.ok).toBe(false);
+ expect(r.permanent).toBe(true);
+ expect(r.error).toMatch(/no-tool/);
+ });
+
+ it('requires the sandboxed posture for the actions stage, not merely a CLI', async () => {
+ const LOCAL_CLAUDE = { id: 'claude-ollama', type: 'cli', command: 'claude', ollamaBacked: true };
+ getAllProviders.mockResolvedValue({ providers: [LOCAL_CLAUDE], activeProvider: { id: 'claude-ollama' } });
+ // Claude has a no-tool recipe but no sandbox recipe.
+ await expect(resolveAgentProviderAndModel({ id: 't', metadata: { executionProfile: 'public-review-gate' } }))
+ .resolves.toMatchObject({ ok: true, provider: { id: 'claude-ollama' } });
+ await expect(resolveAgentProviderAndModel({ id: 't', metadata: { executionProfile: 'public-review-actions' } }))
+ .resolves.toMatchObject({ ok: false, permanent: true });
+ });
+});
diff --git a/server/services/agentTuiSpawning.js b/server/services/agentTuiSpawning.js
index 2ea67daa8c..90d6d955fa 100644
--- a/server/services/agentTuiSpawning.js
+++ b/server/services/agentTuiSpawning.js
@@ -66,7 +66,7 @@ import {
SUBMIT_KEY,
} from '../lib/tuiHandshake.js';
import { injectTuiModelAndEffort } from '../lib/providerVendors.js';
-import { PUBLIC_REVIEW_EXECUTION_PROFILE } from '../lib/agentExecutionProfiles.js';
+import { isPublicReviewNoToolProfile } from '../lib/agentExecutionProfiles.js';
import { agentGuardEnv } from '../lib/agentGuard/index.js';
import { composeProviderEnv } from '../lib/cliChildEnv.js';
import { cliProviderAuthDescriptor } from '../lib/processEnv.js';
@@ -201,7 +201,7 @@ export function buildTuiSpawnConfig(provider, model, {
const command = provider?.command || inferTuiCommand(provider?.id);
const baseArgs = applyCommandDefaults(
command,
- safetyProfile === PUBLIC_REVIEW_EXECUTION_PROFILE ? [] : [...(provider?.args || [])],
+ isPublicReviewNoToolProfile(safetyProfile) ? [] : [...(provider?.args || [])],
{ safetyProfile },
);
// Model+effort injection (including the antigravity-validates-the-pair special
diff --git a/server/services/agentWorkspacePrep.js b/server/services/agentWorkspacePrep.js
index 9ab7ecd430..31f7c0346d 100644
--- a/server/services/agentWorkspacePrep.js
+++ b/server/services/agentWorkspacePrep.js
@@ -123,6 +123,119 @@ async function adoptWorktreeHoldingBranch({ agentId, workspacePath, branchName,
return worktreeInfo ? { worktreeInfo, adoptedFrom: holder.agentId } : null;
}
+/**
+ * Provision a worktree that the task explicitly needs, including a safe
+ * adoption of a surviving branch holder. This is shared by read-only public
+ * review stages and ordinary delivery agents: read-only describes what the
+ * model may do inside the tree, not whether the tree itself may be shared with
+ * the live checkout.
+ */
+async function prepareRequestedWorktree({
+ agentId,
+ workspacePath,
+ task,
+ existingBranch,
+ allowSharedWorkspaceFallback,
+}) {
+ // Detecting the base branch and resolving the branch holder are independent
+ // reads (a git-branches lookup vs. an agent-liveness + worktree-list check) —
+ // kick both off before awaiting either so their I/O overlaps instead of
+ // serializing on the spawn hot path.
+ const detectedBasePromise = git.getRepoBranches(workspacePath).catch(() => ({ baseBranch: null }));
+ // Resolve the branch holder ONCE before creation. `resumeWorktreePath` is a
+ // cache of that answer, not a separate ownership rule: if it is gone or
+ // stale, discovery finds the actual holder. This gives resume retries the
+ // same safe adoption path review-loop follow-ups use, rather than cutting a
+ // fresh branch merely because a cached path could not be moved.
+ const resumeWorktreePath = existingBranch ? task.metadata?.resumeWorktreePath : null;
+ const takeoverPromise = existingBranch
+ ? adoptWorktreeHoldingBranch({
+ agentId,
+ workspacePath,
+ branchName: existingBranch,
+ preferredPath: resumeWorktreePath,
+ taskId: task.id,
+ })
+ : Promise.resolve(null);
+
+ const { baseBranch: detectedBase } = await detectedBasePromise;
+ if (existingBranch) {
+ emitLog('info', `🌳 Worktree requested for task ${task.id} on existing branch ${existingBranch}`, {
+ taskId: task.id, app: task.metadata?.app, branch: existingBranch
+ });
+ } else {
+ emitLog('info', `🌳 Worktree requested for task ${task.id} — creating isolated worktree from ${detectedBase || 'default branch'}`, {
+ taskId: task.id, app: task.metadata?.app, baseBranch: detectedBase
+ });
+ }
+
+ const takeover = await takeoverPromise;
+
+ // Both read only by the block/pause decision below: the failure REASON
+ // decides whether the task is unrunnable or merely early, and `attempt` is
+ // which branch-busy wait this would be (TASKS.md round-trips metadata as
+ // strings, hence the coercion; never reset on revive, so the cap is the
+ // task's whole patience budget rather than a per-attempt one).
+ let worktreeError = null;
+ const attempt = (Number(task.metadata?.worktreeBusyAttempts) || 0) + 1;
+ const worktreeInfo = takeover?.worktreeInfo || await createWorktree(agentId, workspacePath, task.id, {
+ baseBranch: detectedBase || undefined,
+ existingBranch: existingBranch || undefined,
+ planId: task.metadata?.planId || undefined
+ }).catch(err => {
+ worktreeError = err;
+ emitLog('warn', `🌳 Worktree creation failed for task ${task.id}: ${err.message}`, { taskId: task.id });
+ return null;
+ });
+
+ if (worktreeInfo) {
+ const nextWorkspacePath = worktreeInfo.worktreePath;
+ const origin = worktreeInfo.adopted
+ ? `adopted from ${takeover?.adoptedFrom || task.metadata?.resumedFromAgentId || 'the interrupted run'}`
+ : `base: ${worktreeInfo.baseBranch}`;
+ emitLog('success', `🌳 Agent ${agentId} will work in worktree: ${worktreeInfo.branchName} (${origin})`, {
+ agentId, worktreePath: nextWorkspacePath, branchName: worktreeInfo.branchName, baseBranch: worktreeInfo.baseBranch
+ });
+ return { outcome: 'ready', workspacePath: nextWorkspacePath, worktreeInfo };
+ }
+
+ if (allowSharedWorkspaceFallback) {
+ // Reached here only for a resume pointer on a task that never asked for
+ // isolation. Blocking would be STRICTER than the task's own contract — it
+ // would have run in the shared workspace before the pointer existed — so
+ // degrade to that instead. The resume is lost (the leftover work stays on
+ // disk for the next attempt), but the task still runs.
+ emitLog('warn', `🌳 Worktree creation failed for task ${task.id}; resuming is not possible, continuing in the shared workspace`, { taskId: task.id });
+ return { outcome: 'ready', workspacePath, worktreeInfo: null };
+ }
+
+ if (isBranchCheckedOutElsewhereError(worktreeError?.message) && attempt <= WORKTREE_BUSY_MAX_ATTEMPTS) {
+ // The branch is checked out in ANOTHER worktree. Routinely that other
+ // worktree is the previous agent's, still being torn down by the very
+ // cleanup that spawned this task, so waiting it out lands the pull
+ // request a permanent block would have stranded. `worktree-busy` is a
+ // TIMED PAUSE (lib/taskBlockCategories.js): the cooldown sweeper revives
+ // it, and the pause keeps `existingBranch` so the revived attempt still
+ // attaches to the PR branch instead of cutting a fresh one off main.
+ const reason = `Branch ${existingBranch || 'for this task'} is still checked out in another worktree; retrying after a short cooldown`;
+ emitLog('info', `🌳 ${reason} (attempt ${attempt}/${WORKTREE_BUSY_MAX_ATTEMPTS})`, { taskId: task.id, branch: existingBranch || null });
+ await blockTask(task, `${reason}. ${worktreeError?.message || ''}`.trim(), 'worktree-busy', {
+ cooldownUntil: new Date(Date.now() + WORKTREE_BUSY_COOLDOWN_MS).toISOString(),
+ worktreeBusyAttempts: attempt,
+ });
+ return { outcome: 'blocked', reason };
+ }
+
+ // Isolation was explicitly requested (or is required to reach an existing
+ // branch), so falling back to the shared workspace would run the agent
+ // against the live checkout. Fail closed: block the task rather than touch
+ // the working tree behind the user's back.
+ const reason = `Worktree creation failed for task ${task.id}; refusing to run in the shared workspace because isolation was required`;
+ emitLog('warn', `🌳 ${reason}`, { taskId: task.id });
+ await blockTask(task, `Worktree creation failed — isolation was required${worktreeError?.message ? `: ${worktreeError.message}` : ''}`, 'worktree-failed');
+ return { outcome: 'blocked', reason };
+}
+
/**
* Prepare the workspace (and any worktree/JIRA branch) for an agent task.
*
@@ -351,103 +464,24 @@ export async function prepareAgentWorkspace({ agentId, task }) {
}
}
- if (wantsWorktree && !jiraBranchName) {
- // Detecting the base branch and resolving the branch holder are independent
- // reads (a git-branches lookup vs. an agent-liveness + worktree-list check) —
- // kick both off before awaiting either so their I/O overlaps instead of
- // serializing on the spawn hot path.
- const detectedBasePromise = git.getRepoBranches(workspacePath).catch(() => ({ baseBranch: null }));
- // Resolve the branch holder ONCE before creation. `resumeWorktreePath` is a
- // cache of that answer, not a separate ownership rule: if it is gone or
- // stale, discovery finds the actual holder. This gives resume retries the
- // same safe adoption path review-loop follow-ups use, rather than cutting a
- // fresh branch merely because a cached path could not be moved.
- const resumeWorktreePath = existingBranch ? task.metadata?.resumeWorktreePath : null;
- const takeoverPromise = existingBranch
- ? adoptWorktreeHoldingBranch({
- agentId,
- workspacePath,
- branchName: existingBranch,
- preferredPath: resumeWorktreePath,
- taskId: task.id,
- })
- : Promise.resolve(null);
-
- const { baseBranch: detectedBase } = await detectedBasePromise;
- if (existingBranch) {
- emitLog('info', `🌳 Worktree requested for task ${task.id} on existing branch ${existingBranch}`, {
- taskId: task.id, app: task.metadata?.app, branch: existingBranch
- });
- } else {
- emitLog('info', `🌳 Worktree requested for task ${task.id} — creating isolated worktree from ${detectedBase || 'default branch'}`, {
- taskId: task.id, app: task.metadata?.app, baseBranch: detectedBase
- });
- }
-
- const takeover = await takeoverPromise;
-
- // Both read only by the block/pause decision below: the failure REASON
- // decides whether the task is unrunnable or merely early, and `attempt` is
- // which branch-busy wait this would be (TASKS.md round-trips metadata as
- // strings, hence the coercion; never reset on revive, so the cap is the
- // task's whole patience budget rather than a per-attempt one).
- let worktreeError = null;
- const attempt = (Number(task.metadata?.worktreeBusyAttempts) || 0) + 1;
- worktreeInfo = takeover?.worktreeInfo || await createWorktree(agentId, workspacePath, task.id, {
- baseBranch: detectedBase || undefined,
- existingBranch: existingBranch || undefined,
- planId: task.metadata?.planId || undefined
- }).catch(err => {
- worktreeError = err;
- emitLog('warn', `🌳 Worktree creation failed, using shared workspace: ${err.message}`, { taskId: task.id });
- return null;
- });
+ } // end !isReadOnly
- if (worktreeInfo) {
- workspacePath = worktreeInfo.worktreePath;
- const origin = worktreeInfo.adopted
- ? `adopted from ${takeover?.adoptedFrom || task.metadata?.resumedFromAgentId || 'the interrupted run'}`
- : `base: ${worktreeInfo.baseBranch}`;
- emitLog('success', `🌳 Agent ${agentId} will work in worktree: ${worktreeInfo.branchName} (${origin})`, {
- agentId, worktreePath: worktreeInfo.worktreePath, branchName: worktreeInfo.branchName, baseBranch: worktreeInfo.baseBranch
- });
- } else if (!explicitWorktree) {
- // Reached here only for the resume pointer, on a task that never asked for
- // isolation. Blocking would be STRICTER than the task's own contract — it
- // would have run in the shared workspace before the pointer existed — so
- // degrade to that instead. The resume is lost (the leftover work stays on
- // disk for the next attempt), but the task still runs.
- emitLog('warn', `🌳 Worktree creation failed for task ${task.id}; resuming is not possible, continuing in the shared workspace`, { taskId: task.id });
- } else if (isBranchCheckedOutElsewhereError(worktreeError?.message) && attempt <= WORKTREE_BUSY_MAX_ATTEMPTS) {
- // The branch is checked out in ANOTHER worktree. Routinely that other
- // worktree is the previous agent's, still being torn down by the very
- // cleanup that spawned this task, so waiting it out lands the pull
- // request a permanent block would have stranded. `worktree-busy` is a
- // TIMED PAUSE (lib/taskBlockCategories.js): the cooldown sweeper revives
- // it, and the pause keeps `existingBranch` so the revived attempt still
- // attaches to the PR branch instead of cutting a fresh one off main.
- const reason = `Branch ${existingBranch || 'for this task'} is still checked out in another worktree; retrying after a short cooldown`;
- emitLog('info', `🌳 ${reason} (attempt ${attempt}/${WORKTREE_BUSY_MAX_ATTEMPTS})`, { taskId: task.id, branch: existingBranch || null });
- await blockTask(task, `${reason}. ${worktreeError?.message || ''}`.trim(), 'worktree-busy', {
- cooldownUntil: new Date(Date.now() + WORKTREE_BUSY_COOLDOWN_MS).toISOString(),
- worktreeBusyAttempts: attempt,
- });
- return { outcome: 'blocked', reason };
- } else {
- // Isolation was EXPLICITLY requested (useWorktree/openPR) but the
- // worktree couldn't be created. Falling back to the shared workspace
- // would run the agent against the live checkout and — with openPR —
- // auto-commit to the current branch, exactly the isolation the
- // caller opted into. Fail closed: block the task rather than touch
- // the working tree behind the user's back. (The auto-detected
- // conflict branch below keeps its lenient shared-workspace fallback,
- // since there the worktree was only a recommendation, not a request.)
- const reason = `Worktree creation failed for task ${task.id}; refusing to run in the shared workspace because isolation was explicitly requested`;
- emitLog('warn', `🌳 ${reason}`, { taskId: task.id });
- await blockTask(task, `Worktree creation failed — isolation was explicitly requested${worktreeError?.message ? `: ${worktreeError.message}` : ''}`, 'worktree-failed');
- return { outcome: 'blocked', reason };
- }
- } else if (!jiraBranchName && !isFalsyMeta(task.metadata?.useWorktree)) {
+ // A read-only public-review stage still needs a disposable checkout: it may
+ // run repository commands and the action stage's provider-specific sandbox
+ // needs it for tests/patch inspection. Never let `readOnly` turn an explicit
+ // isolation request into the live application checkout.
+ if (wantsWorktree && !jiraBranchName) {
+ const worktreeOutcome = await prepareRequestedWorktree({
+ agentId,
+ workspacePath,
+ task,
+ existingBranch,
+ allowSharedWorkspaceFallback: !isReadOnly && !explicitWorktree,
+ });
+ if (worktreeOutcome.outcome !== 'ready') return worktreeOutcome;
+ workspacePath = worktreeOutcome.workspacePath;
+ worktreeInfo = worktreeOutcome.worktreeInfo;
+ } else if (!isReadOnly && !jiraBranchName && !isFalsyMeta(task.metadata?.useWorktree)) {
const { getAgents } = await import('./cos.js');
const allAgents = await getAgents();
const runningAgents = allAgents.filter(a => a.status === 'running');
@@ -481,7 +515,6 @@ export async function prepareAgentWorkspace({ agentId, task }) {
emitLog('debug', `No conflicts for task ${task.id}, using shared workspace`, { taskId: task.id });
}
}
- } // end !isReadOnly
// Announce the FINAL cwd — emitted here, after any worktree reassignment
// above, so the task log names the directory the agent actually runs in
diff --git a/server/services/agentWorkspacePrep.test.js b/server/services/agentWorkspacePrep.test.js
index 40160b16c2..b954fbbc18 100644
--- a/server/services/agentWorkspacePrep.test.js
+++ b/server/services/agentWorkspacePrep.test.js
@@ -139,6 +139,48 @@ describe('prepareAgentWorkspace', () => {
expect(ensureLatest).not.toHaveBeenCalled();
});
+ it('read-only task with explicit isolation gets a disposable worktree', async () => {
+ createWorktree.mockResolvedValue({
+ worktreePath: '/mock/worktrees/agent-ro-isolated',
+ branchName: 'cos/t-ro-isolated/agent-ro-isolated',
+ baseBranch: 'main',
+ });
+ const task = {
+ id: 't-ro-isolated', taskType: 'internal',
+ metadata: { readOnly: true, useWorktree: true },
+ };
+
+ const r = await prepareAgentWorkspace({ agentId: 'agent-ro-isolated', task });
+
+ expect(r.outcome).toBe('ready');
+ expect(r.workspacePath).toBe('/mock/worktrees/agent-ro-isolated');
+ expect(r.worktreeInfo).toEqual(expect.objectContaining({
+ worktreePath: '/mock/worktrees/agent-ro-isolated',
+ }));
+ expect(createWorktree).toHaveBeenCalledWith('agent-ro-isolated', expect.any(String), 't-ro-isolated', expect.objectContaining({
+ baseBranch: 'main',
+ }));
+ expect(ensureLatest).not.toHaveBeenCalled();
+ expect(detectConflicts).not.toHaveBeenCalled();
+ });
+
+ it('blocks a read-only task when its required worktree cannot be created', async () => {
+ createWorktree.mockResolvedValue(null);
+ const task = {
+ id: 't-ro-blocked', taskType: 'internal',
+ metadata: { readOnly: true, useWorktree: true },
+ };
+
+ const r = await prepareAgentWorkspace({ agentId: 'agent-ro-blocked', task });
+
+ expect(r.outcome).toBe('blocked');
+ expect(r.reason).toContain('isolation was required');
+ expect(updateTask).toHaveBeenCalledWith('t-ro-blocked', expect.objectContaining({
+ status: 'blocked',
+ metadata: expect.objectContaining({ blockedCategory: 'worktree-failed' }),
+ }), 'internal');
+ });
+
it('plan-only task: keeps the no-worktree path even with delivery flags present', async () => {
const task = {
id: 't-plan-only',
diff --git a/server/services/cosTaskGenerator.js b/server/services/cosTaskGenerator.js
index a35d0b8f7c..49a0018699 100644
--- a/server/services/cosTaskGenerator.js
+++ b/server/services/cosTaskGenerator.js
@@ -22,7 +22,7 @@
import { readFile } from 'fs/promises';
import { existsSync } from 'fs';
import { join } from 'path';
-import { sanitizeTaskMetadata, PIPELINE_BEHAVIOR_FLAGS, MAX_TOTAL_SPAWNS, resolveClaimReviewerConfig, reviewerConfigMetadata, SWARM_COUNT_MIN, ISSUE_AUTHOR_FILTERS } from '../lib/validation.js';
+import { sanitizeTaskMetadata, PIPELINE_STAGE_BEHAVIOR_FLAGS, MAX_TOTAL_SPAWNS, resolveClaimReviewerConfig, reviewerConfigMetadata, SWARM_COUNT_MIN, ISSUE_AUTHOR_FILTERS } from '../lib/validation.js';
import { PATHS } from '../lib/fileUtils.js';
import { MODEL_ABUSE_GUARD_ID } from '../lib/modelAbuseGuard.js';
import { isPlainObject } from '../lib/objects.js';
@@ -71,6 +71,7 @@ import {
normalizeWorkItemRef,
} from './cosTaskPrompts.js';
import { appendTaskDataInputs, resolveTaskDataInputs } from './taskDataInputs.js';
+import { ensurePrReviewerPipeline } from './prReviewerPipeline.js';
export {
buildClaimOverrideContextBlock,
@@ -2102,7 +2103,7 @@ function initializePipelineMetadata(metadata) {
// Read-only stages default flags to false to prevent worktree/PR/simplify on review-only stages
metadata.pipeline.taskDefaults = {};
const stageReadOnly = stage0.readOnly ?? false;
- for (const flag of PIPELINE_BEHAVIOR_FLAGS) {
+ for (const flag of PIPELINE_STAGE_BEHAVIOR_FLAGS) {
if (metadata[flag] !== undefined) metadata.pipeline.taskDefaults[flag] = metadata[flag];
if (flag in stage0) {
metadata[flag] = stage0[flag];
@@ -2187,10 +2188,10 @@ function formatSecurityScanContext(scan, reports, status) {
`Reviewed ${reports.length} external pull request${reports.length === 1 ? '' : 's'}${findingCount ? `; ${findingCount} contained model-abuse flags or an unvalidated response` : ''}.`,
'No GitHub pull request or issue actions have been taken.',
status === 'findings'
- ? 'This scan is only a model-abuse boundary. Flagged PR content and its source text are withheld from Stage 2; Stage 2 may process only PRs explicitly marked safe and must not fetch or inspect flagged PRs.'
+ ? 'This scan is only a model-abuse boundary. Flagged PR content and its source text are withheld from the Eligibility Gate; the gate may process only PRs explicitly marked safe and must not fetch or inspect flagged PRs.'
: status === 'unavailable'
? `The scan stopped with ${scan.code || 'an unknown error'} after retaining the reports collected so far. No PR has a safe status; leave every PR untouched until the scan can be completed.`
- : 'All reviewed PRs have an explicit model-abuse safety status. Stage 2 may review only the PRs marked safe, after approval.',
+ : 'All reviewed PRs have an explicit model-abuse safety status. The Eligibility Gate may process only the PRs marked safe, after approval.',
].join('\n')
}
@@ -2209,9 +2210,9 @@ async function findActiveSecurityScanTask(appId, scanKey) {
/**
* Run pr-reviewer's Security Scan through the direct local, no-tools path and
- * hand only safe PR metadata to the next pipeline stage. A normal stage-0
- * agent is intentionally never spawned: `readOnly` is prompt guidance, not an
- * OS sandbox, and the generic agent resolver rejects API providers anyway.
+ * hand only safe PR metadata to the Eligibility Gate. A normal stage-0 agent
+ * is intentionally never spawned: `readOnly` is prompt guidance, not an OS
+ * sandbox, and the generic agent resolver rejects API providers anyway.
*
* External contributor PRs are held for human approval before the stage that
* can review, comment, or merge. The preflight itself remains read-only and
@@ -2231,7 +2232,7 @@ async function runPrReviewerSecurityPreflight(taskType, app, metadata, targetPul
const securityStage = stages?.[0];
const nextStage = stages?.[1];
if (!securityStage || !nextStage) {
- emitLog('warn', `Skipping pr-reviewer for ${app.name}: security pipeline requires two stages`, { appId: app.id, analysisType: taskType });
+ emitLog('warn', `Skipping pr-reviewer for ${app.name}: security pipeline requires an eligibility gate`, { appId: app.id, analysisType: taskType });
return { skipped: true };
}
@@ -2331,6 +2332,7 @@ async function runPrReviewerSecurityPreflight(taskType, app, metadata, targetPul
safePrCount: safeReports.length,
},
};
+ const safeInputByNumber = new Map((scan.reviewInputs || []).map((input) => [input.number, input]));
metadata.issueWatcher = {
repoFullName: scan.repoFullName || target.repoFullName,
defaultBranch: scan.defaultBranch || target.defaultBranch,
@@ -2338,6 +2340,8 @@ async function runPrReviewerSecurityPreflight(taskType, app, metadata, targetPul
pullRequests: safeReports.map((report) => ({
number: report.number,
headSha: report.headRefOid,
+ authorLogin: safeInputByNumber.get(report.number)?.authorLogin || null,
+ eligibilityFacts: safeInputByNumber.get(report.number)?.eligibilityFacts || null,
diffTruncated: false,
contentFingerprint: report.contentFingerprint,
})),
@@ -2360,7 +2364,7 @@ async function runPrReviewerSecurityPreflight(taskType, app, metadata, targetPul
if (nextStage.effort) metadata.effort = nextStage.effort;
const nextStageReadOnly = nextStage.readOnly ?? false;
const taskDefaults = metadata.pipeline.taskDefaults || {};
- for (const flag of PIPELINE_BEHAVIOR_FLAGS) {
+ for (const flag of PIPELINE_STAGE_BEHAVIOR_FLAGS) {
if (flag in nextStage) {
metadata[flag] = nextStage[flag];
} else if (nextStageReadOnly) {
@@ -3498,6 +3502,7 @@ export async function generateManagedAppImprovementTaskForType(taskType, app, st
const appOverride = appOverrides[taskType] || null;
const metadata = buildImprovementTaskMetadata(taskType, app, interval, taskSchedule, appOverride);
+ if (taskType === 'pr-reviewer') ensurePrReviewerPipeline(metadata);
initializePipelineMetadata(metadata);
const securityPreflight = await runPrReviewerSecurityPreflight(taskType, app, metadata, targetPullRequest);
if (securityPreflight.skipped) return null;
diff --git a/server/services/issueWatcher.js b/server/services/issueWatcher.js
index 50b6813d47..6e13a22b3e 100644
--- a/server/services/issueWatcher.js
+++ b/server/services/issueWatcher.js
@@ -24,7 +24,7 @@ import { getAppById, updateApp } from './apps.js';
import { execGh, ensureForgeReachable } from './github.js';
import { mergePR, resolveForgeForRepo } from './git.js';
import { addNotification, NOTIFICATION_TYPES, PRIORITY_LEVELS } from './notifications.js';
-import { runModelAbuseScan } from './modelAbuseGuard.js';
+import { normalizeEligibilityFacts, runModelAbuseScan } from './modelAbuseGuard.js';
const GH_TIMEOUT_MS = 60_000;
const LIST_LIMIT = 100;
@@ -279,6 +279,49 @@ async function readPullRequest(ctx, number) {
], ctx);
}
+function sameNumberList(left, right) {
+ const a = Array.isArray(left) ? left : [];
+ const b = Array.isArray(right) ? right : [];
+ return a.length === b.length && a.every((number, index) => number === b[index]);
+}
+
+/**
+ * Re-fetch the issue facts that admitted a public PR immediately before an
+ * action. Issue state and assignees can change while Stage 2/3 is running; an
+ * old allowlist must never remain sufficient for a later review or merge.
+ */
+async function eligibilityFactsStillCurrent(ctx, pr, target) {
+ const expected = normalizeEligibilityFacts(target?.eligibilityFacts);
+ const authorLogin = typeof target?.authorLogin === 'string' ? target.authorLogin.trim() : '';
+ if (!expected.issueLookupComplete || !authorLogin || !sameLogin(pr?.author?.login, authorLogin)) return false;
+ if (expected.linkedIssueNumbers.length === 0) return false;
+
+ const issues = await Promise.all(expected.linkedIssueNumbers.map((number) => (
+ runJson(apiArgs(ctx, `repos/${ctx.repoFullName}/issues/${number}`), ctx)
+ )));
+ if (issues.some((issue, index) => issue?.number !== expected.linkedIssueNumbers[index])) return false;
+
+ const openLinkedIssueNumbers = issues
+ .filter((issue) => !issue.pull_request && String(issue.state || '').toLowerCase() === 'open')
+ .map((issue) => issue.number);
+ const openerAssignedIssueNumbers = issues
+ .filter((issue) => !issue.pull_request && String(issue.state || '').toLowerCase() === 'open')
+ .filter((issue) => Array.isArray(issue.assignees) && issue.assignees.some((assignee) => (
+ sameLogin(assignee?.login, authorLogin)
+ )))
+ .map((issue) => issue.number);
+ const actual = normalizeEligibilityFacts({
+ linkedIssueNumbers: expected.linkedIssueNumbers,
+ openLinkedIssueNumbers,
+ openerAssignedIssueNumbers,
+ issueLookupComplete: true,
+ });
+ return sameNumberList(expected.linkedIssueNumbers, actual.linkedIssueNumbers)
+ && sameNumberList(expected.openLinkedIssueNumbers, actual.openLinkedIssueNumbers)
+ && sameNumberList(expected.openerAssignedIssueNumbers, actual.openerAssignedIssueNumbers)
+ && expected.issueLookupComplete === actual.issueLookupComplete;
+}
+
async function readBehindBy(ctx, pr) {
if (!pr?.baseRefOid || !pr?.headRefOid) return null;
const compare = await runJson(apiArgs(ctx, `repos/${ctx.repoFullName}/compare/${pr.baseRefOid}...${pr.headRefOid}`), ctx);
@@ -612,6 +655,12 @@ async function processPendingApprovals(app, ctx) {
changed = true;
continue;
}
+ if (approval.eligibilityFacts !== undefined
+ && !await eligibilityFactsStillCurrent(ctx, pr, approval)) {
+ await notifyPendingApproval(app, approval, 'The linked issue state or assignee changed, so the previous approval was discarded.');
+ changed = true;
+ continue;
+ }
if (approval.rebaseRequired) {
const behindBy = await readBehindBy(ctx, pr);
if (behindBy === null) {
@@ -861,7 +910,7 @@ function mergeApproval(existing, approval) {
}
/** Validated reply/review/rebase/merge pass run after cognition. */
-export async function processTaskOutput({ appId, success, payload, task } = {}) {
+export async function processTaskOutput({ appId, success, payload, task, requireEligibilityFacts = false } = {}) {
if (!appId || !success) return { action: 'no-op', reason: !success ? 'agent-failed' : 'missing-app' };
if (!isTaskOutputPayload(payload)) return { action: 'no-op', reason: 'unparseable-response' };
// A compromised reviewer must not be able to smuggle an instruction through
@@ -930,6 +979,14 @@ export async function processTaskOutput({ appId, success, payload, task } = {})
// exact content screened before cognition, not merely the same revision,
// before any review, rebase, or merge action.
if (!target.contentFingerprint || currentContentFingerprint !== target.contentFingerprint) continue;
+ const eligibilityRequired = requireEligibilityFacts
+ || Object.prototype.hasOwnProperty.call(target, 'eligibilityFacts');
+ const eligibilityStillCurrent = async () => {
+ if (!eligibilityRequired) return true;
+ const current = await eligibilityFactsStillCurrent(ctx, pr, target);
+ if (!current) approvals = approvals.filter((entry) => entry.number !== pr.number);
+ return current;
+ };
const anchors = parseAddedDiffLines(diff);
const normalizedFindings = decision.findings.map((finding) => normalizeFinding(finding, anchors)).filter(Boolean);
const findings = normalizedFindings.map(({ comment }) => comment);
@@ -946,6 +1003,7 @@ export async function processTaskOutput({ appId, success, payload, task } = {})
&& !diffInsufficient
&& blockingFindings.length === 0;
if (!canApprove) {
+ if (!await eligibilityStillCurrent()) continue;
const downgraded = hasInvalidFinding && decision.verdict !== 'request_changes';
const summary = `${decision.summary || 'This change needs follow-up before it can merge.'}${
downgraded ? '\n\nPortOS could not anchor one or more reported findings to this diff, so the review is blocking until they are restated against exact added lines.' : ''}`;
@@ -962,6 +1020,7 @@ export async function processTaskOutput({ appId, success, payload, task } = {})
}
const approveBody = decision.summary || 'Reviewed: no material issues found.';
+ if (!await eligibilityStillCurrent()) continue;
const approved = await submitReview(ctx, pr.number, {
body: approveBody,
event: 'APPROVE',
@@ -975,12 +1034,15 @@ export async function processTaskOutput({ appId, success, payload, task } = {})
const behindBy = await readBehindBy(ctx, pr);
if (decision.rebaseRequired && (behindBy === null || behindBy > 0)) {
+ if (!await eligibilityStillCurrent()) continue;
const updated = behindBy > 0 && await updatePullRequestBranch(ctx, pr.number, pr.headRefOid);
if (updated) rebased += 1;
else approvals = mergeApproval(approvals, {
number: pr.number,
headSha: pr.headRefOid,
contentFingerprint: target.contentFingerprint,
+ authorLogin: target.authorLogin,
+ eligibilityFacts: target.eligibilityFacts,
url: pr.url,
ciPolicy: decision.ciPolicy,
rebaseRequired: true,
@@ -994,6 +1056,7 @@ export async function processTaskOutput({ appId, success, payload, task } = {})
const checks = classifyChecks(checkRollup);
const mayMerge = pr.mergeable === 'MERGEABLE' && checks === 'green';
if (mayMerge) {
+ if (!await eligibilityStillCurrent()) continue;
const result = await mergePR(app.repoPath, pr.number).catch(() => ({ success: false }));
if (result.success) {
approvals = approvals.filter((entry) => entry.number !== pr.number);
@@ -1013,6 +1076,8 @@ export async function processTaskOutput({ appId, success, payload, task } = {})
number: pr.number,
headSha: pr.headRefOid,
contentFingerprint: target.contentFingerprint,
+ authorLogin: target.authorLogin,
+ eligibilityFacts: target.eligibilityFacts,
url: pr.url,
ciPolicy: decision.ciPolicy,
rebaseRequired: false,
diff --git a/server/services/issueWatcher.test.js b/server/services/issueWatcher.test.js
index 6e96d5321a..eee5114d52 100644
--- a/server/services/issueWatcher.test.js
+++ b/server/services/issueWatcher.test.js
@@ -27,7 +27,8 @@ vi.mock('./notifications.js', () => ({
}));
const runModelAbuseScanMock = vi.fn();
-vi.mock('./modelAbuseGuard.js', () => ({
+vi.mock('./modelAbuseGuard.js', async (importOriginal) => ({
+ ...(await importOriginal()),
MODEL_ABUSE_GUARD_ID: 'llama-prompt-guard-2-86m',
MODEL_ABUSE_GUARD_MAX_INPUT_CHARS: 2_000_000,
runModelAbuseScan: (...args) => runModelAbuseScanMock(...args),
@@ -89,7 +90,9 @@ function pullRequest(overrides = {}) {
};
}
-function installDefaultGhMock({ pr = pullRequest(), issueRows = [[]], commentRows = [[]], reviews = [[]] } = {}) {
+function installDefaultGhMock({
+ pr = pullRequest(), issueRows = [[]], commentRows = [[]], reviews = [[]], issueDetails = {},
+} = {}) {
execGhMock.mockImplementation(async (args) => {
if (args[0] === 'api' && args.includes('repos/o/r') && !args.some((arg) => String(arg).includes('/issues'))
&& !args.some((arg) => String(arg).includes('/pulls/')) && !args.some((arg) => String(arg).includes('/compare/'))) {
@@ -97,6 +100,11 @@ function installDefaultGhMock({ pr = pullRequest(), issueRows = [[]], commentRow
}
if (args[0] === 'api' && args.some((arg) => String(arg).endsWith('/issues'))) return JSON.stringify(issueRows);
if (args[0] === 'api' && args.some((arg) => String(arg).includes('/comments'))) return JSON.stringify(commentRows);
+ const issueDetail = args
+ .map((arg) => String(arg))
+ .map((arg) => arg.match(/^repos\/o\/r\/issues\/(\d+)$/))
+ .find(Boolean);
+ if (args[0] === 'api' && issueDetail) return JSON.stringify(issueDetails[issueDetail[1]] || {});
if (args[0] === 'pr' && args[1] === 'list') {
return JSON.stringify([{ number: pr.number, title: pr.title, author: pr.author, url: pr.url, isDraft: false, headRefOid: pr.headRefOid, updatedAt: '2026-08-30T01:00:00Z' }]);
}
@@ -283,6 +291,23 @@ describe('processTaskOutput', () => {
}],
},
};
+ const eligibilityFacts = {
+ linkedIssueNumbers: [101],
+ openLinkedIssueNumbers: [101],
+ openerAssignedIssueNumbers: [101],
+ issueLookupComplete: true,
+ };
+ const eligibilityMetadata = {
+ issueWatcher: {
+ ...metadata.issueWatcher,
+ strictPullRequestCoverage: true,
+ pullRequests: [{
+ ...metadata.issueWatcher.pullRequests[0],
+ authorLogin: 'contributor',
+ eligibilityFacts,
+ }],
+ },
+ };
it('posts validated findings as inline review comments and never merges', async () => {
installDefaultGhMock();
@@ -396,6 +421,63 @@ describe('processTaskOutput', () => {
expect(mergePrMock).toHaveBeenCalledWith(APP.repoPath, 7);
});
+ it.each([
+ ['the linked issue closes', { number: 101, state: 'closed', assignees: [{ login: 'contributor' }] }],
+ ['the contributor is unassigned', { number: 101, state: 'open', assignees: [] }],
+ ])('does not review or merge when %s after the eligibility pass', async (_description, issue) => {
+ installDefaultGhMock({ issueDetails: { 101: issue } });
+ mergePrMock.mockResolvedValue({ success: true });
+ const payload = {
+ issueComments: [],
+ pullRequests: [{
+ number: 7, headSha: 'a'.repeat(40), verdict: 'approve', summary: 'No material issues found.', findings: [],
+ rebaseRequired: false, ciPolicy: 'required',
+ }],
+ };
+
+ const result = await processTaskOutput({
+ appId: APP.id,
+ success: true,
+ payload,
+ task: { metadata: eligibilityMetadata },
+ requireEligibilityFacts: true,
+ });
+
+ expect(result).toMatchObject({ reviewed: 0, merged: 0 });
+ expect(execGhMock.mock.calls.some(([args]) => args.includes('/reviews'))).toBe(false);
+ expect(mergePrMock).not.toHaveBeenCalled();
+ });
+
+ it('revalidates matching issue facts before approving and merging', async () => {
+ installDefaultGhMock({
+ issueDetails: {
+ 101: { number: 101, state: 'open', assignees: [{ login: 'contributor' }] },
+ },
+ });
+ mergePrMock.mockResolvedValue({ success: true });
+ const payload = {
+ issueComments: [],
+ pullRequests: [{
+ number: 7, headSha: 'a'.repeat(40), verdict: 'approve', summary: 'No material issues found.', findings: [],
+ rebaseRequired: false, ciPolicy: 'required',
+ }],
+ };
+
+ const result = await processTaskOutput({
+ appId: APP.id,
+ success: true,
+ payload,
+ task: { metadata: eligibilityMetadata },
+ requireEligibilityFacts: true,
+ });
+
+ expect(result).toMatchObject({ reviewed: 1, merged: 1 });
+ expect(mergePrMock).toHaveBeenCalledWith(APP.repoPath, 7);
+ expect(execGhMock.mock.calls.some(([args]) => (
+ args[0] === 'api' && args.some((arg) => String(arg).endsWith('/issues/101'))
+ ))).toBe(true);
+ });
+
it('posts non-blocking findings on an approving review and still merges', async () => {
installDefaultGhMock();
mergePrMock.mockResolvedValue({ success: true });
diff --git a/server/services/modelAbuseGuard.js b/server/services/modelAbuseGuard.js
index 05581c1c3f..146ff27e3f 100644
--- a/server/services/modelAbuseGuard.js
+++ b/server/services/modelAbuseGuard.js
@@ -37,7 +37,7 @@ import {
} from '../lib/modelAbuseGuard.js';
import { findCachedRepoFiles } from '../lib/hfCache.js';
import { localRuntimeForProvider } from '../lib/localProviderRuntime.js';
-import { supportsPublicReviewProvider } from '../lib/providerVendors.js';
+import { supportsPublicReviewPosture, PUBLIC_REVIEW_NO_TOOL_POSTURE } from '../lib/providerVendors.js';
import { withSpawnCwdEnv } from '../lib/spawnCwd.js';
import { detectVenvBasePythonSync, createVenv, installPackages } from '../lib/pythonSetup.js';
import { safeChildProcessOptions } from '../lib/processEnv.js';
@@ -62,6 +62,8 @@ const MAX_INSTALL_EVENT_CHARS = 300;
const MAX_PUBLIC_REVIEW_SNAPSHOT_CHARS = MODEL_ABUSE_GUARD_MAX_INPUT_CHARS * 3;
const PUBLIC_REVIEW_INPUT_DIR = join(PATHS.cos, 'public-review-inputs');
export const PUBLIC_REVIEW_INPUT_FILENAME = 'PORTOS_PUBLIC_REVIEW_INPUT.json';
+export const PUBLIC_REVIEW_PATCH_DIRNAME = '.portos-public-review';
+export const PUBLIC_REVIEW_PATCH_MANIFEST_FILENAME = 'PORTOS_PUBLIC_REVIEW_PATCHES.json';
let cachedRuntime = null;
let installInFlight = null;
@@ -76,7 +78,31 @@ const publicReviewInputPath = (scanKey) => isScanKey(scanKey)
? join(PUBLIC_REVIEW_INPUT_DIR, `${scanKey}.json`)
: null;
-function normalizePublicReviewInput(input) {
+const normalizeIssueNumbers = (value) => Array.isArray(value)
+ ? [...new Set(value.filter((number) => Number.isInteger(number) && number > 0 && number <= 1_000_000))]
+ .sort((a, b) => a - b)
+ .slice(0, 50)
+ : [];
+
+const emptyEligibilityFacts = () => ({
+ linkedIssueNumbers: [],
+ openLinkedIssueNumbers: [],
+ openerAssignedIssueNumbers: [],
+ // Unknown is deliberately false. A missing facts object is not approval.
+ issueLookupComplete: false,
+});
+
+export function normalizeEligibilityFacts(value) {
+ if (!value || typeof value !== 'object' || Array.isArray(value)) return emptyEligibilityFacts();
+ return {
+ linkedIssueNumbers: normalizeIssueNumbers(value.linkedIssueNumbers),
+ openLinkedIssueNumbers: normalizeIssueNumbers(value.openLinkedIssueNumbers),
+ openerAssignedIssueNumbers: normalizeIssueNumbers(value.openerAssignedIssueNumbers),
+ issueLookupComplete: value.issueLookupComplete === true,
+ };
+}
+
+export function normalizePublicReviewInput(input) {
if (!input || typeof input !== 'object' || Array.isArray(input)) return null;
if (!Number.isInteger(input.number) || input.number < 1 || !isSha(input.headSha)) return null;
if (typeof input.title !== 'string' || typeof input.body !== 'string' || typeof input.diff !== 'string') return null;
@@ -93,6 +119,7 @@ function normalizePublicReviewInput(input) {
files: Array.isArray(input.files) ? input.files.filter((file) => typeof file === 'string').slice(0, 10_000) : [],
additions: Number.isInteger(input.additions) ? input.additions : 0,
deletions: Number.isInteger(input.deletions) ? input.deletions : 0,
+ eligibilityFacts: normalizeEligibilityFacts(input.eligibilityFacts),
diff: input.diff,
};
}
@@ -108,24 +135,39 @@ function normalizePublicReviewInputs(pullRequests) {
}
/**
- * Revalidate the Stage 2 model at spawn time.
+ * Revalidate a public-review stage's provider + model at spawn time.
*
* The picker is only a convenience boundary: schedules and API payloads can
- * be edited without the browser. A public-content review therefore cannot
- * rely on a provider/model choice that was previously accepted by the UI.
- * Require the maintained local Claude wrapper, an actually installed Ollama
- * model, and an authoritative text capability report with no `tools` entry.
- * Unknown, stale, or unprobeable capability state is rejected.
+ * be edited without the browser. A public-content stage therefore cannot rely
+ * on a provider/model choice that was previously accepted by the UI.
+ *
+ * Two independent controls apply, and which one is authoritative depends on
+ * where the model runs:
+ *
+ * - Every provider must carry a maintained vendor recipe for the requested
+ * posture. That argv (codex `--sandbox read-only`, grok
+ * `--permission-mode plan --tools ''`, claude `--restricted --tools ''`, …)
+ * is what actually denies tools to a CLOUD model, which PortOS cannot probe.
+ * - A LOCAL runtime can be probed, so it gets the stricter check it always
+ * had: the model must be installed and its authoritative capability report
+ * must contain no `tools` entry. Unknown or unprobeable state fails closed.
+ *
+ * A model id is required only where PortOS picks one. Vendors that select
+ * their own model (grok, antigravity) legitimately run with no `--model` pin.
*/
-export async function validatePublicReviewModel({ provider, model } = {}) {
- if (!supportsPublicReviewProvider(provider)) {
+export async function validatePublicReviewModel({ provider, model, posture = PUBLIC_REVIEW_NO_TOOL_POSTURE } = {}) {
+ if (!supportsPublicReviewPosture(provider, posture)) {
return publicReviewModelFailure('public-review-provider-unsupported');
}
const modelId = typeof model === 'string' ? model.trim() : '';
- if (!modelId) return publicReviewModelFailure('public-review-model-required');
-
const runtime = localRuntimeForProvider(provider);
- if (!runtime || runtime.kind !== 'ollama') {
+ if (!runtime) return { ok: true, model: modelId || null, runtime: null };
+
+ // Local runtimes are probeable, so the capability report is authoritative
+ // and a model id is mandatory — there is no server-side default to fall back
+ // to for an Ollama/LM Studio wrapper.
+ if (!modelId) return publicReviewModelFailure('public-review-model-required');
+ if (runtime.kind !== 'ollama') {
return publicReviewModelFailure('public-review-runtime-unsupported');
}
@@ -166,11 +208,12 @@ export async function writePublicReviewInputSnapshot({ scanKey, pullRequests } =
/**
* Read and validate the server-owned cleared snapshot. This is intentionally
- * separate from `materializePublicReviewInput`: the no-tools reviewer receives
- * the validated JSON in its prompt, while the read-only file remains an audit
- * copy and defense-in-depth fallback.
+ * separate from `materializePublicReviewInput`: the reviewer receives the
+ * validated JSON in its prompt, while the read-only file remains an audit copy
+ * and defense-in-depth fallback. An optional allowlist is applied only after
+ * the complete snapshot has been validated, so Stage 3 cannot widen its input.
*/
-export async function readPublicReviewInputSnapshot({ scanKey } = {}) {
+export async function readPublicReviewInputSnapshot({ scanKey, allowedPullRequestNumbers = null } = {}) {
const sourcePath = publicReviewInputPath(scanKey);
if (!sourcePath) return null;
const raw = await tryReadFile(sourcePath);
@@ -178,23 +221,70 @@ export async function readPublicReviewInputSnapshot({ scanKey } = {}) {
if (!parsed || parsed.schemaVersion !== 1 || parsed.scanKey !== scanKey) return null;
const pullRequests = normalizePublicReviewInputs(parsed.pullRequests);
if (!pullRequests) return null;
- return { schemaVersion: 1, scanKey, pullRequests };
+ const allowed = Array.isArray(allowedPullRequestNumbers)
+ ? new Set(allowedPullRequestNumbers.filter((number) => Number.isInteger(number) && number > 0))
+ : null;
+ return {
+ schemaVersion: 1,
+ scanKey,
+ pullRequests: allowed ? pullRequests.filter((pullRequest) => allowed.has(pullRequest.number)) : pullRequests,
+ };
}
/**
- * Materialize a screened snapshot inside the throwaway Stage 2 worktree. The
- * review CLI can read this file in its enforced read-only posture; it never
+ * Materialize a screened snapshot inside the throwaway review worktree. The
+ * review CLI can read this file in its enforced posture; it never
* needs network access or a contributor checkout to inspect the diff.
*/
-export async function materializePublicReviewInput({ scanKey, workspacePath } = {}) {
+export async function materializePublicReviewInput({ scanKey, workspacePath, allowedPullRequestNumbers = null } = {}) {
if (typeof workspacePath !== 'string' || !workspacePath) return false;
- const parsed = await readPublicReviewInputSnapshot({ scanKey });
+ const parsed = await readPublicReviewInputSnapshot({ scanKey, allowedPullRequestNumbers });
if (!parsed) return false;
const destination = join(workspacePath, PUBLIC_REVIEW_INPUT_FILENAME);
await atomicWrite(destination, parsed);
return chmod(destination, 0o444).then(() => true).catch(() => false);
}
+/**
+ * Materialize one screened unified diff per eligible PR for the final review
+ * stage. The files live only in that stage's disposable worktree and are
+ * read-only, so the sandboxed reviewer can apply them with `git apply` and run
+ * tests without fetching a contributor branch or contacting the forge. The
+ * manifest contains only safe identities and relative paths; source text stays
+ * in the patch files and the already-validated input envelope.
+ */
+export async function materializePublicReviewPatches({ scanKey, workspacePath, allowedPullRequestNumbers = null } = {}) {
+ if (typeof workspacePath !== 'string' || !workspacePath) return false;
+ const parsed = await readPublicReviewInputSnapshot({ scanKey, allowedPullRequestNumbers });
+ if (!parsed) return false;
+
+ const patchDir = join(workspacePath, PUBLIC_REVIEW_PATCH_DIRNAME);
+ await ensureDir(patchDir);
+ const patches = [];
+ for (const pullRequest of parsed.pullRequests) {
+ const filename = `PR-${pullRequest.number}.patch`;
+ const relativePath = `${PUBLIC_REVIEW_PATCH_DIRNAME}/${filename}`;
+ const destination = join(patchDir, filename);
+ await atomicWrite(destination, pullRequest.diff);
+ const restricted = await chmod(destination, 0o444).then(() => true).catch(() => false);
+ if (!restricted) return false;
+ patches.push({
+ number: pullRequest.number,
+ headSha: pullRequest.headSha,
+ contentFingerprint: pullRequest.contentFingerprint || null,
+ path: relativePath,
+ });
+ }
+
+ const manifest = join(patchDir, PUBLIC_REVIEW_PATCH_MANIFEST_FILENAME);
+ await atomicWrite(manifest, {
+ schemaVersion: 1,
+ scanKey,
+ patches,
+ });
+ return chmod(manifest, 0o444).then(() => true).catch(() => false);
+}
+
/**
* Build the scanner's environment. This deliberately does not reuse the
* normal CLI environment builder: that builder carries forge/provider auth so
diff --git a/server/services/modelAbuseGuard.test.js b/server/services/modelAbuseGuard.test.js
index 6ea217e901..c2684c1c17 100644
--- a/server/services/modelAbuseGuard.test.js
+++ b/server/services/modelAbuseGuard.test.js
@@ -48,10 +48,17 @@ describe('validatePublicReviewModel', () => {
await expect(validatePublicReviewModel({ provider: LOCAL_CLAUDE, model: 'safe-model' }))
.resolves.toMatchObject({ ok: false, code: 'public-review-model-not-tool-free' });
+ // A vendor with no maintained recipe for the posture is rejected before
+ // any model probing — that check, not the model's location, is the gate.
await expect(validatePublicReviewModel({
- provider: { ...LOCAL_CLAUDE, envVars: { ANTHROPIC_BASE_URL: 'https://api.example.com' } },
+ provider: { ...LOCAL_CLAUDE, command: 'custom-agent' },
model: 'safe-model',
})).resolves.toMatchObject({ ok: false, code: 'public-review-provider-unsupported' });
+ await expect(validatePublicReviewModel({
+ provider: LOCAL_CLAUDE,
+ model: 'safe-model',
+ posture: 'sandboxed-actions',
+ })).resolves.toMatchObject({ ok: false, code: 'public-review-provider-unsupported' });
});
it('rejects missing model selection and an unavailable catalog', async () => {
diff --git a/server/services/prReviewerPipeline.js b/server/services/prReviewerPipeline.js
new file mode 100644
index 0000000000..0d571c6e06
--- /dev/null
+++ b/server/services/prReviewerPipeline.js
@@ -0,0 +1,223 @@
+/**
+ * Role-aware output contract for the pr-reviewer pipeline.
+ *
+ * Security Scan is a server-side preflight. The Eligibility Gate is a
+ * reasoning-only, tool-free stage whose only durable result is a boolean
+ * allowlist. The Actions stage reuses issue-watcher's deterministic forge
+ * coordinator after the eligible set has been narrowed. Keeping this wrapper
+ * separate means the action hook cannot accidentally consume an eligibility
+ * response, and eligibility reasons can never cross into the action stage.
+ */
+
+import { MODEL_ABUSE_GUARD_ID } from '../lib/modelAbuseGuard.js';
+import { PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE, PUBLIC_REVIEW_GATE_EXECUTION_PROFILE } from '../lib/agentExecutionProfiles.js';
+import { createPrReviewerDefaultStages } from './taskScheduleRegistry.js';
+import {
+ isTaskOutputPayload as isIssueWatcherPayload,
+ processTaskOutput as processIssueWatcherOutput,
+} from './issueWatcher.js';
+
+const HEAD_SHA_RE = /^[a-f0-9]{40}$/i;
+const CONTENT_FINGERPRINT_RE = /^[a-f0-9]{64}$/i;
+const MAX_REASON_CHARS = 2_000;
+
+const roleForPromptKey = (promptKey) => ({
+ 'pr-reviewer-security': 'security',
+ 'pr-reviewer-eligibility': 'eligibility',
+ 'pr-reviewer-review': 'actions',
+}[promptKey] || null);
+
+export function prReviewerStageRole(stage) {
+ if (['security', 'eligibility', 'actions'].includes(stage?.role)) return stage.role;
+ return roleForPromptKey(stage?.promptKey);
+}
+
+function stageWithContract(stage, role) {
+ const base = { ...(stage || {}), role, managed: true, readOnly: true };
+ if (role === 'security') {
+ return {
+ ...base,
+ promptKey: 'pr-reviewer-security',
+ guardId: MODEL_ABUSE_GUARD_ID,
+ };
+ }
+ const executionProfile = role === 'eligibility'
+ ? PUBLIC_REVIEW_GATE_EXECUTION_PROFILE
+ : PUBLIC_REVIEW_ACTIONS_EXECUTION_PROFILE;
+ return {
+ ...base,
+ promptKey: role === 'eligibility' ? 'pr-reviewer-eligibility' : 'pr-reviewer-review',
+ useWorktree: true,
+ openPR: false,
+ simplify: false,
+ reviewLoop: false,
+ discardWorktree: true,
+ noCodeOutput: true,
+ executionProfile,
+ };
+}
+
+/**
+ * Normalize a pr-reviewer pipeline before it is initialized. Old persisted
+ * schedules used two stages and unlabelled stage objects; insert the mandatory
+ * gate while preserving the old review stage's provider/model/effort pins as
+ * the optional Actions stage. The operation is idempotent.
+ */
+export function ensurePrReviewerPipeline(metadata) {
+ const stages = metadata?.pipeline?.stages;
+ if (!Array.isArray(stages) || stages.length === 0) return metadata;
+
+ const defaultStages = createPrReviewerDefaultStages();
+ const firstIsSecurity = prReviewerStageRole(stages[0]) === 'security';
+ const security = stageWithContract(firstIsSecurity ? stages[0] : defaultStages[0], 'security');
+ const candidates = firstIsSecurity ? stages.slice(1) : stages;
+ const eligibilityCandidate = candidates.find((stage) => prReviewerStageRole(stage) === 'eligibility');
+ const eligibility = stageWithContract(eligibilityCandidate || defaultStages[1], 'eligibility');
+ const actionCandidates = candidates.filter((stage) => stage !== eligibilityCandidate);
+ const actions = actionCandidates.map((stage) => stageWithContract(stage, 'actions'));
+ const nextStages = [security, eligibility, ...actions];
+ metadata.pipeline = { ...metadata.pipeline, stages: nextStages };
+ return metadata;
+}
+
+function normalizedExpectedPullRequests(task) {
+ const expected = task?.metadata?.issueWatcher;
+ if (!expected || expected.strictPullRequestCoverage !== true || !Array.isArray(expected.pullRequests)) return null;
+ const seen = new Set();
+ const pullRequests = [];
+ for (const item of expected.pullRequests) {
+ if (!Number.isInteger(item?.number) || item.number < 1 || seen.has(item.number)) return null;
+ if (!HEAD_SHA_RE.test(item.headSha) || !CONTENT_FINGERPRINT_RE.test(item.contentFingerprint)) return null;
+ if (typeof item.authorLogin !== 'string' || !item.authorLogin.trim()) return null;
+ seen.add(item.number);
+ pullRequests.push({
+ number: item.number,
+ headSha: item.headSha,
+ contentFingerprint: item.contentFingerprint,
+ authorLogin: item.authorLogin,
+ eligibilityFacts: item.eligibilityFacts,
+ });
+ }
+ return pullRequests;
+}
+
+function eligibilityFactsAllow(facts) {
+ if (!facts || facts.issueLookupComplete !== true) return false;
+ const linked = new Set(Array.isArray(facts.linkedIssueNumbers) ? facts.linkedIssueNumbers : []);
+ const open = new Set(Array.isArray(facts.openLinkedIssueNumbers) ? facts.openLinkedIssueNumbers : []);
+ const assigned = new Set(Array.isArray(facts.openerAssignedIssueNumbers) ? facts.openerAssignedIssueNumbers : []);
+ return linked.size > 0
+ && [...assigned].some((number) => linked.has(number) && open.has(number));
+}
+
+function validateEligibilityDecision(raw, expected) {
+ if (!raw || typeof raw !== 'object' || Array.isArray(raw)) return null;
+ if (!Number.isInteger(raw.number) || raw.number < 1 || !HEAD_SHA_RE.test(raw.headSha)) return null;
+ if (typeof raw.eligible !== 'boolean' || typeof raw.reason !== 'string') return null;
+ const reason = raw.reason.trim();
+ if (!reason || reason.length > MAX_REASON_CHARS) return null;
+ const target = expected.get(raw.number);
+ if (!target || target.headSha !== raw.headSha) return null;
+ return {
+ number: raw.number,
+ headSha: raw.headSha,
+ eligible: raw.eligible && eligibilityFactsAllow(target.eligibilityFacts),
+ };
+}
+
+function invalidEligibility(reason, message = 'The eligibility gate did not return a complete validated decision set') {
+ return { action: 'no-op', accepted: false, reason, message };
+}
+
+function processEligibilityTaskOutput({ appId, success, payload, task } = {}) {
+ if (!appId) return invalidEligibility('missing-app');
+ if (!success) return invalidEligibility('agent-failed', 'The eligibility gate agent failed before returning a decision');
+ if (!payload || typeof payload !== 'object' || Array.isArray(payload) || !Array.isArray(payload.decisions)) {
+ return invalidEligibility('eligibility-response-invalid');
+ }
+ if (typeof payload.eligible !== 'boolean') return invalidEligibility('eligibility-response-invalid');
+
+ const expectedList = normalizedExpectedPullRequests(task);
+ if (!expectedList) return invalidEligibility('missing-eligibility-metadata');
+ const expected = new Map(expectedList.map((item) => [item.number, item]));
+ const decisions = [];
+ const seen = new Set();
+ for (const raw of payload.decisions) {
+ if (seen.has(raw?.number)) return invalidEligibility('eligibility-response-incomplete');
+ const decision = validateEligibilityDecision(raw, expected);
+ if (!decision) return invalidEligibility('eligibility-response-invalid');
+ seen.add(decision.number);
+ decisions.push(decision);
+ }
+ if (decisions.length !== expected.size || seen.size !== expected.size) {
+ return invalidEligibility('eligibility-response-incomplete');
+ }
+ // The outer flag is redundant but useful as a tamper-evident envelope field.
+ // Compare it with the model's own per-PR answers before applying the stricter
+ // server-side issue/assignment facts above.
+ const modelEligible = payload.decisions.some((decision) => decision?.eligible === true);
+ if (payload.eligible !== modelEligible) return invalidEligibility('eligibility-response-contradictory');
+
+ const eligibleNumbers = decisions.filter((decision) => decision.eligible).map((decision) => decision.number);
+ const rejectedNumbers = decisions.filter((decision) => !decision.eligible).map((decision) => decision.number);
+ const nextIssueWatcher = {
+ ...task.metadata.issueWatcher,
+ pullRequests: expectedList
+ .filter((item) => eligibleNumbers.includes(item.number))
+ .map((item) => ({
+ number: item.number,
+ headSha: item.headSha,
+ contentFingerprint: item.contentFingerprint,
+ authorLogin: item.authorLogin,
+ eligibilityFacts: item.eligibilityFacts,
+ diffTruncated: false,
+ })),
+ };
+ const eligibility = {
+ complete: true,
+ evaluatedCount: decisions.length,
+ eligibleNumbers,
+ rejectedNumbers,
+ decisions,
+ };
+ const previousStageOutput = JSON.stringify({
+ eligibility: 'passed',
+ complete: true,
+ evaluatedCount: decisions.length,
+ eligibleNumbers,
+ rejectedNumbers,
+ });
+ return {
+ action: 'eligibility-evaluated',
+ accepted: true,
+ terminal: eligibleNumbers.length === 0,
+ taskMetadata: {
+ issueWatcher: nextIssueWatcher,
+ prReviewerEligibility: eligibility,
+ pipeline: {
+ ...task.metadata.pipeline,
+ eligibility,
+ previousStageOutput,
+ ...(eligibleNumbers.length === 0
+ ? { status: 'filtered', terminalReason: 'no-eligible-prs' }
+ : {}),
+ },
+ },
+ };
+}
+
+export function isEligibilityPayload(payload) {
+ return Boolean(payload && typeof payload === 'object' && !Array.isArray(payload)
+ && typeof payload.eligible === 'boolean' && Array.isArray(payload.decisions));
+}
+
+export function isTaskOutputPayload(payload) {
+ return isEligibilityPayload(payload) || isIssueWatcherPayload(payload);
+}
+
+export async function processTaskOutput(args = {}, deps) {
+ const role = prReviewerStageRole(args.task?.metadata?.pipeline?.stages?.[args.task?.metadata?.pipeline?.currentStage ?? 0]);
+ if (role === 'eligibility') return processEligibilityTaskOutput(args);
+ if (role === 'actions') return processIssueWatcherOutput({ ...args, requireEligibilityFacts: true }, deps);
+ return invalidEligibility('unsupported-pr-review-stage');
+}
diff --git a/server/services/prReviewerPipeline.test.js b/server/services/prReviewerPipeline.test.js
new file mode 100644
index 0000000000..d14d066f1b
--- /dev/null
+++ b/server/services/prReviewerPipeline.test.js
@@ -0,0 +1,227 @@
+import { describe, expect, it, vi } from 'vitest';
+
+const issueWatcherMock = vi.hoisted(() => ({
+ isTaskOutputPayload: vi.fn((payload) => Boolean(payload?.issueComments || payload?.pullRequests)),
+ processTaskOutput: vi.fn(),
+}));
+
+vi.mock('./issueWatcher.js', () => issueWatcherMock);
+
+import {
+ ensurePrReviewerPipeline,
+ isEligibilityPayload,
+ isTaskOutputPayload,
+ processTaskOutput,
+} from './prReviewerPipeline.js';
+
+const HEAD_SHA = 'a'.repeat(40);
+const CONTENT_FINGERPRINT = 'b'.repeat(64);
+
+const eligibleFacts = {
+ linkedIssueNumbers: [101],
+ openLinkedIssueNumbers: [101],
+ openerAssignedIssueNumbers: [101],
+ issueLookupComplete: true,
+};
+
+function eligibilityTask(overrides = {}) {
+ return {
+ metadata: {
+ issueWatcher: {
+ strictPullRequestCoverage: true,
+ pullRequests: [{
+ number: 12,
+ headSha: HEAD_SHA,
+ contentFingerprint: CONTENT_FINGERPRINT,
+ authorLogin: 'contributor',
+ eligibilityFacts: eligibleFacts,
+ }],
+ },
+ pipeline: {
+ currentStage: 1,
+ stages: [
+ { role: 'security', promptKey: 'pr-reviewer-security' },
+ { role: 'eligibility', promptKey: 'pr-reviewer-eligibility' },
+ ],
+ },
+ ...overrides,
+ },
+ };
+}
+
+const decisionPayload = (overrides = {}) => ({
+ eligible: true,
+ decisions: [{
+ number: 12,
+ headSha: HEAD_SHA,
+ eligible: true,
+ reason: 'Linked issue and focused implementation.',
+ }],
+ ...overrides,
+});
+
+describe('ensurePrReviewerPipeline', () => {
+ it('inserts the mandatory eligibility gate and preserves the former review pins as actions', () => {
+ const metadata = {
+ pipeline: {
+ stages: [
+ { promptKey: 'pr-reviewer-security', readOnly: true },
+ { promptKey: 'pr-reviewer-review', providerId: 'codex-cli', model: 'gpt-5.6', effort: 'high' },
+ ],
+ },
+ };
+
+ ensurePrReviewerPipeline(metadata);
+
+ expect(metadata.pipeline.stages).toEqual([
+ expect.objectContaining({ role: 'security', promptKey: 'pr-reviewer-security', readOnly: true }),
+ expect.objectContaining({ role: 'eligibility', promptKey: 'pr-reviewer-eligibility', readOnly: true }),
+ expect.objectContaining({
+ role: 'actions',
+ promptKey: 'pr-reviewer-review',
+ providerId: 'codex-cli',
+ model: 'gpt-5.6',
+ effort: 'high',
+ executionProfile: 'public-review-actions',
+ }),
+ ]);
+ });
+
+ it('keeps a gate-only pipeline gate-only', () => {
+ const metadata = {
+ pipeline: {
+ stages: [
+ { role: 'security', promptKey: 'pr-reviewer-security' },
+ { role: 'eligibility', promptKey: 'pr-reviewer-eligibility' },
+ ],
+ },
+ };
+
+ ensurePrReviewerPipeline(metadata);
+ expect(metadata.pipeline.stages).toHaveLength(2);
+ expect(metadata.pipeline.stages[1].role).toBe('eligibility');
+ });
+});
+
+describe('pr-reviewer eligibility output', () => {
+ it('returns only the eligible allowlist and carries the server facts into validation', async () => {
+ const result = await processTaskOutput({
+ appId: 'app-example',
+ success: true,
+ payload: decisionPayload(),
+ task: eligibilityTask(),
+ });
+
+ expect(result).toMatchObject({ action: 'eligibility-evaluated', accepted: true, terminal: false });
+ expect(result.taskMetadata.issueWatcher.pullRequests).toEqual([{
+ number: 12,
+ headSha: HEAD_SHA,
+ contentFingerprint: CONTENT_FINGERPRINT,
+ authorLogin: 'contributor',
+ eligibilityFacts: eligibleFacts,
+ diffTruncated: false,
+ }]);
+ expect(result.taskMetadata.prReviewerEligibility).toMatchObject({
+ complete: true,
+ eligibleNumbers: [12],
+ rejectedNumbers: [],
+ });
+ expect(result.taskMetadata.prReviewerEligibility.decisions[0]).toEqual({
+ number: 12,
+ headSha: HEAD_SHA,
+ eligible: true,
+ });
+ expect(result.taskMetadata.prReviewerEligibility.decisions[0]).not.toHaveProperty('reason');
+ });
+
+ it('forces a model-positive decision false when programmatic issue facts do not qualify', async () => {
+ const task = eligibilityTask();
+ task.metadata.issueWatcher.pullRequests[0].eligibilityFacts = {
+ ...eligibleFacts,
+ openerAssignedIssueNumbers: [],
+ };
+
+ const result = await processTaskOutput({
+ appId: 'app-example',
+ success: true,
+ payload: decisionPayload(),
+ task,
+ });
+
+ expect(result).toMatchObject({ accepted: true, terminal: true });
+ expect(result.taskMetadata.prReviewerEligibility.eligibleNumbers).toEqual([]);
+ expect(result.taskMetadata.prReviewerEligibility.rejectedNumbers).toEqual([12]);
+ });
+
+ it('does not trust open or assigned issue IDs that are not linked to the PR', async () => {
+ const task = eligibilityTask();
+ task.metadata.issueWatcher.pullRequests[0].eligibilityFacts = {
+ ...eligibleFacts,
+ linkedIssueNumbers: [],
+ };
+
+ const result = await processTaskOutput({
+ appId: 'app-example',
+ success: true,
+ payload: decisionPayload(),
+ task,
+ });
+
+ expect(result).toMatchObject({ accepted: true, terminal: true });
+ expect(result.taskMetadata.prReviewerEligibility.eligibleNumbers).toEqual([]);
+ expect(result.taskMetadata.prReviewerEligibility.rejectedNumbers).toEqual([12]);
+ });
+
+ it('fails closed when the model omits an expected decision', async () => {
+ const task = eligibilityTask();
+ task.metadata.issueWatcher.pullRequests.push({
+ number: 13,
+ headSha: 'c'.repeat(40),
+ contentFingerprint: 'd'.repeat(64),
+ authorLogin: 'another-contributor',
+ eligibilityFacts: eligibleFacts,
+ });
+
+ const result = await processTaskOutput({
+ appId: 'app-example',
+ success: true,
+ payload: decisionPayload(),
+ task,
+ });
+
+ expect(result).toMatchObject({ accepted: false, reason: 'eligibility-response-incomplete' });
+ });
+});
+
+describe('pr-reviewer output routing', () => {
+ it('routes the final actions stage to the deterministic issue-watcher coordinator', async () => {
+ issueWatcherMock.processTaskOutput.mockResolvedValueOnce({ action: 'reviewed', accepted: true });
+ const task = {
+ metadata: {
+ pipeline: {
+ currentStage: 2,
+ stages: [
+ { role: 'security' },
+ { role: 'eligibility' },
+ { role: 'actions' },
+ ],
+ },
+ },
+ };
+ const args = { appId: 'app-example', success: true, payload: { pullRequests: [] }, task };
+
+ await expect(processTaskOutput(args, { execGh: vi.fn() }))
+ .resolves.toEqual({ action: 'reviewed', accepted: true });
+ expect(issueWatcherMock.processTaskOutput).toHaveBeenCalledWith({
+ ...args,
+ requireEligibilityFacts: true,
+ }, { execGh: expect.any(Function) });
+ });
+
+ it('recognizes both the binary gate envelope and the action envelope', () => {
+ expect(isEligibilityPayload(decisionPayload())).toBe(true);
+ expect(isTaskOutputPayload(decisionPayload())).toBe(true);
+ expect(isTaskOutputPayload({ issueComments: [], pullRequests: [] })).toBe(true);
+ expect(isEligibilityPayload({ decisions: [] })).toBe(false);
+ });
+});
diff --git a/server/services/prReviewerSecurity.js b/server/services/prReviewerSecurity.js
index b63a4ddfd2..5c04041188 100644
--- a/server/services/prReviewerSecurity.js
+++ b/server/services/prReviewerSecurity.js
@@ -13,6 +13,7 @@ import { execGh, ensureForgeReachable } from './github.js';
import { getSelfLogin } from './prWatcher.js';
import { getOriginInfo } from '../lib/gitRemote.js';
import { githubApiHost, githubRepoSpec } from '../lib/workTracker.js';
+import { mapWithConcurrency } from '../lib/mapWithConcurrency.js';
import {
MODEL_ABUSE_GUARD,
MODEL_ABUSE_GUARD_MAX_INPUT_CHARS,
@@ -24,15 +25,79 @@ import { safeJSONParse } from '../lib/fileUtils.js';
export const SECURITY_SCAN_MAX_OPEN_PRS = 200;
export const SECURITY_SCAN_MAX_DIFF_CHARS = MODEL_ABUSE_GUARD_MAX_INPUT_CHARS;
export const SECURITY_SCAN_MAX_REPORT_CHARS = 100_000;
+const MAX_LINKED_ISSUES = 50;
const failure = (code, extra = {}) => ({ ok: false, passed: false, code, ...extra });
const isHeadRefOid = (value) => typeof value === 'string' && /^[a-f0-9]{40}$/i.test(value);
const safeText = (value, max) => typeof value === 'string' ? value.slice(0, max) : '';
+const ELIGIBILITY_LOOKUP_CONCURRENCY = 4;
+
+function issueNumbersFromText(value, repoFullName) {
+ if (typeof value !== 'string' || !repoFullName) return [];
+ const repo = String(repoFullName).toLowerCase();
+ const numbers = new Set();
+ // GitHub's closing/reference syntax is the useful signal here. Limit
+ // repository-qualified references to this repository so a contributor's
+ // unrelated cross-repo issue cannot become an eligibility fact.
+ const referencePattern = /(?:\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?|relate[sd]?\s+to|ref(?:s)?|part\s+of)\s+)?(?:(?:[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+)\s*)?#(\d+)/gi;
+ let match;
+ while ((match = referencePattern.exec(value)) && numbers.size < MAX_LINKED_ISSUES) {
+ const qualified = match[0].match(/([A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+)\s*#\d+$/i)?.[1];
+ if (qualified && qualified.toLowerCase() !== repo) continue;
+ const number = Number(match[1]);
+ if (Number.isInteger(number) && number > 0) numbers.add(number);
+ }
+ return [...numbers].sort((a, b) => a - b);
+}
+
+export function extractLinkedIssueNumbers(pr, repoFullName) {
+ return [...new Set([
+ ...issueNumbersFromText(pr?.title, repoFullName),
+ ...issueNumbersFromText(pr?.body, repoFullName),
+ ])].sort((a, b) => a - b).slice(0, MAX_LINKED_ISSUES);
+}
+
+async function resolveEligibilityFacts(pr, repoFullName, hostname) {
+ const linkedIssueNumbers = extractLinkedIssueNumbers(pr, repoFullName);
+ if (linkedIssueNumbers.length === 0) {
+ return {
+ linkedIssueNumbers: [],
+ openLinkedIssueNumbers: [],
+ openerAssignedIssueNumbers: [],
+ issueLookupComplete: true,
+ };
+ }
+ const openLinkedIssueNumbers = [];
+ const openerAssignedIssueNumbers = [];
+ let issueLookupComplete = true;
+ for (const issueNumber of linkedIssueNumbers) {
+ const raw = await execGh([
+ 'api', '--hostname', hostname, `repos/${repoFullName}/issues/${issueNumber}`,
+ ]).catch(() => null);
+ const issue = safeJSONParse(raw, null);
+ if (!issue || issue.number !== issueNumber) {
+ issueLookupComplete = false;
+ continue;
+ }
+ const isIssue = !issue.pull_request;
+ if (isIssue && String(issue.state).toLowerCase() === 'open') {
+ openLinkedIssueNumbers.push(issueNumber);
+ const assignedLogins = Array.isArray(issue.assignees)
+ ? issue.assignees.map((assignee) => String(assignee?.login || '').toLowerCase()).filter(Boolean)
+ : [];
+ if (assignedLogins.includes(String(pr.authorLogin).toLowerCase())) {
+ openerAssignedIssueNumbers.push(issueNumber);
+ }
+ }
+ }
+ return {
+ linkedIssueNumbers,
+ openLinkedIssueNumbers,
+ openerAssignedIssueNumbers,
+ issueLookupComplete,
+ };
+}
-/**
- * Find every currently-open PR from an external human contributor. This is a
- * public-metadata operation only; it does not fetch a branch or run code.
- */
/**
* The three facts that decide which of a repo's open PRs pr-reviewer may target:
* the `gh` repo selector, the default branch it reviews against, and the login
@@ -64,6 +129,7 @@ export async function resolvePrReviewerTargetScope(app) {
ok: true,
repoSpec,
repoFullName: origin.fullName,
+ hostname: githubApiHost(origin.host),
defaultBranch: defaultBranch.trim(),
selfLogin,
};
@@ -87,7 +153,7 @@ export function isReviewablePullRequest(scope, pullRequest) {
export async function listExternalOpenPullRequests(app) {
const scope = await resolvePrReviewerTargetScope(app);
if (!scope.ok) return scope;
- const { repoSpec, repoFullName, defaultBranch, selfLogin } = scope;
+ const { repoSpec, repoFullName, hostname, defaultBranch, selfLogin } = scope;
const raw = await execGh([
'pr', 'list', '--repo', repoSpec,
@@ -101,7 +167,7 @@ export async function listExternalOpenPullRequests(app) {
if (!Array.isArray(parsed)) return failure('security-scan-pr-list-unreadable');
if (parsed.length >= SECURITY_SCAN_MAX_OPEN_PRS) return failure('security-scan-too-many-open-prs');
- const prs = parsed.map((pr) => ({
+ const listedPrs = parsed.map((pr) => ({
number: pr?.number,
authorLogin: pr?.author?.login,
headRefOid: isHeadRefOid(pr?.headRefOid) ? pr.headRefOid : null,
@@ -110,7 +176,7 @@ export async function listExternalOpenPullRequests(app) {
title: typeof pr?.title === 'string' ? pr.title : null,
body: typeof pr?.body === 'string' ? pr.body : '',
}));
- if (prs.some((pr) => (
+ if (listedPrs.some((pr) => (
!Number.isInteger(pr.number)
|| pr.number < 1
|| typeof pr.authorLogin !== 'string'
@@ -120,12 +186,18 @@ export async function listExternalOpenPullRequests(app) {
return failure('security-scan-pr-list-unreadable');
}
+ const externalPrs = listedPrs.filter((pr) => String(pr.authorLogin).toLowerCase() !== String(selfLogin).toLowerCase());
+ const prs = await mapWithConcurrency(externalPrs, ELIGIBILITY_LOOKUP_CONCURRENCY, async (pr) => ({
+ ...pr,
+ eligibilityFacts: await resolveEligibilityFacts(pr, repoFullName, hostname),
+ }));
+
return {
ok: true,
repoSpec,
repoFullName,
- defaultBranch,
- prs: prs.filter((pr) => String(pr.authorLogin).toLowerCase() !== String(selfLogin).toLowerCase()),
+ defaultBranch: defaultBranch.trim(),
+ prs,
};
}
@@ -143,6 +215,9 @@ export function securityScanFingerprint(target) {
prs: target.prs
.map((pr) => ({ number: pr.number, headRefOid: pr.headRefOid }))
.sort((a, b) => a.number - b.number),
+ eligibilityFacts: target.prs
+ .map((pr) => ({ number: pr.number, facts: pr.eligibilityFacts || null }))
+ .sort((a, b) => a.number - b.number),
};
return createHash('sha256').update(JSON.stringify(identity)).digest('hex');
}
@@ -230,6 +305,7 @@ export async function runPrReviewerSecurityScan({ app, timeoutMs, target = null
url: pr.url,
headSha: pr.headRefOid,
baseRefName: resolvedTarget.defaultBranch,
+ eligibilityFacts: pr.eligibilityFacts,
behindBy: null,
files: [],
additions: 0,
diff --git a/server/services/prReviewerSecurity.test.js b/server/services/prReviewerSecurity.test.js
index 44a12223b3..47bcbcb781 100644
--- a/server/services/prReviewerSecurity.test.js
+++ b/server/services/prReviewerSecurity.test.js
@@ -97,6 +97,52 @@ describe('pr-reviewer model-abuse preflight', () => {
])
})
+ it('records only current open issues assigned to the PR opener as eligibility facts', async () => {
+ execGhMock
+ .mockResolvedValueOnce('main')
+ .mockResolvedValueOnce(JSON.stringify([
+ listedPr(12, 'Contributor-A', 'b'.repeat(40), {
+ title: 'Fixes #101 and unrelated/repo#202',
+ body: 'Refs #101',
+ }),
+ ]))
+ .mockResolvedValueOnce(JSON.stringify({
+ number: 101,
+ state: 'open',
+ assignees: [{ login: 'contributor-a' }],
+ }))
+
+ const result = await listExternalOpenPullRequests(app)
+
+ expect(result.prs[0].eligibilityFacts).toEqual({
+ linkedIssueNumbers: [101],
+ openLinkedIssueNumbers: [101],
+ openerAssignedIssueNumbers: [101],
+ issueLookupComplete: true,
+ })
+ expect(execGhMock).toHaveBeenLastCalledWith([
+ 'api', '--hostname', 'github.com', 'repos/example/repo/issues/101',
+ ])
+ })
+
+ it('fails the programmatic issue lookup fact closed when a linked issue cannot be read', async () => {
+ execGhMock
+ .mockResolvedValueOnce('main')
+ .mockResolvedValueOnce(JSON.stringify([
+ listedPr(12, 'Contributor-A', 'b'.repeat(40), { title: 'Fixes #101' }),
+ ]))
+ .mockRejectedValueOnce(new Error('forge unavailable'))
+
+ const result = await listExternalOpenPullRequests(app)
+
+ expect(result.prs[0].eligibilityFacts).toEqual({
+ linkedIssueNumbers: [101],
+ openLinkedIssueNumbers: [],
+ openerAssignedIssueNumbers: [],
+ issueLookupComplete: false,
+ })
+ })
+
it('keys a pending report to the exact external PR head set', () => {
const base = {
ok: true,
diff --git a/server/services/publicReviewProviderSelection.js b/server/services/publicReviewProviderSelection.js
new file mode 100644
index 0000000000..5b58d618fd
--- /dev/null
+++ b/server/services/publicReviewProviderSelection.js
@@ -0,0 +1,88 @@
+/**
+ * Public-review provider selection.
+ *
+ * A pr-reviewer stage declares the POSTURE it needs (`no-tool` for the
+ * screening/eligibility stages, `sandboxed-actions` for the optional final
+ * review). It never names a vendor. This module turns that posture into a
+ * concrete provider by intersecting the install's own enabled AI providers
+ * with the vendor rows that declare a maintained recipe for the posture.
+ *
+ * That indirection is the point: an install with only grok, only a local
+ * Claude wrapper, or only codex configures the same three stages. Adding a
+ * vendor recipe in `providerVendors.js` makes it selectable everywhere at once
+ * — here, in the schedule UI, and at spawn time — with no list of vendor names
+ * to keep in sync.
+ *
+ * Ordinary provider fallback is deliberately NOT reused: falling back off an
+ * eligible provider onto an ineligible one would run untrusted public content
+ * through a provider with no enforced posture. A stage with no eligible
+ * provider fails closed instead.
+ */
+
+import { getAllProviders } from './providers.js';
+import { isProviderAvailable } from './providerStatus.js';
+import {
+ publicReviewCapableVendorIds,
+ publicReviewPostureForProfile,
+ supportsPublicReviewPosture,
+} from '../lib/providerVendors.js';
+
+/** The posture a task's execution profile requires, or null for a normal task. */
+export function publicReviewPostureForTask(task) {
+ return publicReviewPostureForProfile(task?.metadata?.executionProfile);
+}
+
+const isEnabled = (provider) => provider?.enabled !== false;
+
+/**
+ * Every configured provider that could run `posture` on this install, in the
+ * provider list's own order. `enabled: false` providers are excluded — the
+ * user has switched them off — but momentarily-unavailable ones are kept so a
+ * rate-limited provider still shows as a legal choice in the picker.
+ *
+ * @param {string} posture
+ * @param {{ providers?: object[] }} [options]
+ * @returns {Promise