Skip to content

feat(comms): route aileron-sh shell approvals through the daemon#479

Merged
ALRubinger merged 1 commit intomainfrom
worktree-comms-approvals-daemon-owned
May 6, 2026
Merged

feat(comms): route aileron-sh shell approvals through the daemon#479
ALRubinger merged 1 commit intomainfrom
worktree-comms-approvals-daemon-owned

Conversation

@ALRubinger
Copy link
Copy Markdown
Owner

Summary

Step 9A of #454. The per-launch approval unix socket (/tmp/ai-{sessionID}.sock) is gone; aileron-sh now POSTs to the daemon's /v1/sessions/{id}/approvals/shell, which long-polls until the user clicks Approve / Deny on the webapp /approvals page.

This is the first half of the comms+approvals daemon-owned work. Step 9B (#454) moves the comms server (read_messages / send / draft / http_request) similarly; that one needs a notification-config migration so it lands as a separate PR.

Surface

New endpoint:

POST /v1/sessions/{session_id}/approvals/shell
Request:  {command, reason?, cwd?}
Response: {decision: allow_once|deny|allow_project|allow_user}

The handler registers a ApprovalKindShell entry on the daemon's existing action-approval queue (the same queue powering /v1/action-approvals for the webapp), blocks on entry.Wait for the action-approval TTL (5 min default), and maps the verdict via the new approval.WireDecision.

Migrations

  • aileron-sh switches from unix-socket dial (AILERON_APPROVAL_SOCKET) to HTTP POST (AILERON_APPROVAL_URL + AILERON_SESSION_ID). Same fail-closed semantics on every error path — env unset, daemon unreachable, non-200, garbage body all collapse to deny.
  • Launch stops binding /tmp/ai-{sessionID}.sock and stops calling NewApprovalSocketServer; sets AILERON_APPROVAL_URL=daemonURL for child processes.
  • Deletes internal/launch/approval.go and approval_test.go (the socket server + RequestApproval client + decision constants).
  • New internal/approval/wire_decision.go is the shared home for the four decision-string constants and the (ActionDecision, error) → wire-string mapping. Used by the daemon handler and (in spirit) aileron-sh.

Test plan

  • go test ./internal/app/... ./internal/approval/... ./internal/launch/... ./cmd/aileron-sh/... -race is green.
  • go vet ./... is clean.
  • New tests:
    • handlers_shell_approval_test.go: AllowOnce / AllowProject / Deny / Timeout / 400 missing-command / 400 garbage-body / 503 no-queue.
    • approval/wire_decision_test.go: full mapping table including non-string save_policy, case-sensitive guard, degradation paths.
    • aileron-sh/promptapproval_test.go: env-unset → deny; decision mapping for all four wire strings + unknown; request body shape (command/reason/cwd); unreachable/non-200/garbage all fail closed.
  • All existing launch tests still pass — they don't depend on the socket server.

Notes

  • Comms server in launch keeps its own per-launch ApprovalQueue for now. Step 9B unifies it with the daemon's queue.
  • Settings.AuditLog and aileron.yaml.notifications are still read by launch in the comms path — Step 9B handles the notification-config migration.
  • The original conversation's bug (vault re-prompt across CLI + launch) is already fixed by refactor(launch): aileron launch becomes a thin client of the daemon #476; this PR is structural cleanup that makes the architecture honest about where approvals live.

🤖 Generated with Claude Code

Step 9A of #454. The per-launch approval unix socket
(/tmp/ai-{sessionID}.sock) is gone; aileron-sh now POSTs to the
daemon's /v1/sessions/{id}/approvals/shell, which long-polls until
the user clicks Approve / Deny on the webapp /approvals page.

Surface:
- New OpenAPI: POST /v1/sessions/{session_id}/approvals/shell
  Request:  {command, reason?, cwd?}
  Response: {decision: allow_once|deny|allow_project|allow_user}
- New handler internal/app/handlers_shell_approval.go: registers
  ApprovalKindShell on the daemon's existing action-approval queue,
  blocks on entry.Wait, maps the verdict via the new
  approval.WireDecision.
- internal/approval/wire_decision.go: shared home for the four
  decision-string constants and the (ActionDecision, error) →
  wire-string mapping. Was previously in internal/launch where it no
  longer belongs.

Migrations:
- aileron-sh switches from unix-socket dial (AILERON_APPROVAL_SOCKET)
  to HTTP POST (AILERON_APPROVAL_URL + AILERON_SESSION_ID). Same
  fail-closed semantics on every error path.
- launch stops binding /tmp/ai-{sessionID}.sock and stops calling
  NewApprovalSocketServer; sets AILERON_APPROVAL_URL=daemonURL for
  child processes.
- Deletes internal/launch/approval.go and approval_test.go (the
  socket server + RequestApproval client + decision constants).
  Comms server in launch keeps its own per-launch ApprovalQueue
  for now — Step 9B moves that to the daemon.

Tests:
- handlers_shell_approval_test.go: AllowOnce / AllowProject / Deny /
  Timeout / 400 / 503 across the daemon-side lifecycle.
- approval/wire_decision_test.go: full mapping table + degradation.
- aileron-sh/promptapproval_test.go: env-unset → deny; decision
  mapping; request body shape; unreachable / non-200 / garbage body
  all fail closed.

Refs #454.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@railway-app
Copy link
Copy Markdown

railway-app Bot commented May 6, 2026

🚅 Deployed to the aileron-pr-479 environment in aileron

1 service not affected by this PR
  • docs

@codecov
Copy link
Copy Markdown

codecov Bot commented May 6, 2026

Codecov Report

❌ Patch coverage is 92.18750% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.35%. Comparing base (a4d87a5) to head (dc7ffc4).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #479      +/-   ##
==========================================
+ Coverage   82.25%   82.35%   +0.09%     
==========================================
  Files         234      235       +1     
  Lines       24153    24112      -41     
==========================================
- Hits        19867    19857      -10     
+ Misses       3143     3117      -26     
+ Partials     1143     1138       -5     
Flag Coverage Δ
integration 8.43% <0.00%> (+0.02%) ⬆️
unit 79.01% <92.18%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ALRubinger ALRubinger merged commit ad11be9 into main May 6, 2026
10 checks passed
@ALRubinger ALRubinger deleted the worktree-comms-approvals-daemon-owned branch May 6, 2026 19:51
ALRubinger added a commit that referenced this pull request May 6, 2026
* feat(comms): move comms + listeners to daemon ownership (#482)

Final ADR-0012 cleanup: per-launch CommsServer (~530 lines of unix
socket IPC) is replaced by four HTTP long-poll endpoints on the
daemon, and Slack/Discord listener startup moves to a vault-unlock
callback that fires from POST /v1/vault/unlock.

- Drop notifications: from per-project policy schema; types live in
  internal/config (via #481).
- Move NotifyQueue + listener startup helpers from internal/launch
  to internal/comms; daemon owns the queue and the registry of
  active listeners across the lifetime of the process.
- Add four OpenAPI endpoints under /v1/sessions/{id}/comms/ —
  messages (GET), send / draft / http (POST). Send-shaped
  endpoints register on the daemon's existing
  ActionApprovalQueue and long-poll for the user verdict, mirroring
  the 9A shell-approval pattern from #479.
- Wire app.Config.OnVaultUnlock; UnlockLocalVault fires it
  synchronously after swapping the unlocked inner vault into the
  LockableVault wrapper. The daemon registers a callback that
  resolves Slack/Discord tokens and starts listeners.
- Switch aileron-mcp's four comms tools from the unix socket dial
  to HTTP. AILERON_COMMS_SOCKET is gone; AILERON_COMMS_URL +
  AILERON_SESSION_ID replace it.
- Strip launch-side comms: delete commsserver.go, the bridge /
  startup helpers in launcher.go, and the per-session unix socket
  binding. Launch's only remaining comms job is to set
  AILERON_COMMS_URL=daemonURL on the agent's environment.
- Update Slack/Discord guides to point at ~/.aileron/config.yaml.
- showStatusNotifications now reads from the user-scoped config.

Closes the comms half of #454.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(comms): cover listener startup + handler error branches

Push patch coverage above the 80% bar:

- Extract `startBuiltListeners` from `StartListeners` so tests can drive
  the connect/listen/bridge phase with fake listeners. Covers happy
  path, Connect-error skip, Listen-error skip, mixed good/bad list.
  StartListeners: 34.8% → 80%; startBuiltListeners: 100%.

- Add 503/400/ctx-cancel branch tests for `DraftCommsReply` (73.7% →
  89.5%) and matching shape tests for `SendCommsMessage` (80.5% →
  90.2%) and `RequestCommsHTTP` (83.9% → 87.1%).

internal/comms: 87.0% → 89.5%. internal/app: 79.9% → 80.2%.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant