Merge branch 'main' into feat/onecli-reject-with-reason

Remove the dead /workspace/global mount and untrack v1 group seed files

container/agent-runner/src/providers/claude.ts

@@ -179,13 +179,61 @@ const preToolUseHook: HookCallback = async (input) => {
   return { continue: true };
 };
 
-/** Clear in-flight tool on PostToolUse / PostToolUseFailure. */
-const postToolUseHook: HookCallback = async () => {
+/**
+ * OneCLI reject-with-reason handoff: when an admin rejects a gateway request
+ * with a reason, the host drops it at /workspace/onecli-rejection.json BEFORE
+ * releasing the deny, so the failed tool call and the reason arrive as one
+ * event. Consume the file (single use) and hand the reason to the model as
+ * additional context on this tool result. Freshness-gated: a stale file from
+ * an interrupted run must not annotate an unrelated failure.
+ */
+const ONECLI_REJECTION_FILE = '/workspace/onecli-rejection.json';
+const ONECLI_REJECTION_FRESH_MS = 10 * 60 * 1000;
+
+function consumeOneCLIRejection(input: unknown): string | null {
+  try {
+    if (!fs.existsSync(ONECLI_REJECTION_FILE)) return null;
+    const info = JSON.parse(fs.readFileSync(ONECLI_REJECTION_FILE, 'utf8')) as {
+      rejectedAt?: string;
+      reason?: string;
+      host?: string | null;
+    };
+    // The rejection annotates a FAILED gateway call — require failure-shaped
+    // output (or the target host) in this tool result before consuming.
+    const resp = JSON.stringify((input as { tool_response?: unknown }).tool_response ?? '');
+    const looksRelated =
+      /denied|reject|403|approval|forbidden/i.test(resp) || (info.host ? resp.includes(info.host) : false);
+    if (!looksRelated) return null;
+    fs.unlinkSync(ONECLI_REJECTION_FILE);
+    if (!info.reason || Date.now() - new Date(info.rejectedAt ?? 0).getTime() > ONECLI_REJECTION_FRESH_MS) {
+      return null;
+    }
+    return (
+      `This request was rejected by the admin with the following reason: "${info.reason}". ` +
+      'Do not retry the same request as-is — address the reason first (revise and re-attempt, or explain to the user).'
+    );
+  } catch (err) {
+    log(`PostToolUse: rejection-file check failed: ${err instanceof Error ? err.message : String(err)}`);
+    return null;
+  }
+}
+
+/** Clear in-flight tool on PostToolUse / PostToolUseFailure; inject a held
+ *  OneCLI rejection reason into the failed call's context when present. */
+const postToolUseHook: HookCallback = async (input) => {
   try {
     clearContainerToolInFlight();
   } catch (err) {
     log(`PostToolUse: failed to clear container_state: ${err instanceof Error ? err.message : String(err)}`);
   }
+  const rejection = consumeOneCLIRejection(input);
+  if (rejection) {
+    log('PostToolUse: injected OneCLI rejection reason into tool response context');
+    return {
+      continue: true,
+      hookSpecificOutput: { hookEventName: 'PostToolUse', additionalContext: rejection },
+    } as unknown as Awaited<ReturnType<HookCallback>>;
+  }
   return { continue: true };
 };
 

src/modules/approvals/finalize.ts

@@ -32,9 +32,19 @@ export async function finalizeReject(
   userId: string,
   reason?: string,
 ): Promise<void> {
-  const text = reason
-    ? `Your ${approval.action} request was rejected by admin: "${reason}"`
-    : `Your ${approval.action} request was rejected by admin.`;
+  // 'onecli_credential' is an internal action key — describe the actual
+  // request (host from the persisted payload) to the agent instead.
+  let what = `${approval.action} request`;
+  if (approval.action === 'onecli_credential') {
+    let host: string | undefined;
+    try {
+      host = (JSON.parse(approval.payload ?? '{}') as { host?: string }).host;
+    } catch {
+      /* keep generic */
+    }
+    what = host ? `credential request to ${host}` : 'credential request';
+  }
+  const text = reason ? `Your ${what} was rejected by admin: "${reason}"` : `Your ${what} was rejected by admin.`;
 
   writeSessionMessage(session.agent_group_id, session.id, {
     id: `appr-note-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`,

src/modules/approvals/onecli-approvals.ts

@@ -17,9 +17,20 @@
  * Startup sweep edits any leftover cards from a previous process to
  * "Expired (host restarted)" and drops the rows.
  */
+import fs from 'fs';
+import path from 'path';
+
 import { OneCLI, type ApprovalRequest, type ManualApprovalHandle } from '@onecli-sh/sdk';
 
-import { pickApprovalDelivery, pickApprover } from './primitive.js';
+import { finalizeReject } from './finalize.js';
+import {
+  notifyApprovalResolved,
+  pickApprovalDelivery,
+  pickApprover,
+  registerApprovalResolvedHandler,
+  registerReasonRejectFinalizer,
+  REJECT_WITH_REASON_VALUE,
+} from './primitive.js';
 import { ONECLI_API_KEY, ONECLI_URL } from '../../config.js';
 import { getAgentGroup } from '../../db/agent-groups.js';
 import {
@@ -30,7 +41,8 @@ import {
 } from '../../db/sessions.js';
 import type { ChannelDeliveryAdapter } from '../../delivery.js';
 import { log } from '../../log.js';
-import type { PendingApproval } from '../../types.js';
+import { sessionDir } from '../../session-manager.js';
+import type { PendingApproval, Session } from '../../types.js';
 
 export const ONECLI_ACTION = 'onecli_credential';
 
@@ -41,6 +53,9 @@ const onecli = new OneCLI({ url: ONECLI_URL, apiKey: ONECLI_API_KEY });
 interface PendingState {
   resolve: (decision: Decision) => void;
   timer: NodeJS.Timeout;
+  /** Reject-with-reason: the gateway decision is held open until the reason
+   *  is captured and relayed (or the gateway TTL forces the deny out). */
+  heldForReason?: boolean;
 }
 
 const pending = new Map<string, PendingState>();
@@ -82,10 +97,101 @@ export function resolveOneCLIApproval(approvalId: string, selectedOption: string
   return true;
 }
 
+/**
+ * Reject-with-reason: keep the gateway decision UNRESOLVED while the reason is
+ * being captured, so the agent's held HTTP call fails only after the reason is
+ * available to it. The row stays for reason-capture; the TTL timer stays armed
+ * as the safety valve — if the approver types slower than the gateway TTL, the
+ * deny goes out then and a late reason still relays through the
+ * awaiting_reason row.
+ */
+export function holdOneCLIApprovalForReason(approvalId: string): boolean {
+  const state = pending.get(approvalId);
+  if (!state) return false;
+  state.heldForReason = true;
+  log.info('OneCLI approval held awaiting rejection reason', { approvalId });
+  return true;
+}
+
+/**
+ * Custom reason delivery for OneCLI rejects (registered with reason-capture's
+ * finalizer registry): drop the reason where the agent-runner's PostToolUse
+ * hook can inject it into the FAILED TOOL CALL itself — the session workspace,
+ * mounted at /workspace in the container — then release the held deny. The
+ * agent experiences one coherent event: its gmail call fails carrying the
+ * admin's reason, instead of a bare denial plus a separate chat message.
+ *
+ * Falls back to the chat-message relay when the decision is no longer held
+ * (gateway TTL beat the approver, or a restart lost the resolver) — the tool
+ * call already failed bare, so a session message is the only remaining path.
+ */
+async function finalizeOneCLIReasonReject(
+  approval: PendingApproval,
+  session: Session,
+  userId: string,
+  reason?: string,
+): Promise<void> {
+  const state = pending.get(approval.approval_id);
+  if (!state || !reason) {
+    await finalizeReject(approval, session, userId, reason);
+    return;
+  }
+
+  let requestMeta: { host?: string; method?: string; path?: string } = {};
+  try {
+    requestMeta = JSON.parse(approval.payload ?? '{}') as typeof requestMeta;
+  } catch {
+    /* best-effort metadata */
+  }
+  fs.writeFileSync(
+    path.join(sessionDir(session.agent_group_id, session.id), 'onecli-rejection.json'),
+    JSON.stringify({
+      rejectedAt: new Date().toISOString(),
+      reason,
+      host: requestMeta.host ?? null,
+      method: requestMeta.method ?? null,
+      path: requestMeta.path ?? null,
+    }),
+  );
+
+  pending.delete(approval.approval_id);
+  clearTimeout(state.timer);
+  updatePendingApprovalStatus(approval.approval_id, 'rejected');
+  deletePendingApproval(approval.approval_id);
+  await notifyApprovalResolved({ approval, session, outcome: 'reject', userId });
+  // Release the deny LAST — the reason file is already in place when the
+  // gateway fails the agent's held HTTP call.
+  state.resolve('deny');
+  log.info('OneCLI approval denied with reason injected into tool response', {
+    approvalId: approval.approval_id,
+    sessionId: session.id,
+  });
+}
+
+registerReasonRejectFinalizer(ONECLI_ACTION, finalizeOneCLIReasonReject);
+
+let resolvedHandlerRegistered = false;
+
 export function startOneCLIApprovalHandler(deliveryAdapter: ChannelDeliveryAdapter): void {
   if (handle) return;
   adapterRef = deliveryAdapter;
 
+  // Releases a held reject-with-reason decision once finalizeReject has
+  // relayed the reason (or the sweep gave up on the approver) — the reason
+  // reaches the agent's session BEFORE its held HTTP call fails.
+  if (!resolvedHandlerRegistered) {
+    resolvedHandlerRegistered = true;
+    registerApprovalResolvedHandler(async (event) => {
+      if (event.approval.action !== ONECLI_ACTION) return;
+      const state = pending.get(event.approval.approval_id);
+      if (!state) return; // TTL already released the deny — nothing held
+      pending.delete(event.approval.approval_id);
+      clearTimeout(state.timer);
+      state.resolve('deny');
+      log.info('OneCLI approval denied after reason relay', { approvalId: event.approval.approval_id });
+    });
+  }
+
   // Sweep any rows left over from a previous process.
   sweepStaleApprovals().catch((err) => log.error('OneCLI approval sweep failed', { err }));
 
@@ -149,6 +255,7 @@ async function handleRequest(request: ApprovalRequest): Promise<Decision> {
   const onecliOptions = [
     { label: 'Approve', selectedLabel: '✅ Approved', value: 'approve' },
     { label: 'Reject', selectedLabel: '❌ Rejected', value: 'reject' },
+    { label: 'Reject with reason…', selectedLabel: '📝 Rejected — reason requested', value: REJECT_WITH_REASON_VALUE },
   ];
   let platformMessageId: string | undefined;
   try {
@@ -202,8 +309,17 @@ async function handleRequest(request: ApprovalRequest): Promise<Decision> {
 
   return new Promise<Decision>((resolve) => {
     const timer = setTimeout(() => {
-      if (!pending.has(approvalId)) return;
+      const state = pending.get(approvalId);
+      if (!state) return;
       pending.delete(approvalId);
+      if (state.heldForReason) {
+        // Gateway TTL caught up while the approver was still typing the
+        // reason. Release the deny; keep the awaiting_reason row so the late
+        // reason (or the ghost sweep) still relays to the agent.
+        log.info('OneCLI approval TTL reached while awaiting reason — denying now', { approvalId });
+        resolve('deny');
+        return;
+      }
       expireApproval(approvalId, 'no response').catch((err) =>
         log.error('Failed to mark OneCLI approval expired', { approvalId, err }),
       );

src/modules/approvals/primitive.ts

@@ -107,6 +107,29 @@ export function registerApprovalResolvedHandler(handler: ApprovalResolvedHandler
   approvalResolvedHandlers.push(handler);
 }
 
+// ── Reason-reject finalizer overrides ──
+// By default a captured rejection reason is relayed as a session chat message
+// (finalizeReject). An action can register a custom finalizer to deliver the
+// reason differently — e.g. OneCLI credential rejects inject it into the
+// agent's failed tool call instead of a separate message.
+
+export type ReasonRejectFinalizer = (
+  approval: PendingApproval,
+  session: Session,
+  userId: string,
+  reason?: string,
+) => Promise<void>;
+
+const reasonRejectFinalizers = new Map<string, ReasonRejectFinalizer>();
+
+export function registerReasonRejectFinalizer(action: string, finalizer: ReasonRejectFinalizer): void {
+  reasonRejectFinalizers.set(action, finalizer);
+}
+
+export function getReasonRejectFinalizer(action: string): ReasonRejectFinalizer | undefined {
+  return reasonRejectFinalizers.get(action);
+}
+
 /** Fire every registered approval-resolved callback. Called by the response handler. */
 export async function notifyApprovalResolved(event: ApprovalResolvedEvent): Promise<void> {
   for (const handler of approvalResolvedHandlers) {

src/modules/approvals/reason-capture.ts

@@ -22,6 +22,7 @@ import type { InboundEvent } from '../../channels/adapter.js';
 import { getDeliveryAdapter } from '../../delivery.js';
 import {
   deletePendingApproval,
+  findSessionByAgentGroup,
   getExpiredAwaitingReasonApprovals,
   getPendingApproval,
   getSession,
@@ -32,6 +33,7 @@ import { registerMessageInterceptor } from '../../router.js';
 import type { PendingApproval, Session } from '../../types.js';
 import { ensureUserDm } from '../permissions/user-dm.js';
 import { finalizeReject } from './finalize.js';
+import { getReasonRejectFinalizer } from './primitive.js';
 
 /** How long an awaiting-reason hold waits for the admin's reply before the sweep finalizes a plain reject. */
 const REASON_CAPTURE_WINDOW_MS = 5 * 60 * 1000;
@@ -136,14 +138,21 @@ export async function captureReasonReply(event: InboundEvent): Promise<boolean>
     return false;
   }
 
-  const session = approval.session_id ? getSession(approval.session_id) : null;
+  // OneCLI credential rows carry no session_id — fall back to the originating
+  // agent group's active session so the reason still reaches the agent.
+  const session =
+    (approval.session_id ? getSession(approval.session_id) : null) ??
+    (approval.agent_group_id ? findSessionByAgentGroup(approval.agent_group_id) : null);
   if (!session) {
     deletePendingApproval(approval.approval_id);
     return true;
   }
 
   const reason = clampReason(extractText(event));
-  await finalizeReject(approval, session, arming.userId, reason || undefined);
+  // Action-specific delivery (e.g. OneCLI injects the reason into the failed
+  // tool call); default is the chat-message relay.
+  const finalizer = getReasonRejectFinalizer(approval.action) ?? finalizeReject;
+  await finalizer(approval, session, arming.userId, reason || undefined);
   log.info('reject-with-reason: reason captured and relayed', {
     approvalId: approval.approval_id,
     hasReason: reason.length > 0,
@@ -162,7 +171,9 @@ registerMessageInterceptor(captureReasonReply);
 export async function sweepAwaitingReasonRejects(): Promise<void> {
   const rows = getExpiredAwaitingReasonApprovals(new Date().toISOString());
   for (const approval of rows) {
-    const session = approval.session_id ? getSession(approval.session_id) : null;
+    const session =
+      (approval.session_id ? getSession(approval.session_id) : null) ??
+      (approval.agent_group_id ? findSessionByAgentGroup(approval.agent_group_id) : null);
     if (!session) {
       deletePendingApproval(approval.approval_id);
       continue;

src/modules/approvals/response-handler.ts

@@ -16,14 +16,14 @@
  * core iterates handlers and the first one to return `true` claims the response.
  */
 import { wakeContainer } from '../../container-runner.js';
-import { deletePendingApproval, getPendingApproval, getSession } from '../../db/sessions.js';
+import { deletePendingApproval, findSessionByAgentGroup, getPendingApproval, getSession } from '../../db/sessions.js';
 import type { ResponsePayload } from '../../response-registry.js';
 import { log } from '../../log.js';
 import { writeSessionMessage } from '../../session-manager.js';
 import type { PendingApproval } from '../../types.js';
 import { hasAdminPrivilege, isGlobalAdmin, isOwner } from '../permissions/db/user-roles.js';
 import { finalizeReject } from './finalize.js';
-import { ONECLI_ACTION, resolveOneCLIApproval } from './onecli-approvals.js';
+import { holdOneCLIApprovalForReason, ONECLI_ACTION, resolveOneCLIApproval } from './onecli-approvals.js';
 import { getApprovalHandler, notifyApprovalResolved, REJECT_WITH_REASON_VALUE } from './primitive.js';
 import { armReasonCapture } from './reason-capture.js';
 
@@ -42,6 +42,28 @@ export async function handleApprovalsResponse(payload: ResponsePayload): Promise
   }
 
   if (approval.action === ONECLI_ACTION) {
+    // "Reject with reason…" — the gateway decision is HELD open while the
+    // reason is captured, so the agent's pending HTTP call fails only after
+    // the reason is already relayed into its session (finalizeReject relays,
+    // then the approval-resolved handler in onecli-approvals releases the
+    // deny; the gateway TTL is the safety valve). OneCLI rows carry no
+    // session_id; resolve the session via the originating agent group.
+    if (payload.value === REJECT_WITH_REASON_VALUE) {
+      const session = approval.agent_group_id ? findSessionByAgentGroup(approval.agent_group_id) : undefined;
+      if (!session) {
+        log.warn('OneCLI reject-with-reason: no active session to relay to — plain reject', {
+          approvalId: approval.approval_id,
+          agentGroupId: approval.agent_group_id,
+        });
+        if (!resolveOneCLIApproval(payload.questionId, 'reject')) deletePendingApproval(payload.questionId);
+        return true;
+      }
+      // false ⇒ the resolver died with a previous process (the gateway side is
+      // already settled) — the reason relay is still worth arming.
+      holdOneCLIApprovalForReason(payload.questionId);
+      await armReasonCapture(approval, session, namespacedUserId(payload) ?? '');
+      return true;
+    }
     if (resolveOneCLIApproval(payload.questionId, payload.value)) {
       return true;
     }