Skip to content

[T-A355AA] Autopilot mode#60

Open
weekly-daniel wants to merge 11 commits into
mainfrom
feature/t-a355aa-autopilot-mode
Open

[T-A355AA] Autopilot mode#60
weekly-daniel wants to merge 11 commits into
mainfrom
feature/t-a355aa-autopilot-mode

Conversation

@weekly-daniel

Copy link
Copy Markdown
Collaborator

Summary

Add an Autopilot mode with a Supervisor Agent that auto-approves plans and makes intelligent retry decisions on review failures, reducing human intervention in the task pipeline.

Key Changes

  • server/src/supervisor.js (NEW — supervisor evaluation logic using claude --print for plan approval and review failure analysis) - server/src/config.js (add autopilotMode setting to defaults, normalization, and validation)
  • server/src/orchestrator.js (integrate supervisor at plan-complete and review-failure decision points) - client/src/App.jsx (add autopilot mode selector to Settings modal General tab)
  • server/src/supervisor.test.js (NEW — unit tests for supervisor module) - server/src/config.test.js (add tests for autopilotMode setting)
  • server/src/orchestrator.test.js (add tests for supervisor integration paths)

Review

  • Verdict: N/A
  • Summary: Changed files: server/src/supervisor.js (new), server/src/supervisor.test.js (new), server/src/supervisor-integration.test.js (new), server/src/orchestrator.js, server/src/orchestrator.test.js, server/src/config.js, server/src/config.test.js, server/src/index.js, client/src/App.jsx, client/src/useFactory.js, plus lock file churn. The feature is well-architected: the supervisor module is clean with fail-safe-to-ESCALATE semantics, decision parsing is robust with multi-line feedback support, config validation is thorough, the UI toggle is simple, and the orchestrator integration includes race condition guards and a hard cap on cycle extensions. The main gaps are silent catch blocks and missing orchestrator-level integration tests for the supervisor paths. === REVIEW END ===
  • Minor issues: supervisor.js:62-64 — Potential double-resolve in runSupervisorQuery (confidence: 80): If execFile fails to spawn the process, both the execFile callback (with err) and the child.on('error') handler fire, calling resolve() twice. Promise silently ignores the second call so it won't crash, but it's a latent bug pattern. Fix: Track res lution s ate with a let resolved = fals guard, or remove the child.on('error') handler since execFile's callback alr ady handles all error paths (spawn fa lure, imeout exit code). - orchestrator.js:906 — Silent .catch(() => {}) swallows errors without logging (confidence: 85): The plan evaluation catch block and the two review-failure catch blocks (lines 906, 1137, 1195 approx.) silently swallow errors. The fail-safe behavior is correct (leave in awaiting_approval / block / retry with raw issu s), but there's zero observability in o why the supervisor all failed. Fix: Add console.error('Supervisor evaluation failed:', err.message) in each cat h block. - orchestrator.test.js — No integration tests for supervisor decision paths in orchestrator (confidence: 78): The orchestrator test only asserts MAX_SUPERVISOR_EXTENSIONS is a constant. There are no tests for the actual orchestrator behavior: plan auto-approval triggering, review failure supervisor routing, rac ondition guard, or the hard-cap exten ion logic. The supervisor-integration.test.js tests th supervi or module in isol ti n but no how the chestrator consumes i . Fix: Add orch str tor-level tests that mock supervisor.js expo ts and config.js's loadSettings to verify: (1) onPla Complete calls evaluatePlan when mode != manual, (2) review failure paths r ute th ough supervi or in autopilot, (3) the ext nsion hard c p blocks after MAX_SUPERVISOR_EXTENSIONS.

AI Factory added 4 commits March 20, 2026 19:59
Add three execution modes (manual/hybrid/autopilot) that control how
much human intervention the pipeline requires:

- manual: current behavior, human approves all plans
- hybrid: AI supervisor auto-approves/rejects plans, standard auto-retry
  on review failures
- autopilot: fully automated with supervisor making intelligent retry
  decisions on review failures, extending review cycles when needed,
  and escalating only when stuck

New files:
- server/src/supervisor.js: supervisor evaluation logic using CLI --print
  mode for plan approval and review failure analysis
- server/src/supervisor.test.js: unit tests for decision parsing

Modified:
- config.js: autopilotMode setting with defaults, normalization, validation
- orchestrator.js: supervisor integration at plan-complete and review-failure
  decision points with race condition guards
- index.js: broadcast supervisor:decision events via WebSocket
- App.jsx: segmented button group for execution mode in Settings General tab
- useFactory.js: handle SUPERVISOR_DECISION WebSocket messages

🤖 Ban Kan Autopilot
…sionBlock

Replace regex-based feedback extraction with slice-based approach to capture
all lines after the FEEDBACK/ENHANCED_FEEDBACK label. The previous non-greedy
regex with $ in multiline mode only captured the first line.

🧑🏻‍💻 Weekly Daniel
Add decision validation in supervisor.js to ensure parsed decisions are
one of the expected values (APPROVE/REJECT/ESCALATE for plans,
RETRY/ESCALATE for reviews). Invalid decisions fall back to ESCALATE
as a fail-safe. Add comprehensive integration tests covering supervisor
plan approval, review failure paths, and result structure contracts.

🧑🏻‍💻 Weekly Daniel
Prevent unbounded cycle extensions when autopilot supervisor keeps
returning RETRY at max review cycles. Caps extensions at 2 beyond
the configured maxReviewCycles, then blocks the task for human input.

🧑🏻‍💻 Weekly Daniel

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an Autopilot execution mode to the task pipeline by introducing a Supervisor agent that can auto-approve plans and guide retries/escalations on review failures, with corresponding server config + UI controls.

Changes:

  • Add server/src/supervisor.js (and tests) to query a CLI model and parse structured supervisor decisions.
  • Integrate supervisor decision points into server/src/orchestrator.js for plan completion and review-failure handling (including a hard cap on cycle extensions).
  • Add autopilotMode to settings defaults/normalization/validation and expose it in the Settings modal; broadcast supervisor decisions to the client.

Reviewed changes

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

Show a summary per file
File Description
server/src/supervisor.js New supervisor query + decision parsing/validation; emits fail-safe ESCALATE outcomes.
server/src/supervisor.test.js Unit tests for decision parsing and validation helpers.
server/src/supervisor-integration.test.js Integration tests around evaluatePlan/evaluateReviewFailure with mocked execFile.
server/src/orchestrator.js Hooks supervisor into plan auto-approval (autopilot/hybrid) and review-failure retry/escalate logic (autopilot).
server/src/orchestrator.test.js Adds basic assertion for MAX_SUPERVISOR_EXTENSIONS export/default.
server/src/config.js Adds autopilotMode default + normalization + validation.
server/src/config.test.js Adds tests for autopilotMode defaulting/normalization and validation.
server/src/index.js Broadcasts new SUPERVISOR_DECISION event to clients.
client/src/useFactory.js Displays notifications for supervisor decisions.
client/src/App.jsx Adds execution-mode selector to Settings modal and validates autopilotMode.

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

Comment thread server/src/supervisor.js Outdated
Comment thread server/src/supervisor.js Outdated
Comment thread server/src/orchestrator.js Outdated
Comment thread server/src/orchestrator.js Outdated
Comment thread server/src/orchestrator.js Outdated
Comment thread server/src/orchestrator.js
Comment thread client/src/App.jsx Outdated
- Fix BLOCKING race: set task status to 'evaluating' before async
  supervisor call to prevent poll loop from re-spawning reviewers
- Convert plan auto-approval from fire-and-forget .then()/.catch()
  to proper async function with error handling
- Normalize supervisor return shape: both evaluatePlan and
  evaluateReviewFailure now return { decision, feedback, logMessage }
- Add queue depth cap (20) and slot acquisition timeout (30s) to
  supervisor concurrency limiter with proper cleanup on reset
- Sanitize stderr before storing in task log / broadcasting to clients
- Add structured error logging with taskId, cli, elapsed time
- Use consistent 'review' stage value (was 'review-failure'/'review-max-cycles')
- Deduplicate RETRY updateTask calls in handleSupervisorReviewDecision
- Include stage context in ESCALATE notification message
- Handle non-Error throws safely in catch blocks

🐛 Fixes issues identified by hardcore-code-reviewer

👨🏻‍💻 Daniel Stilero Eliasson
- Add 'evaluating' to restartRecovery map so tasks stuck mid-supervisor
  evaluation are recovered to 'review' on server restart
- Move acquireSupervisorSlot() inside try block so slot-acquisition
  rejections (queue full, timeout) don't leak slots permanently
- Fix releaseSupervisorSlot counter to use Math.max(0, ...) guard
- Update isMaxReviewCyclesBlocker regex in workflow.js and
  TaskDetailModal.jsx to match supervisor-blocked reason strings
  (exhausted extensions, escalated) so UI action buttons render
- Strip decision markers from task fields before interpolating into
  supervisor prompts to prevent prompt injection attacks
- Fix _isError flag leaking through evaluatePlan spread operator
- Add .catch() to fire-and-forget autoApprovePlan call
- Expand sanitizeStderr to cover Bearer tokens, GitHub PATs, AWS keys

👨🏻‍💻 Daniel Stilero Eliasson
- Wrap handleSupervisorReviewDecision calls in try/catch to prevent
  tasks from being permanently stuck in 'evaluating' if the function
  throws — catch block rescues to 'blocked' or 'queued' as appropriate
- Pass supervisor prompt via stdin instead of CLI argument to avoid
  ARG_MAX (256KB on macOS) overflow with large plans/reviews
- Guard releaseSupervisorSlot with slotAcquired flag so finally block
  doesn't release a slot that was never acquired on queue-full rejection
- Add reject parameter to inner Promise to handle sync exceptions
- Add 'evaluating' to statusToStage mapping (→ review stage)
- Add 'evaluating' to orphan-check exclusion list to prevent
  conflicting status updates during supervisor evaluation
- Remove unused _isError flag from supervisor error responses

👨🏻‍💻 Daniel Stilero Eliasson

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.


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

Comment thread server/src/supervisor-integration.test.js Outdated
Comment thread client/src/App.jsx
Comment thread server/src/orchestrator.js Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.


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

Comment thread server/src/supervisor.js
}
const parsed = parseDecisionBlock(stdout);
if (!parsed) {
console.error('Supervisor returned unparseable output', { ...context, cli, elapsed, output: stdout?.slice(0, 500) });

Copilot AI Mar 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the supervisor output is unparseable, the code logs a raw stdout snippet (stdout?.slice(0, 500)). Supervisor stdout can contain task content, code, or other sensitive data, so this risks leaking secrets into server logs. Consider omitting stdout entirely, or sanitizing it similarly to sanitizeStderr (and/or only logging a hash/length plus the first line) before writing to logs.

Suggested change
console.error('Supervisor returned unparseable output', { ...context, cli, elapsed, output: stdout?.slice(0, 500) });
console.error('Supervisor returned unparseable output', { ...context, cli, elapsed });

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants