chore: capture current state and polish GitHub surface - #89
Conversation
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Compatibility | 2 medium |
| BestPractice | 11 medium |
| ErrorProne | 2 high |
| Security | 11 critical 5 high |
| Complexity | 69 medium |
🟢 Metrics 1524 complexity · -8 duplication
Metric Results Complexity 1524 Duplication -8
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
Retain both repository histories and resolve the integration to the reviewed polished tree.
There was a problem hiding this comment.
Pull Request Overview
This PR is exceptionally large (3,400+ lines), consolidating a significant amount of 'state-capture' work into the repository. This broad scope increases regression risk across multiple modules, including the new experimental platform and Workflow 2.2 implementation. Codacy analysis reports that the PR is currently not up to standards, with 107 new issues introduced.
Critical functional issues exist in the remote relay implementation; hardcoded 15-second timeouts will terminate Server-Sent Event (SSE) streams, rendering the operator UI's remote mode unstable. Additionally, the macOS-specific Seatbelt sandbox rules for OpenCode incorrectly block .git directory access, which will prevent the verification broker from executing its intended Git-based checks. Several complex files related to core workflow scheduling and the OpenCode adapter lack test coverage, representing a significant maintenance risk. Specifically, opencode-adapter.mjs and the workflow logic files (workflow-scheduler-v22.mjs, workflow-context-v22.mjs) combine high cyclomatic complexity with a lack of automated validation.
About this PR
- The PR is exceptionally broad and large (over 3,400 lines), which significantly complicates review and increases the risk of regression in unrelated components.
- The OpenCode execution route and verification broker are strictly limited to macOS Darwin, creating platform-specific implementation forks that may limit future scalability.
Test suggestions
- OpenCode adapter executes runs using explicit JSON protocol and normalizes event streams on macOS
- Verification broker resolves approved commands and executes them under a no-network Seatbelt sandbox
- Workflow v2.2 scheduler manages durable waits and reduces signals with idempotency
- Execution profile v3 correctly validates and maps logical tiers to named Codex or OpenCode routes
- Operator console remote mode safely relays API requests and upstream tokens to a separately hosted API
- Platform store implementation handles transactional run creation and enforces the 256 KiB envelope limit
- Unit test for renderInspector logic in workflows.js
- Unit test for ordinaryEnvelope mapping in workflow-scheduler-v22.mjs
- Unit test for assembleWorkflowContextV22 policy enforcement in workflow-context-v22.mjs
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Unit test for renderInspector logic in workflows.js
2. Unit test for ordinaryEnvelope mapping in workflow-scheduler-v22.mjs
3. Unit test for assembleWorkflowContextV22 policy enforcement in workflow-context-v22.mjs
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| }, | ||
| body: body?.length ? body : undefined, | ||
| redirect: "manual", | ||
| signal: AbortSignal.timeout(15_000), |
There was a problem hiding this comment.
🔴 HIGH RISK
The 15-second AbortSignal timeout will terminate proxied event streams prematurely. Use a separate signal for the initial connection/headers and allow the stream body to remain open, or detect /events/stream and omit the timeout.
Try running the following prompt in your coding agent:
In
packages/orchestration/operator/lib/remote.mjs, modify theforwardfunction to use a different timeout strategy for/events/streamroutes. The currentAbortSignal.timeout(15_000)aborts the entire stream consumption after 15 seconds.
| await reader.cancel().catch(() => {}); | ||
| res.end(); | ||
| }; | ||
| const timeout = setTimeout(() => void finish(), 15_000); |
There was a problem hiding this comment.
🔴 HIGH RISK
This 15-second hard timeout terminates event streams (like /events/stream) prematurely. For streaming routes, the timeout should be disabled or significantly increased to support long-lived connections.
Try running the following prompt in your coding agent:
Update
sendRemoteResponseinpackages/orchestration/operator/server.mjsto only apply the 15-second timeout if the response is not an event stream (SSE), as streaming routes need to remain open for long periods.
| export function assembleWorkflowContextV22({ | ||
| task = "", | ||
| node = { id: "unknown", guidance: "" }, | ||
| item = null, | ||
| inputs = [], | ||
| verifiedGraphRecords = [], | ||
| admittedMemory = [], | ||
| contextPolicy = {}, | ||
| capBytes = DEFAULT_CONTEXT_CAP_BYTES, | ||
| }) { | ||
| assertCap(capBytes); | ||
| const selected = []; | ||
| const omitted = []; | ||
| const mandatory = [ | ||
| ["task", { trust_class: "operator" }, { task }], | ||
| ["node-guidance", { trust_class: "workflow" }, { node_id: node.id, guidance: node.guidance }], | ||
| ["mapped-item", { trust_class: "workflow" }, { item }], | ||
| ...[...inputs] | ||
| .sort((left, right) => | ||
| String(left.envelope.instance_id ?? left.envelope.node_id).localeCompare( | ||
| String(right.envelope.instance_id ?? right.envelope.node_id), | ||
| ), | ||
| ) | ||
| .map((entry) => ["predecessor", entry, predecessor(entry)]), | ||
| ]; | ||
| for (const [source, entry, value] of mandatory) | ||
| addItem({ | ||
| selected, | ||
| omitted, | ||
| capBytes, | ||
| item: entry, | ||
| source, | ||
| mandatory: true, | ||
| value, | ||
| canReference: source === "predecessor", | ||
| }); | ||
| const optionalBudget = Math.min(Number(contextPolicy.optional_budget_bytes ?? 0), capBytes); | ||
| const optional = []; | ||
| if ( | ||
| contextPolicy.allow_operational_evidence === true && | ||
| node.context?.include_operational_evidence === true | ||
| ) { | ||
| optional.push(...inputs.map((entry) => ["operational", entry, operational(entry)])); | ||
| } else if (node.context?.include_operational_evidence === true) { | ||
| omitted.push( | ||
| ...inputs.map((entry) => ({ | ||
| source: "operational", | ||
| source_digest: digest(operational(entry)), | ||
| bytes: bytes(operational(entry)), | ||
| reason: "policy-denied", | ||
| trust_class: "local", | ||
| })), | ||
| ); | ||
| } | ||
| if (contextPolicy.allow_verified_graph === true) | ||
| optional.push(...verifiedGraphRecords.map((entry) => ["verified-graph", entry, entry])); | ||
| else | ||
| omitted.push( | ||
| ...verifiedGraphRecords.map((entry) => ({ | ||
| source: "verified-graph", | ||
| source_digest: digest(entry), | ||
| bytes: bytes(entry), | ||
| reason: "policy-denied", | ||
| trust_class: entry.trust_class ?? "advisory", | ||
| })), | ||
| ); | ||
| if (contextPolicy.allow_admitted_memory === true) | ||
| optional.push(...admittedMemory.map((entry) => ["admitted-memory", entry, entry])); | ||
| else | ||
| omitted.push( | ||
| ...admittedMemory.map((entry) => ({ | ||
| source: "admitted-memory", | ||
| source_digest: digest(entry), | ||
| bytes: bytes(entry), | ||
| reason: "policy-denied", | ||
| trust_class: entry.trust_class ?? "advisory", | ||
| })), | ||
| ); | ||
| let optionalUsed = 0; | ||
| for (const [source, entry, value] of optional) { | ||
| const entryBytes = bytes(value); | ||
| if (optionalUsed + entryBytes > optionalBudget) { | ||
| omitted.push({ | ||
| source, | ||
| source_digest: digest(value), | ||
| bytes: entryBytes, | ||
| reason: "optional-budget", | ||
| trust_class: entry.trust_class ?? "advisory", | ||
| }); | ||
| continue; | ||
| } | ||
| const before = selected.length; | ||
| addItem({ | ||
| selected, | ||
| omitted, | ||
| capBytes, | ||
| item: entry, | ||
| source, | ||
| mandatory: false, | ||
| value, | ||
| summary: entry.summary ?? null, | ||
| }); | ||
| if (selected.length > before) optionalUsed += entryBytes; | ||
| } | ||
| const assembledBytes = bytes({ items: selected }); | ||
| const included = selected.map((entry) => ({ | ||
| source: entry.source, | ||
| source_digest: entry.source_digest, | ||
| bytes: bytes(entry.value ?? entry.reference), | ||
| reason: entry.reference ? "artifact-reference" : "inline", | ||
| trust_class: entry.value?.trust_class ?? "local", | ||
| })); | ||
| const manifest = { | ||
| cap_bytes: capBytes, | ||
| assembled_bytes: assembledBytes, | ||
| mandatory_budget_bytes: capBytes, | ||
| optional_budget_bytes: optionalBudget, | ||
| included, | ||
| omitted, | ||
| artifact_refs: selected.filter((entry) => entry.reference).map((entry) => entry.reference), | ||
| inline_artifacts: selected | ||
| .filter((entry) => entry.value) | ||
| .map(({ source, source_digest }) => ({ source, source_digest })), | ||
| }; | ||
| return Object.freeze({ | ||
| manifest: Object.freeze(manifest), | ||
| prompt_context: Object.freeze({ items: selected }), | ||
| digest: digest(manifest), | ||
| }); |
There was a problem hiding this comment.
🟡 MEDIUM RISK
The assembleWorkflowContextV22 function contains repetitive branching logic to check for policy permission and remaining budget. This makes the assembly sequence harder to audit for safety gaps. Note: This complex file currently lacks test coverage.
Try running the following prompt in your IDE agent:
The
assembleWorkflowContextV22function inpackages/orchestration/scripts/pipeline/lib/workflow-context-v22.mjshas a complexity of 20. Refactor it to use a configuration-driven approach where context sources are processed by a common pipeline.
| function ordinaryEnvelope({ runId, workflowHash, node, inputs, attempt, result, context }) { | ||
| const payload = result?.payload ?? result ?? {}; | ||
| const output = { | ||
| payload, | ||
| findings: result?.findings ?? payload.findings ?? [], | ||
| evidence_refs: result?.evidence_refs ?? [], | ||
| ownership: result?.ownership ?? {}, | ||
| changed_paths: result?.changed_paths ?? [], | ||
| command_evidence: result?.command_evidence ?? [], | ||
| resource_usage: result?.resource_usage ?? {}, | ||
| }; | ||
| return freezeEnvelope({ | ||
| run_id: runId, | ||
| workflow_digest: workflowHash, | ||
| node_id: node.id, | ||
| instance_id: node.id, | ||
| attempt, | ||
| status: result?.status ?? "passed", | ||
| payload, | ||
| ...output, | ||
| input_digest: digest(inputs.map(({ envelope }) => envelope.output_digest)), | ||
| output_digest: digest(output), | ||
| execution_tier: result?.execution_tier ?? node.tier ?? "runtime", | ||
| context_manifest: context.manifest, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🟡 MEDIUM RISK
The ordinaryEnvelope function's logic for defaulting and extracting properties is brittle. As this function generates the immutable record of execution, its clarity is critical for auditability. Note: This complex file currently lacks test coverage.
Try running the following prompt in your IDE agent:
The
ordinaryEnvelopefunction inpackages/orchestration/scripts/pipeline/lib/workflow-scheduler-v22.mjsis a complex mapper (complexity 25). Refactor it to use declarative mapping or sub-mappers for each logical group of fields.
| function renderInspector(definition) { | ||
| const node = selectedNode(definition); | ||
| const controls = { | ||
| "workflow-node-guidance": node?.guidance ?? "", | ||
| "workflow-node-role": node?.role ?? "", | ||
| "workflow-node-kind": node?.kind ?? "agent", | ||
| "workflow-node-access": node?.access ?? "read", | ||
| "workflow-node-tier": node?.tier ?? "", | ||
| "workflow-node-payload": node?.payload_contract ?? "", | ||
| "workflow-node-join": node?.join ?? "", | ||
| "workflow-node-quorum": node?.quorum?.threshold ?? "", | ||
| "workflow-node-failure": node?.failure_handling?.mode ?? "", | ||
| "workflow-node-resource": node?.resource ?? "", | ||
| "workflow-node-loop-mode": node?.loop?.mode ?? "bounded", | ||
| "workflow-node-loop-bound": node?.loop?.max_iterations ?? 3, | ||
| "workflow-node-loop-members": (node?.loop?.members ?? []).join(", "), | ||
| }; | ||
| for (const [id, value] of Object.entries(controls)) elements[id].value = value; | ||
| elements["workflow-node-verification"].checked = node?.verification === true; | ||
| elements["workflow-node-checkpoint"].checked = node?.mutation_checkpoint === true; | ||
| elements["workflow-node-ownership"].checked = node?.ownership_plan === true; | ||
| elements["workflow-inspector-help"].textContent = node | ||
| ? `Editing ${node.id}. Use Delete to remove the selected node or edge, and connect two selected nodes in the structured list.` | ||
| : "Add a node to begin structured workflow authoring."; | ||
| } |
There was a problem hiding this comment.
🟡 MEDIUM RISK
The renderInspector function is overly complex due to the manual synchronization of a large set of form fields and node properties. This makes the UI-to-data mapping logic difficult to follow. Note: This complex file currently lacks test coverage.
Try running the following prompt in your IDE agent:
The
renderInspectorfunction inpackages/orchestration/operator/static/js/workflows.jsis overly complex (complexity 29). Refactor it by extracting the logic for rendering different categories of controls into smaller, specialized helper functions.
| export function runControlCommand(command, options) { | ||
| const context = controlCommandContext(options); | ||
| switch (command) { | ||
| case "status": | ||
| return reportControlStatus(context, options); | ||
| case "stop": | ||
| return requestControlStop(context, options); | ||
| case "resolve-checkpoint": | ||
| return resolveControlCheckpoint(context, options); | ||
| case "events": | ||
| return reportControlEvents(context, options); | ||
| default: | ||
| throw new Error(`unsupported control command: ${command}`); | ||
| } | ||
| } | ||
|
|
||
| function reportControlStatus(context, options) { | ||
| const runDir = getRunDir(context.runId, context.workspaceRoot); | ||
| emitReadableControlResult( | ||
| context, | ||
| { | ||
| schema_version: "1.0.0", | ||
| run_id: context.runId, | ||
| workspace_root: context.workspaceRoot, | ||
| active_lock: existsSync(resolve(runDir, "autonomous.lock")), | ||
| completed_gates: context.state.completed_gates ?? [], | ||
| operator_control: readOperatorControl(context.runId, context.workspaceRoot), | ||
| checkpoints: listCheckpoints(context.runId, context.workspaceRoot), | ||
| }, | ||
| options, | ||
| ); | ||
| } | ||
|
|
||
| function requestControlStop(context, options) { | ||
| const previous = readOperatorControl(context.runId, context.workspaceRoot); | ||
| const control = requestStop(context.runId, context.workspaceRoot); | ||
| if (!["stop-requested", "stopped"].includes(previous.status)) { | ||
| if (command === "status") { | ||
| const runDir = getRunDir(context.runId, context.workspaceRoot); | ||
| emitReadableControlResult( | ||
| context, | ||
| { | ||
| schema_version: "1.0.0", | ||
| run_id: context.runId, | ||
| workspace_root: context.workspaceRoot, | ||
| active_lock: existsSync(resolve(runDir, "autonomous.lock")), | ||
| completed_gates: context.state.completed_gates ?? [], | ||
| operator_control: readOperatorControl(context.runId, context.workspaceRoot), | ||
| checkpoints: listCheckpoints(context.runId, context.workspaceRoot), | ||
| }, | ||
| options, | ||
| ); | ||
| return; | ||
| } | ||
| if (command === "stop") { | ||
| const previous = readOperatorControl(context.runId, context.workspaceRoot); | ||
| const control = requestStop(context.runId, context.workspaceRoot); | ||
| if (!["stop-requested", "stopped"].includes(previous.status)) { | ||
| appendTraceEvent( | ||
| context.runId, | ||
| { event: "run_stop_requested", phase: nextRunPhase(context.state), status: "ok" }, | ||
| context.workspaceRoot, | ||
| ); | ||
| } | ||
| emitReadableControlResult( | ||
| context, | ||
| { success: true, run_id: context.runId, operator_control: control }, | ||
| options, | ||
| ); | ||
| return; | ||
| } | ||
| if (command === "signal") { | ||
| for (const key of ["node-id", "signal", "idempotency-key"]) { | ||
| if (!options[key]) throw new Error(`signal requires --${key}`); | ||
| } | ||
| const request = readJsonStrict( | ||
| resolve(getRunDir(context.runId, context.workspaceRoot), "request.json"), | ||
| ); | ||
| const workflow = request.workflow?.snapshot; | ||
| if (workflow?.schema_version !== "2.2.0") { | ||
| throw new Error("signal is available only for a workflow schema 2.2.0 run"); | ||
| } | ||
| const node = workflow.nodes.find((candidate) => candidate.id === options["node-id"]); | ||
| if (!node || node.kind !== "wait") throw new Error("--node-id must name a v2.2 wait node"); | ||
| if (!node.wait.signals.includes(options.signal)) { | ||
| throw new Error(`wait ${node.id} does not accept signal ${options.signal}`); | ||
| } | ||
| let payload = null; | ||
| if (options["payload-json"]) { | ||
| try { | ||
| payload = JSON.parse(options["payload-json"]); | ||
| } catch { | ||
| throw new Error("--payload-json must be valid JSON"); | ||
| } | ||
| } | ||
| const validateSignal = new Ajv2020({ allErrors: true, strict: false }).compile( | ||
| workflow.signal_contracts[node.wait.signal_contract], | ||
| ); | ||
| if (!validateSignal(payload)) { | ||
| const detail = validateSignal.errors | ||
| .map((error) => `${error.instancePath || "/"} ${error.message}`) | ||
| .join("; "); | ||
| throw new Error(`signal payload does not match ${node.wait.signal_contract}: ${detail}`); | ||
| } | ||
| const state = recordWorkflowV22Signal({ | ||
| runDir: getRunDir(context.runId, context.workspaceRoot), | ||
| runId: context.runId, | ||
| workflowDigest: request.workflow.digest, | ||
| nodeId: node.id, | ||
| signal: options.signal, | ||
| idempotencyKey: options["idempotency-key"], | ||
| payload, | ||
| }); | ||
| appendTraceEvent( | ||
| context.runId, | ||
| { event: "run_stop_requested", phase: nextRunPhase(context.state), status: "ok" }, | ||
| { | ||
| event: "workflow_signal_recorded", | ||
| phase: node.id, | ||
| status: "ok", | ||
| metadata: { signal: options.signal }, | ||
| }, | ||
| context.workspaceRoot, | ||
| ); | ||
| emitReadableControlResult( | ||
| context, | ||
| { success: true, run_id: context.runId, node_id: node.id, signal: options.signal, state }, | ||
| options, | ||
| ); | ||
| return; | ||
| } | ||
| emitReadableControlResult( | ||
| context, | ||
| { success: true, run_id: context.runId, operator_control: control }, | ||
| options, | ||
| ); | ||
| } | ||
|
|
||
| function requiredCheckpointDecision(options) { | ||
| const decision = options.decision; | ||
| if (!["approved", "rejected", "escalated"].includes(decision)) | ||
| throw new Error("--decision must be approved, rejected, or escalated"); | ||
| for (const key of ["checkpoint-id", "decision-id", "actor", "rationale"]) | ||
| if (!options[key]) throw new Error(`resolve-checkpoint requires --${key}`); | ||
| return decision; | ||
| } | ||
|
|
||
| function resolveControlCheckpoint(context, options) { | ||
| const decision = requiredCheckpointDecision(options); | ||
| const checkpoint = resolveCheckpointById( | ||
| context.runId, | ||
| options["checkpoint-id"], | ||
| { | ||
| status: decision, | ||
| decisionId: options["decision-id"], | ||
| actor: options.actor, | ||
| rationale: options.rationale, | ||
| }, | ||
| context.workspaceRoot, | ||
| ); | ||
| appendTraceEvent( | ||
| context.runId, | ||
| { | ||
| event: "checkpoint_resolved", | ||
| phase: checkpoint.phase, | ||
| status: decision === "approved" ? "ok" : "blocked", | ||
| metadata: { checkpoint_id: checkpoint.checkpoint_id, outcome: decision }, | ||
| }, | ||
| context.workspaceRoot, | ||
| ); | ||
| if (decision !== "approved") | ||
| if (command === "resolve-checkpoint") { | ||
| const decision = options.decision; | ||
| if (!["approved", "rejected", "escalated"].includes(decision)) { | ||
| throw new Error("--decision must be approved, rejected, or escalated"); | ||
| } | ||
| for (const key of ["checkpoint-id", "decision-id", "actor", "rationale"]) { | ||
| if (!options[key]) throw new Error(`resolve-checkpoint requires --${key}`); | ||
| } | ||
| const checkpoint = resolveCheckpointById( | ||
| context.runId, | ||
| options["checkpoint-id"], | ||
| { | ||
| status: decision, | ||
| decisionId: options["decision-id"], | ||
| actor: options.actor, | ||
| rationale: options.rationale, | ||
| }, | ||
| context.workspaceRoot, | ||
| ); | ||
| appendTraceEvent( | ||
| context.runId, | ||
| { event: "run_blocked", phase: checkpoint.phase, status: "blocked" }, | ||
| { | ||
| event: "checkpoint_resolved", | ||
| phase: checkpoint.phase, | ||
| status: decision === "approved" ? "ok" : "blocked", | ||
| metadata: { checkpoint_id: checkpoint.checkpoint_id, outcome: decision }, | ||
| }, | ||
| context.workspaceRoot, | ||
| ); | ||
| emitReadableControlResult(context, { success: true, run_id: context.runId, checkpoint }, options); | ||
| } | ||
|
|
||
| function validEventRange(options) { | ||
| const afterSeq = Number(options["after-seq"] ?? 0); | ||
| const limit = Number(options.limit ?? 100); | ||
| assertEventRange(afterSeq, limit); | ||
| return { afterSeq, limit }; | ||
| } | ||
|
|
||
| function assertEventRange(afterSeq, limit) { | ||
| if (!validAfterSequence(afterSeq)) throw new Error("--after-seq must be a non-negative integer"); | ||
| if (!validEventLimit(limit)) throw new Error("--limit must be an integer between 1 and 1000"); | ||
| } | ||
|
|
||
| function validAfterSequence(value) { | ||
| return Number.isInteger(value) && value >= 0; | ||
| } | ||
| function validEventLimit(value) { | ||
| return Number.isInteger(value) && value >= 1 && value <= 1000; | ||
| } | ||
|
|
||
| function reportControlEvents(context, options) { | ||
| const { afterSeq, limit } = validEventRange(options); | ||
| const all = projectOperatorEvents(context.runId, context.workspaceRoot).filter( | ||
| (event) => event.seq > afterSeq, | ||
| ); | ||
| const events = all.slice(0, limit); | ||
| emitReadableControlResult( | ||
| context, | ||
| { | ||
| schema_version: "1.0.0", | ||
| run_id: context.runId, | ||
| after_seq: afterSeq, | ||
| next_after_seq: events.at(-1)?.seq ?? afterSeq, | ||
| has_more: all.length > events.length, | ||
| events, | ||
| }, | ||
| { ...options, json: true }, | ||
| ); | ||
| if (decision !== "approved") { | ||
| appendTraceEvent( | ||
| context.runId, | ||
| { event: "run_blocked", phase: checkpoint.phase, status: "blocked" }, | ||
| context.workspaceRoot, | ||
| ); | ||
| } | ||
| emitReadableControlResult( | ||
| context, | ||
| { success: true, run_id: context.runId, checkpoint }, | ||
| options, | ||
| ); | ||
| return; | ||
| } | ||
| if (command === "events") { | ||
| const afterSeq = Number(options["after-seq"] ?? 0); | ||
| const limit = Number(options.limit ?? 100); | ||
| if (!Number.isInteger(afterSeq) || afterSeq < 0) { | ||
| throw new Error("--after-seq must be a non-negative integer"); | ||
| } | ||
| if (!Number.isInteger(limit) || limit < 1 || limit > 1000) { | ||
| throw new Error("--limit must be an integer between 1 and 1000"); | ||
| } | ||
| const all = projectOperatorEvents(context.runId, context.workspaceRoot).filter( | ||
| (event) => event.seq > afterSeq, | ||
| ); | ||
| const events = all.slice(0, limit); | ||
| emitReadableControlResult( | ||
| context, | ||
| { | ||
| schema_version: "1.0.0", | ||
| run_id: context.runId, | ||
| after_seq: afterSeq, | ||
| next_after_seq: events.at(-1)?.seq ?? afterSeq, | ||
| has_more: all.length > events.length, | ||
| events, | ||
| }, | ||
| { ...options, json: true }, | ||
| ); | ||
| return; | ||
| } | ||
| throw new Error(`unsupported control command: ${command}`); |
There was a problem hiding this comment.
🟡 MEDIUM RISK
The runControlCommand function uses a long if-else chain to handle five different logic-heavy commands. This increases the risk of side effects when adding new functionality and makes it difficult to test individual command handlers in isolation.
Try running the following prompt in your IDE agent:
The
runControlCommandfunction inpackages/orchestration/scripts/pipeline/lib/autonomous-actions.mjshas a cyclomatic complexity of 35. Refactor this function to use a command mapping object where each command is handled by a separate private function.
| @@ -0,0 +1,693 @@ | |||
| /** Runs OpenCode behind RAE's macOS containment and normalized-event boundary. */ | |||
There was a problem hiding this comment.
🟡 MEDIUM RISK
The opencode-adapter.mjs file is becoming a 'God object' for the OpenCode integration. Mixing low-level OS containment logic with high-level event parsing increases the risk of side effects during maintenance. Note: This complex file currently lacks test coverage.
Try running the following prompt in your IDE agent:
Refactor
packages/orchestration/scripts/pipeline/lib/opencode-adapter.mjsby extracting the macOS Seatbelt profile generation into a separate module (e.g.,lib/sandbox/seatbelt.mjs) and the event stream parsing into another (e.g.,lib/opencode/events.mjs).
| ]) { | ||
| if (protectedPath) { | ||
| const escaped = escapeSeatbelt(protectedPath); | ||
| rules.push(`(deny file-read* file-write* (literal "${escaped}") (subpath "${escaped}"))`); |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Denying read access to the .git directory prevents the verification broker from executing Git-based checks. Consider allowing file-read* for Git metadata while maintaining the file-write* denial to ensure verifiers can function correctly.
| rules.push(`(deny file-read* file-write* (literal "${escaped}") (subpath "${escaped}"))`); | |
| rules.push(`(deny file-write* (literal "${escaped}") (subpath "${escaped}"))`); |
Summary
Verification
git diff --checkReview note
This is intentionally a broad state-capture PR. Review the commits separately: product-state capture first, repository polish second.