mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 02:29:33 +01:00
Merge #798: omp tool approvals raise an alert in Codeman
This commit is contained in:
commit
13f1b986af
14 files changed
+922
-25
No files matched your search
+17
-4
@@ -55,6 +55,19 @@ documented default `tools.approvalMode` is `yolo`, so an omp pane auto-approves
|
||||
with no flag from Codeman — the CLI's own config, not Codeman, is what would need to
|
||||
change that.
|
||||
|
||||
**Approval alerts.** When a session is created (`POST /api/sessions` or
|
||||
`POST /api/quick-start`, local workspaces only, and only while `workspaceHooksEnabled`
|
||||
is on), Codeman writes a hook module, `.claude/hooks/pre/codeman-approval.ts`, into the
|
||||
workspace; omp loads project hooks from that folder. It forwards omp's
|
||||
`tool_approval_requested` / `tool_approval_resolved` events to `/api/hook-event`, so a
|
||||
pending approval raises a tab alert, a notification and an Approvals Inbox item. The
|
||||
item is **alert-only**: answer the dialog in the omp pane (omp's composer keeps taking
|
||||
typing while the prompt is docked, so Codeman never types an answer for you). The
|
||||
registry declares this as `capabilities.approvalHookFile`, separate from `hooks`, which
|
||||
stays `'none'`: the `blocked` wait signal works for omp, `stop` is still a 400. ⚠️ With
|
||||
omp's default `tools.approvalMode: yolo` omp never asks, so the module only does
|
||||
anything once that setting is changed.
|
||||
|
||||
Env overrides: the `OMP_*` prefix is allowlisted, and per omp's own
|
||||
`docs/environment-variables.md` it is not the narrow surface it looks like. omp reads
|
||||
roughly 40 provider keys from the environment (`ANTHROPIC_API_KEY`, `OPENAI_API_KEY`,
|
||||
@@ -163,13 +176,13 @@ remote user's home, so resolving locally would pin a stranger's id. See
|
||||
|
||||
## Known gaps
|
||||
|
||||
- **No idle/completion hook.** Idle detection reads the screen instead: the
|
||||
- **No idle/completion hook.** The approval hook module (see Approval alerts
|
||||
above) reports approvals only, so idle detection reads the screen instead: the
|
||||
registry entry's `workDetect` names omp's `╰─` input row as the glyph that arms
|
||||
the idle check, and the status bar's spinner plus elapsed time (` ⠼ 14s > ⬢ …`)
|
||||
or the `⎋ Working…` row as the working line, measured on omp 18.8.6 and 18.0.11.
|
||||
Without it an omp session that had started a turn never left `busy`. If omp ever
|
||||
ships a hooks system, a Codeman hook POSTing to `/api/hook-event` would still be
|
||||
the highest-value follow-up.
|
||||
Without it an omp session that had started a turn never left `busy`. Reporting
|
||||
end-of-turn from the same module would be the follow-up that earns omp `stop`.
|
||||
- **Killing a pane mid-turn loses the conversation for real.** `tmux kill-session`
|
||||
before an in-TUI `/exit` beats omp's own session-file flush — confirmed by direct
|
||||
testing (kill after a clean `/exit` resumes correctly; kill without `/exit` first
|
||||
|
||||
@@ -389,6 +389,10 @@ question or trust dialog is up; a wait for it answers at once if one already is)
|
||||
lifecycle transitions there are coarse (a short shell command may emit **no**
|
||||
`idle` transition at all, verified live), so synchronize those with markers.
|
||||
|
||||
`blocked` also works on a local `omp` session: Codeman installs a hook module into its
|
||||
workspace that reports omp's approval dialogs (the inbox item is alert-only, so answer in
|
||||
the pane). `stop` stays a 400 there, so wait on `idle` or a marker for end-of-turn.
|
||||
|
||||
⚠️ A dsh session can still refuse them for a per-SESSION reason: `statusReporting:
|
||||
false` at create time disarms the bridge, and an explicit `until=stop` is then a 400
|
||||
naming that setting. And a `stop` that is *accepted* is not proof it will ever fire —
|
||||
|
||||
@@ -389,6 +389,10 @@ question or trust dialog is up; a wait for it answers at once if one already is)
|
||||
lifecycle transitions there are coarse (a short shell command may emit **no**
|
||||
`idle` transition at all, verified live), so synchronize those with markers.
|
||||
|
||||
`blocked` also works on a local `omp` session: Codeman installs a hook module into its
|
||||
workspace that reports omp's approval dialogs (the inbox item is alert-only, so answer in
|
||||
the pane). `stop` stays a 400 there, so wait on `idle` or a marker for end-of-turn.
|
||||
|
||||
⚠️ A dsh session can still refuse them for a per-SESSION reason: `statusReporting:
|
||||
false` at create time disarms the bridge, and an explicit `until=stop` is then a 400
|
||||
naming that setting. And a `stop` that is *accepted* is not proof it will ever fire —
|
||||
|
||||
@@ -325,6 +325,14 @@ const capabilitiesSchema = z
|
||||
// A closed list on purpose: it decides where the server WRITES, and this file is
|
||||
// user-editable, so it can never be a free path (`..`, absolute).
|
||||
agentSkillDir: z.enum(AGENT_SKILL_DIRS).optional(),
|
||||
// The hook module Codeman installs for a CLI whose hooks are files, not a settings
|
||||
// block (omp). Same closed-list reasoning as `agentSkillDir`, stricter still: this
|
||||
// name is written into the user's workspace, so a value like `../../evil.ts` must be
|
||||
// refused by the schema rather than by whatever calls the writer.
|
||||
approvalHookFile: z
|
||||
.string()
|
||||
.regex(/^[A-Za-z0-9._-]+\.ts$/)
|
||||
.optional(),
|
||||
statusLineTelemetry: z.boolean(),
|
||||
// How many columns this CLI indents its transcript body by, so a copy can take
|
||||
// that much off the clipboard. Bounded, because it is the whole strip: a copy
|
||||
|
||||
@@ -1661,9 +1661,11 @@ const DEEPSEEK: CliEntry = {
|
||||
};
|
||||
|
||||
// OMP (`omp`, omp.sh). The plainest entry in the catalog after opencode: no permission
|
||||
// flags at all — omp reads its model routing and hooks from `~/.omp/agent`, so the CLI's
|
||||
// own config governs and there is deliberately nothing bypass-shaped to clamp. Its only
|
||||
// privileged surface is a pair of ENV keys (see privilegedEnvKeys below).
|
||||
// flags at all — omp reads its model routing, approval mode and user hooks from
|
||||
// `~/.omp/agent`, so the CLI's own config governs and there is deliberately nothing
|
||||
// bypass-shaped to clamp. The one thing Codeman adds is a project hook module that
|
||||
// reports approval dialogs (`approvalHookFile` below). Its only privileged surface is a
|
||||
// pair of ENV keys (see privilegedEnvKeys below).
|
||||
const OMP: CliEntry = {
|
||||
id: 'omp' as CliEntry['id'],
|
||||
label: 'OMP',
|
||||
@@ -1742,6 +1744,19 @@ const OMP: CliEntry = {
|
||||
capabilities: {
|
||||
...agentDefaults(),
|
||||
...AGENTS_DIR_SKILL,
|
||||
// omp emits REAL approval events (`tool_approval_requested` / `tool_approval_resolved`),
|
||||
// so an omp pane can report a pending dialog first-hand instead of by inference.
|
||||
// Codeman installs the hook module named by `approvalHookFile` into
|
||||
// `<workspace>/.claude/hooks/pre/` when a session is created (both create paths); it
|
||||
// forwards those two events to /api/hook-event as `permission_prompt` /
|
||||
// `elicitation_complete`, so the tab alert, the desktop notification and an ALERT-ONLY
|
||||
// Approvals Inbox item (answer in the terminal) work.
|
||||
// ⚠️ `hooks` stays 'none' (inherited from agentDefaults): that value means Claude's full
|
||||
// hook set (`stop`, `idle_prompt`, answerable dialogs, `.claude/settings.local.json`),
|
||||
// and omp has none of it. `approvalHookFile` is the separate, narrower capability;
|
||||
// `approvalHookAvailableForMode()` is its predicate. Idle stays screen-read via
|
||||
// `workDetect` below.
|
||||
approvalHookFile: 'codeman-approval.ts',
|
||||
// Fullscreen alt-screen TUI, same shape as antigravity/grok: only the
|
||||
// tmux-attach-time smcup strip, not Ink's full erase-scrollback+DECSET strip.
|
||||
altScreen: 'strip-mux-only',
|
||||
@@ -1763,7 +1778,8 @@ const OMP: CliEntry = {
|
||||
promptGlyph: '╰─',
|
||||
workingLine: '[⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏] [0-9hms ]+> |⎋ Working',
|
||||
},
|
||||
// No permission prompts and no bypass flag, so nothing config-shaped to clamp — the
|
||||
// No permission FLAGS (omp's approval mode lives in its own config, `tools.approvalMode`)
|
||||
// and no bypass flag, so nothing config-shaped to clamp — the
|
||||
// whole privileged surface here is env-shaped.
|
||||
privilegedParams: [],
|
||||
// Where omp resolves its auth from. No known concrete exfiltration path today (omp
|
||||
|
||||
@@ -508,8 +508,11 @@ export interface CliCapabilities {
|
||||
* Whether `stop`/`blocked` wait signals can ever fire for this CLI.
|
||||
*
|
||||
* ⚠️ A TRI-STATE, not a boolean, because for one CLI this is a per-SESSION question:
|
||||
* 'none' — no hook signals, ever (every external CLI, and `shell`).
|
||||
* 'always' — the CLI installs Codeman's hooks (claude).
|
||||
* 'none' — no hook signals, ever (every external CLI, and `shell`). A CLI can
|
||||
* still report approval dialogs alone through `approvalHookFile` below.
|
||||
* 'always' — the CLI installs Codeman's hooks (claude). Every consumer reads this
|
||||
* as Claude's FULL hook set: `stop`, `idle_prompt`, answerable inbox
|
||||
* items, and a `.claude/settings.local.json` block in the workspace.
|
||||
* 'supervised' — the CLI REPORTS its own idle/working/blocked state to a supervisor
|
||||
* over a generic env-gated contract, and Codeman is that supervisor
|
||||
* (deepseek, via deepseek-status-shim.ts). Definitive rather than
|
||||
@@ -523,6 +526,21 @@ export interface CliCapabilities {
|
||||
* timeout the predicate exists to prevent.
|
||||
*/
|
||||
hooks: 'none' | 'always' | 'supervised';
|
||||
/**
|
||||
* The hook MODULE Codeman installs into the workspace of a CLI that reports its
|
||||
* approval dialogs, and nothing else, through hooks. Independent of `hooks` above.
|
||||
*
|
||||
* omp is the case that exists today: it loads `<cwd>/.claude/hooks/pre/*.ts`, and the
|
||||
* installed module forwards `tool_approval_requested`/`tool_approval_resolved` to
|
||||
* `/api/hook-event` as `permission_prompt`/`elicitation_complete`. What that earns
|
||||
* (`approvalHookAvailableForMode()`): the `blocked` wait signal, a tab alert, a
|
||||
* notification and an ALERT-ONLY inbox item (`answerInTerminal`). Never `stop`, never
|
||||
* `idle_prompt`, never Approve/Deny keystrokes.
|
||||
*
|
||||
* Declared as data, not a branch on the id, so both create paths can ask
|
||||
* "does this CLI need a hook file installed?" (see cli-registry-no-id-branching).
|
||||
*/
|
||||
approvalHookFile?: string;
|
||||
/**
|
||||
* Which transcript reader, if any, understands this CLI's on-disk history.
|
||||
*
|
||||
|
||||
@@ -1669,6 +1669,167 @@ export async function removeAgentSkillFrom(skillDir: string): Promise<AgentSkill
|
||||
});
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
// omp approval hooks
|
||||
// ============================================================================
|
||||
|
||||
/** Marks the hook file this module owns inside a workspace. */
|
||||
const OMP_APPROVAL_HOOK_MARKER = 'CODEMAN_OMP_APPROVAL_HOOK_V1';
|
||||
|
||||
/**
|
||||
* omp's own hook directory, workspace-relative. omp scans `<cwd>/.claude/hooks/pre`
|
||||
* and `<cwd>/.claude/hooks/post` (`projectDir` for the `claude` persona) plus a
|
||||
* user-level `<agentDir>/hooks`, and loads each `.ts`/`.js` file's default export as
|
||||
* `(pi: HookAPI) => void`. A project-level `pre` hook is deliberately the target: it
|
||||
* travels with the workspace the session actually runs in, so no per-machine or
|
||||
* per-user install is needed and removing the workspace removes the hook.
|
||||
*/
|
||||
export const OMP_APPROVAL_HOOK_DIR = '.claude/hooks/pre';
|
||||
export const OMP_APPROVAL_HOOK_FILE = 'codeman-approval.ts';
|
||||
|
||||
/**
|
||||
* Source of the hook that turns omp's own approval events into Codeman hook events.
|
||||
*
|
||||
* WHY this exists: omp installs no Claude-style `settings.local.json` hook block — its
|
||||
* hooks are modules — so without this file an omp pane asking for tool approval is
|
||||
* silent: `/api/approvals` stays empty, the tab never goes red and no notification
|
||||
* fires. The registry declares it through `capabilities.approvalHookFile`, NOT
|
||||
* `hooks: 'always'` (which would also promise `stop` and answerable dialogs).
|
||||
*
|
||||
* The mapping uses Claude's event names, because the server routes on them
|
||||
* (`APPROVAL_KIND_BY_EVENT` in web/routes/hook-event-routes.ts):
|
||||
* - `tool_approval_requested` -> `permission_prompt` (opens an alert-only inbox item)
|
||||
* - `tool_approval_resolved` -> `elicitation_complete` (resolves it, whether the user
|
||||
* approved or denied, so a denied dialog does not leave a red tab forever)
|
||||
*
|
||||
* Several calls in one batch can each wait on their own prompt, and the inbox keeps one
|
||||
* item per session, so the module tracks pending `toolCallId`s and reports the close only
|
||||
* when the last one resolves. That set drains because omp pairs every request with a
|
||||
* resolve, an aborted prompt included (its approval wrapper emits `approved: false` from
|
||||
* the catch, omp 18.18.6 `extensibility/extensions/wrapper.ts`); only omp dying mid-prompt
|
||||
* skips it, which leaves the alert up exactly as it would without the set. Reports go out
|
||||
* in order (one chain), so a fast approve cannot overtake the prompt that opened it.
|
||||
*
|
||||
* `reason` is omp's POLICY note (a restricted folder, a critical-pattern match), not the
|
||||
* command, and is absent on an ordinary mode-raised prompt, so it rides `message`.
|
||||
*
|
||||
* HTTPS: `CODEMAN_API_URL` is loopback HTTPS with a self-signed certificate on `--https`
|
||||
* and tailscale installs (Codeman's own hook curl passes `-k`, the DeepSeek shim sets
|
||||
* `rejectUnauthorized: false`). omp runs on Bun, whose fetch verifies certificates by
|
||||
* default, so an `https:` URL gets Bun's `tls: { rejectUnauthorized: false }`.
|
||||
*
|
||||
* ⚠️ Fails OPEN and SILENT, like the memory-gate hook: a hook that throws would break
|
||||
* the approval flow it exists to report. Every path catches, and nothing is awaited
|
||||
* unbounded — each POST carries a short AbortSignal and the handlers never await it.
|
||||
*/
|
||||
export function generateOmpApprovalHook(): string {
|
||||
return `// ${OMP_APPROVAL_HOOK_MARKER} — auto-generated by Codeman; safe to delete, rewritten when a session is created.
|
||||
//
|
||||
// Reports omp's tool-approval dialogs to Codeman's /api/hook-event so an omp pane
|
||||
// raises a tab alert and a desktop notification. Answer the dialog in the terminal.
|
||||
// Codeman owns this file: a foreign file of the same name is never overwritten.
|
||||
import { readFile } from "node:fs/promises";
|
||||
import type { HookAPI } from "@oh-my-pi/pi-coding-agent/extensibility/hooks";
|
||||
|
||||
const API_URL = process.env.CODEMAN_API_URL;
|
||||
const SESSION_ID = process.env.CODEMAN_SESSION_ID;
|
||||
const SECRET_FILE = process.env.CODEMAN_HOOK_SECRET_FILE;
|
||||
// Loopback HTTPS (--https, tailscale) uses a self-signed certificate, which Bun's fetch
|
||||
// refuses by default; Codeman's own hook curl passes -k for the same reason.
|
||||
const TLS = API_URL?.startsWith("https:") ? { tls: { rejectUnauthorized: false } } : {};
|
||||
|
||||
/** POST one Codeman hook event. Never throws; bounded by its timeout. */
|
||||
async function send(event: string, data: Record<string, unknown>): Promise<void> {
|
||||
if (!API_URL || !SESSION_ID) return;
|
||||
try {
|
||||
const secret = SECRET_FILE ? (await readFile(SECRET_FILE, "utf8")).trim() : "";
|
||||
await fetch(\`\${API_URL}/api/hook-event\`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json", "X-Codeman-Hook-Secret": secret },
|
||||
body: JSON.stringify({ event, sessionId: SESSION_ID, data }),
|
||||
signal: AbortSignal.timeout(5000),
|
||||
...TLS,
|
||||
});
|
||||
} catch {
|
||||
// Best effort by contract: a failed report must never disturb the dialog itself.
|
||||
}
|
||||
}
|
||||
|
||||
// One chain, so reports arrive in the order omp raised them: a fast approve must not
|
||||
// overtake the prompt it answers and leave the alert up.
|
||||
let chain: Promise<void> = Promise.resolve();
|
||||
function report(event: string, data: Record<string, unknown>): void {
|
||||
chain = chain.then(() => send(event, data));
|
||||
}
|
||||
|
||||
export default function codemanApprovalHook(pi: HookAPI): void {
|
||||
// Calls in one batch can each wait on their own prompt, and Codeman keeps one alert per
|
||||
// session, so it is cleared only when the LAST pending approval resolves.
|
||||
const pending = new Set<string>();
|
||||
|
||||
pi.on("tool_approval_requested", (event) => {
|
||||
pending.add(event.toolCallId);
|
||||
// \`reason\` is omp's policy note (restricted folder, critical pattern), not the command.
|
||||
report("permission_prompt", {
|
||||
tool_name: event.toolName,
|
||||
...(event.reason ? { message: event.reason } : {}),
|
||||
});
|
||||
});
|
||||
|
||||
pi.on("tool_approval_resolved", (event) => {
|
||||
pending.delete(event.toolCallId);
|
||||
if (pending.size > 0) return;
|
||||
report("elicitation_complete", {
|
||||
tool_name: event.toolName,
|
||||
approved: event.approved,
|
||||
});
|
||||
});
|
||||
}
|
||||
`;
|
||||
}
|
||||
|
||||
export type ApprovalHookApplyResult = 'installed' | 'refreshed' | 'unchanged' | 'foreign' | 'symlink';
|
||||
|
||||
/**
|
||||
* Install or refresh the omp approval hook in `<workspace>/.claude/hooks/pre/`.
|
||||
*
|
||||
* Ownership: the file carries `OMP_APPROVAL_HOOK_MARKER` in its first line. A file
|
||||
* WITHOUT the marker is somebody else's hook and is left untouched (`foreign`) — the
|
||||
* same posture `installAgentSkillInto` takes, so Codeman never clobbers user work.
|
||||
* EVERY component below the workspace (`.claude`, `.claude/hooks`, `.claude/hooks/pre`)
|
||||
* and the target file itself is `lstat`ed, and a symlink anywhere is refused rather than
|
||||
* written through: a linked `.claude/hooks` once put `pre/<file>` in the link's target.
|
||||
*
|
||||
* Idempotent: an unchanged file is not rewritten (`unchanged`), so this is safe to call
|
||||
* on every omp session create.
|
||||
*/
|
||||
export async function applyApprovalHookFile(workspace: string, fileName: string): Promise<ApprovalHookApplyResult> {
|
||||
const hookDir = join(workspace, OMP_APPROVAL_HOOK_DIR);
|
||||
const target = join(hookDir, fileName);
|
||||
return withSettingsLock(target, async () => {
|
||||
let component = workspace;
|
||||
for (const part of [...OMP_APPROVAL_HOOK_DIR.split('/'), fileName]) {
|
||||
component = join(component, part);
|
||||
if (await isSymlink(component)) return 'symlink';
|
||||
}
|
||||
|
||||
let existing: string | null = null;
|
||||
try {
|
||||
existing = await readFile(target, 'utf-8');
|
||||
} catch {
|
||||
// absent: fresh install
|
||||
}
|
||||
if (existing !== null && !existing.includes(OMP_APPROVAL_HOOK_MARKER)) return 'foreign';
|
||||
|
||||
const content = generateOmpApprovalHook();
|
||||
if (existing === content) return 'unchanged';
|
||||
|
||||
await mkdir(hookDir, { recursive: true });
|
||||
await writeSkillFileAtomic(target, content);
|
||||
return existing === null ? 'installed' : 'refreshed';
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Claude Code's project skills directory, and the meaning `agentSkillInjection: true`
|
||||
* had before `capabilities.agentSkillDir` existed.
|
||||
|
||||
@@ -13,11 +13,24 @@ import { HookEventSchema, isValidWorkingDir } from '../schemas.js';
|
||||
import { sanitizeHookData, parseBody } from '../route-helpers.js';
|
||||
import { persistDockerCaseClaudeSessionId } from '../../docker-hosts.js';
|
||||
import { getDataDir } from '../../config/instance.js';
|
||||
import { sessionWaits, hooksAvailableForMode, sessionHookOptions } from '../session-wait-registry.js';
|
||||
import {
|
||||
sessionWaits,
|
||||
hooksAvailableForMode,
|
||||
approvalHookAvailableForMode,
|
||||
sessionHookOptions,
|
||||
} from '../session-wait-registry.js';
|
||||
import { getCli } from '../../config/cli-registry/registry.js';
|
||||
import { approvalInbox, type ApprovalKind } from '../approval-inbox.js';
|
||||
import type { SessionPort, EventPort, RespawnPort, ConfigPort, InfraPort } from '../ports/index.js';
|
||||
|
||||
/**
|
||||
* The only events an approval-only hook module (`capabilities.approvalHookFile`, omp)
|
||||
* sends: a dialog opened, and a dialog closed. Anything else claimed for such a session
|
||||
* (`stop`, `idle_prompt`) is a signal it can never legitimately emit, so it is dropped
|
||||
* like any other mode-gated forgery.
|
||||
*/
|
||||
const APPROVAL_HOOK_EVENTS = new Set(['permission_prompt', 'elicitation_complete']);
|
||||
|
||||
/** Hook events that open an Approvals Inbox item. */
|
||||
const APPROVAL_KIND_BY_EVENT: Record<string, ApprovalKind> = {
|
||||
permission_prompt: 'permission',
|
||||
@@ -93,12 +106,21 @@ export function registerHookEventRoutes(
|
||||
// could never legitimately emit one is now dropped instead of steering another
|
||||
// agent's control flow.
|
||||
const waitSession = ctx.sessions.get(sessionId);
|
||||
// An approval-only session (omp) reports dialogs and nothing else, so it reaches the
|
||||
// `blocked` half of the gate and never the `stop` half.
|
||||
const approvalOnly =
|
||||
!!waitSession &&
|
||||
!hooksAvailableForMode(waitSession.mode, sessionHookOptions(waitSession)) &&
|
||||
approvalHookAvailableForMode(waitSession.mode, sessionHookOptions(waitSession)) &&
|
||||
APPROVAL_HOOK_EVENTS.has(event);
|
||||
if (waitSession && hooksAvailableForMode(waitSession.mode, sessionHookOptions(waitSession))) {
|
||||
if (event === 'stop') {
|
||||
sessionWaits.notifySignal(sessionId, 'stop');
|
||||
} else if (event === 'permission_prompt' || event === 'elicitation_dialog') {
|
||||
sessionWaits.notifySignal(sessionId, 'blocked');
|
||||
}
|
||||
} else if (approvalOnly && event === 'permission_prompt') {
|
||||
sessionWaits.notifySignal(sessionId, 'blocked');
|
||||
}
|
||||
|
||||
// Signal the respawn controller based on hook event type
|
||||
@@ -195,7 +217,7 @@ export function registerHookEventRoutes(
|
||||
// replacement would forget the item was unseen, or that a person had seen it). With no idle
|
||||
// item pending, the keep-alive's idle prompt opens a pre-acknowledged one.
|
||||
const keepsPendingIdle = keepAliveReason !== undefined && approvalInbox.getForSession(sessionId)?.kind === 'idle';
|
||||
if (session && hooksAvailableForMode(session.mode, sessionHookOptions(session))) {
|
||||
if (session && (hooksAvailableForMode(session.mode, sessionHookOptions(session)) || approvalOnly)) {
|
||||
if (approvalKind === 'idle' && keepsPendingIdle) {
|
||||
// Left in place: no new item, and the broadcast below carries the keep-alive reason.
|
||||
} else if (approvalKind) {
|
||||
@@ -214,10 +236,12 @@ export function registerHookEventRoutes(
|
||||
toolSummary: typeof toolSummary === 'string' ? toolSummary : undefined,
|
||||
message: typeof safeData.message === 'string' ? safeData.message : undefined,
|
||||
cwd: typeof safeData.cwd === 'string' ? safeData.cwd : undefined,
|
||||
// Supervised-hook CLIs (DeepSeek Harness): dialog keystrokes are unverifiable,
|
||||
// so the item is an alert — answer in the terminal. Capability-gated, not by
|
||||
// mode id, so a future supervised CLI inherits the same shape.
|
||||
answerInTerminal: getCli(session.mode)?.capabilities.hooks === 'supervised',
|
||||
// Supervised-hook CLIs (DeepSeek Harness) and approval-only hook modules (omp):
|
||||
// dialog keystrokes are unverifiable (the inbox's re-capture parses Claude's
|
||||
// numbered dialog only, and omp's composer keeps taking typing while its prompt
|
||||
// is docked, so a `1` would land there), so the item is an alert — answer in
|
||||
// the terminal. Capability-gated, not by mode id.
|
||||
answerInTerminal: getCli(session.mode)?.capabilities.hooks === 'supervised' || approvalOnly,
|
||||
// What the pane says is still running in the background. An idle prompt from a
|
||||
// session that is watching its own work opens acknowledged, so it never arms an
|
||||
// alert nobody can answer; notePrompt() carries the whole reasoning.
|
||||
@@ -268,9 +292,12 @@ export function registerHookEventRoutes(
|
||||
sessionId,
|
||||
sessionName,
|
||||
...safeData,
|
||||
// Supervised-hook CLIs answer in the terminal: omit approvalId so push
|
||||
// cannot offer Approve/Deny that the answer route would refuse.
|
||||
...(approvalId && session && getCli(session.mode)?.capabilities.hooks !== 'supervised' && { approvalId }),
|
||||
// Supervised-hook CLIs and approval-only modules answer in the terminal: omit
|
||||
// approvalId so push cannot offer Approve/Deny that the answer route would refuse.
|
||||
...(approvalId &&
|
||||
session &&
|
||||
getCli(session.mode)?.capabilities.hooks !== 'supervised' &&
|
||||
!approvalOnly && { approvalId }),
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -42,7 +42,12 @@ import { clampEnvOverridesForOwner } from '../../session-env-clamp.js';
|
||||
import { Session } from '../../session.js';
|
||||
import { resolveClaudeModeForUsername } from '../../user-store.js';
|
||||
import { getCli } from '../../config/cli-registry/registry.js';
|
||||
import { agentSkillDir, applyWorkspaceHooks, seedAgentSessionPreamble } from '../../hooks-config.js';
|
||||
import {
|
||||
agentSkillDir,
|
||||
applyApprovalHookFile,
|
||||
applyWorkspaceHooks,
|
||||
seedAgentSessionPreamble,
|
||||
} from '../../hooks-config.js';
|
||||
import { getLifecycleLog } from '../../session-lifecycle-log.js';
|
||||
import { STATS_COLLECTION_INTERVAL_MS } from '../../config/server-timing.js';
|
||||
import { SseEvent } from '../sse-events.js';
|
||||
@@ -260,6 +265,17 @@ export function registerRebootRestoreRoutes(app: FastifyInstance, ctx: RebootRes
|
||||
console.warn(`[reboot-restore] hook install failed for ${session.workingDir}: ${getErrorMessage(err)}`)
|
||||
);
|
||||
}
|
||||
// Same reasoning for a CLI that reports approvals through a hook MODULE (omp,
|
||||
// `approvalHookFile`): a deleted or recreated workspace would leave the restored
|
||||
// pane's approval dialogs silent. Local-only is structural here (see below).
|
||||
const approvalHookFile = getCli(session.mode)?.capabilities.approvalHookFile;
|
||||
if (workspaceHooksEnabled && approvalHookFile) {
|
||||
await applyApprovalHookFile(session.workingDir, approvalHookFile).catch((err: unknown) =>
|
||||
console.warn(
|
||||
`[reboot-restore] approval hook install failed for ${session.workingDir}: ${getErrorMessage(err)}`
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
// Both create paths seed this; without it a restored claude session's agent
|
||||
// skill falls back to writing out the whole ~150-line §0 preamble. Remote and
|
||||
|
||||
@@ -156,6 +156,7 @@ import {
|
||||
seedAgentSessionPreamble,
|
||||
applyWorkspaceHooks,
|
||||
refreshStaleCodemanHooks,
|
||||
applyApprovalHookFile,
|
||||
} from '../../hooks-config.js';
|
||||
import { generateClaudeMd } from '../../templates/claude-md.js';
|
||||
import { imageWatcher } from '../../image-watcher.js';
|
||||
@@ -1089,6 +1090,19 @@ export function registerSessionRoutes(
|
||||
/** Test seam: inject a registry with fake IO instead of the real TCP/WoL probes. */
|
||||
options: { remoteWake?: RemoteWakeRegistry } = {}
|
||||
): RemoteWakeRegistry {
|
||||
/**
|
||||
* Install the approval hook MODULE a CLI declares (`capabilities.approvalHookFile`,
|
||||
* omp) into a LOCAL workspace, so its approval dialogs raise an alert. Shared by
|
||||
* POST /api/sessions and POST /api/quick-start so the two cannot drift; callers own
|
||||
* the remote/Docker/named-workspace conditions and call it only after the launch
|
||||
* pre-flight. Gated on `workspaceHooksEnabled` like Claude's block. Best-effort: a
|
||||
* failure here must not fail the create.
|
||||
*/
|
||||
const installApprovalHook = async (mode: SessionMode, workspace: string): Promise<void> => {
|
||||
const fileName = getCli(mode)?.capabilities.approvalHookFile;
|
||||
if (!fileName || !(await ctx.getWorkspaceHooksEnabled())) return;
|
||||
await applyApprovalHookFile(workspace, fileName).catch(() => {});
|
||||
};
|
||||
// Wake-on-LAN for sleeping remote hosts (see remote-wake.ts). One registry per
|
||||
// route registration (= one web server) — the same shape as the process-wide
|
||||
// `sessionWaits` singleton, but without the global.
|
||||
@@ -1352,6 +1366,13 @@ export function registerSessionRoutes(
|
||||
}
|
||||
}
|
||||
|
||||
// Approval hook module (omp): after the pre-flight, like the agent skill below, so a
|
||||
// CLI that cannot start leaves nothing in the workspace. Same remote and
|
||||
// named-workingDir conditions as the hooks above.
|
||||
if (!remote && body.workingDir) {
|
||||
await installApprovalHook(body.mode ?? 'claude', workingDir);
|
||||
}
|
||||
|
||||
// Agent skill (docs/agent-control-plan.md §2): ADD-ONLY on create, same shared-
|
||||
// workspace rationale as the statusLine above: a create must never remove the
|
||||
// skill from under other live sessions in the repo. Marker-guarded, so a user's
|
||||
@@ -4630,6 +4651,13 @@ export function registerSessionRoutes(
|
||||
}
|
||||
}
|
||||
|
||||
// Approval hook module (omp), for a new AND an existing case. The launch pre-flight
|
||||
// above has passed. Local only: a remote case path lives on another host, and a Docker
|
||||
// pane cannot reach the loopback API (`approvalHookAvailableForMode()` answers no there).
|
||||
if (!remote && !docker) {
|
||||
await installApprovalHook(mode, workspacePath);
|
||||
}
|
||||
|
||||
// Agent skill injection (docs/agent-control-plan.md §2): ADD-ONLY on create,
|
||||
// marker-guarded (a user's own copy is never touched). Each CLI that takes the
|
||||
// skill gets it in the directory it scans (`capabilities.agentSkillDir`:
|
||||
|
||||
@@ -295,15 +295,38 @@ export function sessionHookOptions(session: {
|
||||
|
||||
/**
|
||||
* Whether `blocked` can ever fire for this session: its CLI reports it through hooks
|
||||
* (`hooksAvailableForMode()`), or Codeman reads the dialogs it blocks on off its screen
|
||||
* (`capabilities.workDetect.blockedLine`, codex). The screen half needs no per-session
|
||||
* facts: a remote or Docker pane is still a local tmux pane showing the CLI's screen
|
||||
* (their nested tmux runs with `status off`, so nothing is drawn below the CLI).
|
||||
* (`hooksAvailableForMode()`), through an approval-only hook module
|
||||
* (`approvalHookAvailableForMode()`, omp), or Codeman reads the dialogs it blocks on off
|
||||
* its screen (`capabilities.workDetect.blockedLine`, codex). The screen half needs no
|
||||
* per-session facts: a remote or Docker pane is still a local tmux pane showing the CLI's
|
||||
* screen (their nested tmux runs with `status off`, so nothing is drawn below the CLI).
|
||||
*
|
||||
* ⚠️ Says nothing about `stop`. A screen shows a dialog; it never says a turn ended.
|
||||
*/
|
||||
export function blockedSignalAvailable(mode: SessionMode, options: HookCapabilityOptions = {}): boolean {
|
||||
return hooksAvailableForMode(mode, options) || !!getCli(mode)?.capabilities.workDetect?.blockedLine;
|
||||
return (
|
||||
hooksAvailableForMode(mode, options) ||
|
||||
approvalHookAvailableForMode(mode, options) ||
|
||||
!!getCli(mode)?.capabilities.workDetect?.blockedLine
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether this session reports its approval dialogs, and ONLY those, through a hook
|
||||
* module Codeman installed into its workspace (`capabilities.approvalHookFile`, omp).
|
||||
*
|
||||
* Deliberately separate from `hooksAvailableForMode()`: its consumers read a yes as
|
||||
* "this CLI has Claude's full hook set", so `stop` would be accepted and never fire, and
|
||||
* the inbox would offer Approve/Deny keystrokes it cannot verify. A yes HERE means
|
||||
* `blocked` can fire and a dialog opens an alert-only inbox item, nothing more.
|
||||
*
|
||||
* Per-session like the supervised bridge: the module is installed only into a LOCAL
|
||||
* workspace Codeman created the session in, so an adopted pane, a remote attach and a
|
||||
* Docker case (`deepSeekBridgeUnreachable`, set for both) never have one.
|
||||
*/
|
||||
export function approvalHookAvailableForMode(mode: SessionMode, options: HookCapabilityOptions = {}): boolean {
|
||||
if (options.adopted || options.deepSeekBridgeUnreachable) return false;
|
||||
return !!getCli(mode)?.capabilities.approvalHookFile;
|
||||
}
|
||||
|
||||
/** Outcome of resolving a caller-supplied wait target against a session's mode. */
|
||||
|
||||
@@ -0,0 +1,322 @@
|
||||
/**
|
||||
* @fileoverview omp approval-hook wiring.
|
||||
*
|
||||
* Pins what makes an omp pane able to report a pending tool approval, and what it must
|
||||
* NOT earn by doing so: the registry declares the narrow `approvalHookFile` capability
|
||||
* while `hooks` stays 'none' (so `stop` is still a 400), Codeman installs the module
|
||||
* that emits the events, and the module maps omp's events onto the names the server
|
||||
* already routes on. The route side (both create paths, alert-only inbox items) lives in
|
||||
* test/routes/omp-approval-hook-routes.test.ts and test/routes/approval-routes.test.ts.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
import { mkdtempSync, mkdirSync, readFileSync, rmSync, symlinkSync, writeFileSync, existsSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { pathToFileURL } from 'node:url';
|
||||
import {
|
||||
applyApprovalHookFile,
|
||||
generateOmpApprovalHook,
|
||||
OMP_APPROVAL_HOOK_DIR,
|
||||
OMP_APPROVAL_HOOK_FILE,
|
||||
} from '../src/hooks-config.js';
|
||||
import { getCli } from '../src/config/cli-registry/registry.js';
|
||||
import {
|
||||
hooksAvailableForMode,
|
||||
approvalHookAvailableForMode,
|
||||
blockedSignalAvailable,
|
||||
resolveWaitSignals,
|
||||
} from '../src/web/session-wait-registry.js';
|
||||
|
||||
const MARKER = 'CODEMAN_OMP_APPROVAL_HOOK_V1';
|
||||
|
||||
describe('omp approval hook — registry', () => {
|
||||
it('keeps hooks: none, because every consumer reads "always" as Claude\'s full hook set', () => {
|
||||
expect(getCli('omp')?.capabilities.hooks).toBe('none');
|
||||
expect(hooksAvailableForMode('omp')).toBe(false);
|
||||
});
|
||||
|
||||
it('reports approvals through the separate approval-hook capability', () => {
|
||||
expect(approvalHookAvailableForMode('omp')).toBe(true);
|
||||
expect(blockedSignalAvailable('omp')).toBe(true);
|
||||
expect(approvalHookAvailableForMode('claude')).toBe(false);
|
||||
});
|
||||
|
||||
it('answers no where Codeman never installed the module (adopted, remote, Docker)', () => {
|
||||
// sessionHookOptions() sets deepSeekBridgeUnreachable for remote and Docker sessions.
|
||||
expect(approvalHookAvailableForMode('omp', { adopted: true })).toBe(false);
|
||||
expect(approvalHookAvailableForMode('omp', { deepSeekBridgeUnreachable: true })).toBe(false);
|
||||
expect(blockedSignalAvailable('omp', { deepSeekBridgeUnreachable: true })).toBe(false);
|
||||
});
|
||||
|
||||
it('refuses an explicit stop wait for omp (the 400), and accepts blocked', () => {
|
||||
const stop = resolveWaitSignals('stop', { mode: 'omp' });
|
||||
expect(stop.error).toMatch(/stop/);
|
||||
expect(stop.until).toEqual([]);
|
||||
const blocked = resolveWaitSignals('blocked', { mode: 'omp' });
|
||||
expect(blocked.error).toBeNull();
|
||||
expect(blocked.until).toEqual(['blocked']);
|
||||
// The default set must never hold stop for omp, or a bare wait sits out its timeout.
|
||||
expect(resolveWaitSignals(undefined, { mode: 'omp' }).until).not.toContain('stop');
|
||||
});
|
||||
|
||||
it('refuses blocked for a remote omp session, where no module reports', () => {
|
||||
expect(resolveWaitSignals('blocked', { mode: 'omp', deepSeekBridgeUnreachable: true }).error).toMatch(/blocked/);
|
||||
});
|
||||
|
||||
it('leaves every other hook-less CLI failing the ask', () => {
|
||||
// opencode/gemini/pi/grok/antigravity have no approval events.
|
||||
for (const mode of ['opencode', 'gemini', 'pi', 'grok', 'antigravity', 'shell'] as const) {
|
||||
expect(hooksAvailableForMode(mode), mode).toBe(false);
|
||||
expect(approvalHookAvailableForMode(mode), mode).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it('names the hook module as registry data, so no call site branches on the id', () => {
|
||||
// Both create paths ask "does this CLI declare approvalHookFile?"; if the field is
|
||||
// absent the hook is never installed and the pane stays silent.
|
||||
expect(getCli('omp')?.capabilities.approvalHookFile).toBe(OMP_APPROVAL_HOOK_FILE);
|
||||
// claude installs a settings.local.json block instead and must NOT be given a file.
|
||||
expect(getCli('claude')?.capabilities.approvalHookFile).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('omp approval hook — generated module (source)', () => {
|
||||
const src = generateOmpApprovalHook();
|
||||
|
||||
it('carries the ownership marker on its first line', () => {
|
||||
expect(src.split('\n')[0]).toContain(MARKER);
|
||||
});
|
||||
|
||||
it('imports HookAPI from the subpath omp exports it from, type-only', () => {
|
||||
// omp's package root does not re-export it; erased at runtime either way.
|
||||
expect(src).toContain('import type { HookAPI } from "@oh-my-pi/pi-coding-agent/extensibility/hooks";');
|
||||
});
|
||||
|
||||
it('uses a static import, so the module parses without a dynamic loader', () => {
|
||||
expect(src).toContain('import { readFile } from "node:fs/promises";');
|
||||
expect(src).not.toContain('await import(');
|
||||
});
|
||||
});
|
||||
|
||||
/** A fake omp HookAPI: records the handlers the module registers. */
|
||||
function fakeHookApi() {
|
||||
const handlers = new Map<string, (event: Record<string, unknown>) => void>();
|
||||
return {
|
||||
api: { on: (name: string, fn: (event: Record<string, unknown>) => void) => handlers.set(name, fn) },
|
||||
emit: (name: string, event: Record<string, unknown>) => handlers.get(name)?.(event),
|
||||
};
|
||||
}
|
||||
|
||||
const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
|
||||
|
||||
describe('omp approval hook — generated module (runtime)', () => {
|
||||
let dir: string;
|
||||
let fetchMock: ReturnType<typeof vi.fn>;
|
||||
let counter = 0;
|
||||
const savedEnv = { ...process.env };
|
||||
|
||||
/** Write the module to disk and import a FRESH copy (env is read at load time). */
|
||||
async function loadModule(env: Record<string, string>) {
|
||||
Object.assign(process.env, env);
|
||||
const file = join(dir, `codeman-approval-${counter++}.ts`);
|
||||
writeFileSync(file, generateOmpApprovalHook());
|
||||
const mod = (await import(pathToFileURL(file).href)) as { default: (pi: unknown) => void };
|
||||
const pi = fakeHookApi();
|
||||
mod.default(pi.api);
|
||||
return pi;
|
||||
}
|
||||
|
||||
/** The `{ event, sessionId, data }` bodies posted, in order. */
|
||||
const posted = () =>
|
||||
fetchMock.mock.calls.map(
|
||||
([, init]) => JSON.parse((init as { body: string }).body) as { event: string; data: Record<string, unknown> }
|
||||
);
|
||||
|
||||
/**
|
||||
* Wait until the module's report chain has made exactly `calls` POSTs, then a little
|
||||
* longer so a surplus report would show up here instead of leaking into the next test.
|
||||
*/
|
||||
async function settle(calls: number) {
|
||||
await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(calls));
|
||||
await sleep(50);
|
||||
expect(fetchMock).toHaveBeenCalledTimes(calls);
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
dir = mkdtempSync(join(tmpdir(), 'codeman-omp-hook-mod-'));
|
||||
writeFileSync(join(dir, 'secret'), 's3cret\n');
|
||||
fetchMock = vi.fn(async () => new Response('{}'));
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals();
|
||||
process.env = { ...savedEnv };
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
const baseEnv = () => ({
|
||||
CODEMAN_API_URL: 'http://127.0.0.1:3000',
|
||||
CODEMAN_SESSION_ID: 'sess-1',
|
||||
CODEMAN_HOOK_SECRET_FILE: join(dir, 'secret'),
|
||||
});
|
||||
|
||||
it('maps a request to permission_prompt and a resolve to elicitation_complete, with the secret', async () => {
|
||||
const pi = await loadModule(baseEnv());
|
||||
pi.emit('tool_approval_requested', { toolCallId: 'a', toolName: 'bash' });
|
||||
pi.emit('tool_approval_resolved', { toolCallId: 'a', toolName: 'bash', approved: false });
|
||||
await settle(2);
|
||||
|
||||
expect(posted().map((b) => b.event)).toEqual(['permission_prompt', 'elicitation_complete']);
|
||||
const [url, init] = fetchMock.mock.calls[0] as [string, { headers: Record<string, string> }];
|
||||
expect(url).toBe('http://127.0.0.1:3000/api/hook-event');
|
||||
expect(init.headers['X-Codeman-Hook-Secret']).toBe('s3cret');
|
||||
expect(init).not.toHaveProperty('tls');
|
||||
});
|
||||
|
||||
it('clears the alert only when the LAST pending approval in a batch resolves', async () => {
|
||||
const pi = await loadModule(baseEnv());
|
||||
pi.emit('tool_approval_requested', { toolCallId: 'a', toolName: 'bash' });
|
||||
pi.emit('tool_approval_requested', { toolCallId: 'b', toolName: 'write' });
|
||||
pi.emit('tool_approval_resolved', { toolCallId: 'a', toolName: 'bash', approved: true });
|
||||
await settle(2);
|
||||
expect(posted().map((b) => b.event)).toEqual(['permission_prompt', 'permission_prompt']);
|
||||
|
||||
pi.emit('tool_approval_resolved', { toolCallId: 'b', toolName: 'write', approved: true });
|
||||
await settle(3);
|
||||
expect(posted().map((b) => b.event)).toEqual(['permission_prompt', 'permission_prompt', 'elicitation_complete']);
|
||||
});
|
||||
|
||||
it('keeps request-then-resolve order when the first POST is slow', async () => {
|
||||
// Without the chain, the resolve's POST would start while the prompt's is still in
|
||||
// flight and could land first, leaving the server's alert up.
|
||||
const started: string[] = [];
|
||||
fetchMock.mockImplementation(async (_url: string, init: { body: string }) => {
|
||||
const { event } = JSON.parse(init.body) as { event: string };
|
||||
started.push(event);
|
||||
if (event === 'permission_prompt') await sleep(150);
|
||||
return new Response('{}');
|
||||
});
|
||||
const pi = await loadModule(baseEnv());
|
||||
pi.emit('tool_approval_requested', { toolCallId: 'a', toolName: 'bash' });
|
||||
pi.emit('tool_approval_resolved', { toolCallId: 'a', toolName: 'bash', approved: true });
|
||||
await sleep(60);
|
||||
// The resolve has not even started while the prompt is still in flight.
|
||||
expect(started).toEqual(['permission_prompt']);
|
||||
await settle(2);
|
||||
expect(started).toEqual(['permission_prompt', 'elicitation_complete']);
|
||||
});
|
||||
|
||||
it("sends omp's policy reason as message, and nothing summary-shaped without one", async () => {
|
||||
const pi = await loadModule(baseEnv());
|
||||
pi.emit('tool_approval_requested', { toolCallId: 'a', toolName: 'bash', reason: 'restricted folder' });
|
||||
pi.emit('tool_approval_requested', { toolCallId: 'b', toolName: 'bash' });
|
||||
await settle(2);
|
||||
const [withReason, without] = posted();
|
||||
expect(withReason.data).toEqual({ tool_name: 'bash', message: 'restricted folder' });
|
||||
expect(without.data).toEqual({ tool_name: 'bash' });
|
||||
});
|
||||
|
||||
it('skips certificate verification for an https URL (self-signed loopback)', async () => {
|
||||
const pi = await loadModule({ ...baseEnv(), CODEMAN_API_URL: 'https://127.0.0.1:3000' });
|
||||
pi.emit('tool_approval_requested', { toolCallId: 'a', toolName: 'bash' });
|
||||
await settle(1);
|
||||
expect(fetchMock.mock.calls[0][1]).toMatchObject({ tls: { rejectUnauthorized: false } });
|
||||
});
|
||||
|
||||
it('fails open: a rejected fetch never throws out of a handler, and later reports still go', async () => {
|
||||
fetchMock.mockRejectedValueOnce(new Error('ECONNREFUSED'));
|
||||
const pi = await loadModule(baseEnv());
|
||||
expect(() => pi.emit('tool_approval_requested', { toolCallId: 'a', toolName: 'bash' })).not.toThrow();
|
||||
pi.emit('tool_approval_resolved', { toolCallId: 'a', toolName: 'bash', approved: true });
|
||||
await settle(2);
|
||||
});
|
||||
|
||||
it('posts nothing outside a Codeman session', async () => {
|
||||
delete process.env.CODEMAN_API_URL;
|
||||
delete process.env.CODEMAN_SESSION_ID;
|
||||
const pi = await loadModule({});
|
||||
pi.emit('tool_approval_requested', { toolCallId: 'a', toolName: 'bash' });
|
||||
await sleep(100);
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('omp approval hook — install', () => {
|
||||
let workspace: string;
|
||||
|
||||
beforeEach(() => {
|
||||
workspace = mkdtempSync(join(tmpdir(), 'codeman-omp-hook-'));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(workspace, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('installs into .claude/hooks/pre, where omp scans for project hooks', async () => {
|
||||
expect(await applyApprovalHookFile(workspace, OMP_APPROVAL_HOOK_FILE)).toBe('installed');
|
||||
expect(existsSync(join(workspace, OMP_APPROVAL_HOOK_DIR, OMP_APPROVAL_HOOK_FILE))).toBe(true);
|
||||
expect(readFileSync(join(workspace, OMP_APPROVAL_HOOK_DIR, OMP_APPROVAL_HOOK_FILE), 'utf-8')).toBe(
|
||||
generateOmpApprovalHook()
|
||||
);
|
||||
});
|
||||
|
||||
it('is idempotent — a second call rewrites nothing', async () => {
|
||||
await applyApprovalHookFile(workspace, OMP_APPROVAL_HOOK_FILE);
|
||||
expect(await applyApprovalHookFile(workspace, OMP_APPROVAL_HOOK_FILE)).toBe('unchanged');
|
||||
});
|
||||
|
||||
it('refreshes a stale Codeman copy', async () => {
|
||||
await applyApprovalHookFile(workspace, OMP_APPROVAL_HOOK_FILE);
|
||||
writeFileSync(join(workspace, OMP_APPROVAL_HOOK_DIR, OMP_APPROVAL_HOOK_FILE), `// ${MARKER}\n// stale old shape\n`);
|
||||
expect(await applyApprovalHookFile(workspace, OMP_APPROVAL_HOOK_FILE)).toBe('refreshed');
|
||||
expect(readFileSync(join(workspace, OMP_APPROVAL_HOOK_DIR, OMP_APPROVAL_HOOK_FILE), 'utf-8')).toBe(
|
||||
generateOmpApprovalHook()
|
||||
);
|
||||
});
|
||||
|
||||
it("never overwrites another project's hook of the same name", async () => {
|
||||
mkdirSync(join(workspace, OMP_APPROVAL_HOOK_DIR), { recursive: true });
|
||||
const foreign = 'export default function mine() {}\n';
|
||||
writeFileSync(join(workspace, OMP_APPROVAL_HOOK_DIR, OMP_APPROVAL_HOOK_FILE), foreign);
|
||||
expect(await applyApprovalHookFile(workspace, OMP_APPROVAL_HOOK_FILE)).toBe('foreign');
|
||||
expect(readFileSync(join(workspace, OMP_APPROVAL_HOOK_DIR, OMP_APPROVAL_HOOK_FILE), 'utf-8')).toBe(foreign);
|
||||
});
|
||||
|
||||
// Every component below the workspace, and the file itself: a link at any level
|
||||
// would put the module wherever it points.
|
||||
it.each([
|
||||
['.claude', '.claude'],
|
||||
['.claude/hooks', join('.claude', 'hooks')],
|
||||
['.claude/hooks/pre', OMP_APPROVAL_HOOK_DIR],
|
||||
])('refuses to write through a symlinked %s', async (_label, linkRel) => {
|
||||
const elsewhere = mkdtempSync(join(tmpdir(), 'codeman-omp-hook-target-'));
|
||||
try {
|
||||
const link = join(workspace, linkRel);
|
||||
mkdirSync(join(link, '..'), { recursive: true });
|
||||
symlinkSync(elsewhere, link);
|
||||
expect(await applyApprovalHookFile(workspace, OMP_APPROVAL_HOOK_FILE)).toBe('symlink');
|
||||
// Nothing written anywhere under the link target.
|
||||
expect(existsSync(join(elsewhere, 'pre'))).toBe(false);
|
||||
expect(existsSync(join(elsewhere, 'hooks'))).toBe(false);
|
||||
expect(existsSync(join(elsewhere, OMP_APPROVAL_HOOK_FILE))).toBe(false);
|
||||
} finally {
|
||||
rmSync(elsewhere, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('refuses a symlinked target file', async () => {
|
||||
const elsewhere = mkdtempSync(join(tmpdir(), 'codeman-omp-hook-target-'));
|
||||
try {
|
||||
const outside = join(elsewhere, 'victim.ts');
|
||||
writeFileSync(outside, `// ${MARKER}\n`);
|
||||
mkdirSync(join(workspace, OMP_APPROVAL_HOOK_DIR), { recursive: true });
|
||||
symlinkSync(outside, join(workspace, OMP_APPROVAL_HOOK_DIR, OMP_APPROVAL_HOOK_FILE));
|
||||
expect(await applyApprovalHookFile(workspace, OMP_APPROVAL_HOOK_FILE)).toBe('symlink');
|
||||
expect(readFileSync(outside, 'utf-8')).toBe(`// ${MARKER}\n`);
|
||||
} finally {
|
||||
rmSync(elsewhere, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -13,6 +13,7 @@ import fastifyCookie from '@fastify/cookie';
|
||||
import { registerApprovalRoutes } from '../../src/web/routes/approval-routes.js';
|
||||
import { registerHookEventRoutes } from '../../src/web/routes/hook-event-routes.js';
|
||||
import { approvalInbox } from '../../src/web/approval-inbox.js';
|
||||
import { sessionWaits } from '../../src/web/session-wait-registry.js';
|
||||
import { raiseScreenBlocked } from '../../src/web/screen-blocked.js';
|
||||
import { installRouteErrorHandler } from '../../src/web/route-error-handler.js';
|
||||
import { httpStatusForErrorCode, type ApiErrorCode } from '../../src/types.js';
|
||||
@@ -522,6 +523,85 @@ describe('approval routes: supervised-hook (DeepSeek) alert-only items', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('approval routes: approval-only hook module (omp) alert-only items', () => {
|
||||
let harness: RouteTestHarness;
|
||||
let session: MockSession;
|
||||
|
||||
beforeEach(async () => {
|
||||
harness = await createEnvelopeHarness();
|
||||
session = harness.ctx.sessions.get(SESSION_ID)!;
|
||||
session.mode = 'omp';
|
||||
// A Claude-shaped frame on purpose: even when the capture parses into options, an
|
||||
// omp item must stay alert-only, because omp's composer takes the keystroke.
|
||||
session.terminalBuffer = PERMISSION_DIALOG;
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
vi.restoreAllMocks();
|
||||
for (const item of approvalInbox.listPending()) {
|
||||
approvalInbox.resolveForSession(item.sessionId, 'dismissed');
|
||||
}
|
||||
approvalInbox.onPending = approvalInbox.onUpdated = approvalInbox.onResolved = undefined;
|
||||
await harness.app.close();
|
||||
});
|
||||
|
||||
it('creates an answerInTerminal item and omits approvalId from push', async () => {
|
||||
await postHook(harness, 'permission_prompt', { tool_name: 'bash', message: 'restricted folder' });
|
||||
const approvals = await listApprovals(harness);
|
||||
expect(approvals).toHaveLength(1);
|
||||
expect(approvals[0]).toMatchObject({
|
||||
sessionId: SESSION_ID,
|
||||
kind: 'permission',
|
||||
toolName: 'bash',
|
||||
message: 'restricted folder',
|
||||
answerInTerminal: true,
|
||||
});
|
||||
const push = harness.ctx.sendPushNotifications.mock.calls.find((c) => c[0] === 'hook:permission_prompt');
|
||||
expect(push?.[1]).toBeDefined();
|
||||
expect(push?.[1]).not.toHaveProperty('approvalId');
|
||||
});
|
||||
|
||||
it('refuses Approve with a terminal-only error and writes nothing into the pane', async () => {
|
||||
await postHook(harness, 'permission_prompt', { tool_name: 'bash' });
|
||||
const [item] = await listApprovals(harness);
|
||||
for (const payload of [{ action: 'approve' }, { action: 'deny' }, { action: 'option', option: 1 }]) {
|
||||
const res = await harness.app.inject({ method: 'POST', url: `/api/approvals/${item.id}/answer`, payload });
|
||||
expect(res.statusCode).toBe(400);
|
||||
expect(res.json().error).toMatch(/answered in the terminal/i);
|
||||
}
|
||||
expect(session.writeBuffer).toEqual([]);
|
||||
expect(await listApprovals(harness)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('resolves the item on elicitation_complete', async () => {
|
||||
await postHook(harness, 'permission_prompt', { tool_name: 'bash' });
|
||||
await postHook(harness, 'elicitation_complete', { tool_name: 'bash', approved: true });
|
||||
expect(await listApprovals(harness)).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('wakes a blocked waiter, and never a stop waiter', async () => {
|
||||
const notify = vi.spyOn(sessionWaits, 'notifySignal');
|
||||
await postHook(harness, 'permission_prompt', { tool_name: 'bash' });
|
||||
await postHook(harness, 'stop', {});
|
||||
expect(notify.mock.calls).toEqual([[SESSION_ID, 'blocked']]);
|
||||
});
|
||||
|
||||
it('drops events the module never sends (stop, idle_prompt) instead of opening items', async () => {
|
||||
await postHook(harness, 'idle_prompt', {});
|
||||
expect(await listApprovals(harness)).toHaveLength(0);
|
||||
await postHook(harness, 'permission_prompt', { tool_name: 'bash' });
|
||||
// A forged stop must not clear a real pending dialog either.
|
||||
await postHook(harness, 'stop', {});
|
||||
expect(await listApprovals(harness)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('opens nothing for a remote omp session, where no module was installed', async () => {
|
||||
(session as unknown as { remote: unknown }).remote = { host: 'box' };
|
||||
await postHook(harness, 'permission_prompt', { tool_name: 'bash' });
|
||||
expect(await listApprovals(harness)).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('approval routes: dialogs read off a hook-less screen (codex) are alert-only', () => {
|
||||
let harness: RouteTestHarness;
|
||||
let session: MockSession;
|
||||
|
||||
@@ -0,0 +1,177 @@
|
||||
/**
|
||||
* @fileoverview The omp approval hook module on BOTH session create paths.
|
||||
*
|
||||
* `POST /api/quick-start` is how the Run button, the agent skill's `spawn_worker` and
|
||||
* `codeman agent spawn` start omp, so installing only from `POST /api/sessions` left the
|
||||
* usual omp launch silent. These tests pin, on disk:
|
||||
* - both paths write `.claude/hooks/pre/codeman-approval.ts` into a local workspace;
|
||||
* - neither writes Claude's `.claude/settings.local.json` hook block for omp (only
|
||||
* claude reads it) or, for a Docker case, a CLAUDE.md;
|
||||
* - `workspaceHooksEnabled: false` writes nothing;
|
||||
* - a create refused by the launch pre-flight leaves nothing behind.
|
||||
*
|
||||
* The launch pre-flight is mocked so the result does not depend on whether omp is
|
||||
* installed on the machine running the suite. Uses app.inject(), no port.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
import Fastify, { type FastifyInstance } from 'fastify';
|
||||
import fastifyCookie from '@fastify/cookie';
|
||||
import { mkdtemp, rm, mkdir, writeFile } from 'node:fs/promises';
|
||||
import { existsSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { createMockRouteContext, safeRmHomeTree } from '../mocks/index.js';
|
||||
import { installRouteErrorHandler } from '../../src/web/route-error-handler.js';
|
||||
import { registerSessionRoutes } from '../../src/web/routes/session-routes.js';
|
||||
import { OMP_APPROVAL_HOOK_DIR, OMP_APPROVAL_HOOK_FILE } from '../../src/hooks-config.js';
|
||||
import { getDataDir } from '../../src/config/instance.js';
|
||||
import { CASES_DIR } from '../../src/web/route-helpers.js';
|
||||
import { Session } from '../../src/session.js';
|
||||
|
||||
const launch = vi.hoisted(() => ({ error: null as string | null }));
|
||||
vi.mock('../../src/utils/cli-launcher.js', async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import('../../src/utils/cli-launcher.js')>()),
|
||||
resolveCliLaunchError: vi.fn(async () => launch.error),
|
||||
}));
|
||||
|
||||
const hookFileIn = (dir: string) => join(dir, OMP_APPROVAL_HOOK_DIR, OMP_APPROVAL_HOOK_FILE);
|
||||
const claudeSettingsIn = (dir: string) => join(dir, '.claude', 'settings.local.json');
|
||||
|
||||
/**
|
||||
* A create that SUCCEEDED. The status alone cannot say so: `createErrorResponse()`
|
||||
* envelopes come back as 200 here (no preSerialization hook), so an early refusal would
|
||||
* make every "wrote nothing" assertion below pass vacuously. A success body is the raw
|
||||
* payload (no envelope), so the proof is a real session id in it.
|
||||
*/
|
||||
function expectCreated(res: { statusCode: number; body: string }) {
|
||||
expect(res.statusCode).toBe(200);
|
||||
const body = JSON.parse(res.body) as { success?: boolean; session?: { id?: string }; sessionId?: string };
|
||||
expect(body.success, res.body).not.toBe(false);
|
||||
expect(body.session?.id ?? body.sessionId, res.body).toBeTruthy();
|
||||
}
|
||||
|
||||
describe('omp approval hook on the create paths', () => {
|
||||
let app: FastifyInstance;
|
||||
|
||||
const useApp = async (workspaceHooksEnabled = true) => {
|
||||
await app?.close();
|
||||
app = Fastify({ logger: false });
|
||||
await app.register(fastifyCookie);
|
||||
registerSessionRoutes(app, createMockRouteContext({ workspaceHooksEnabled }));
|
||||
installRouteErrorHandler(app);
|
||||
await app.ready();
|
||||
};
|
||||
|
||||
beforeEach(async () => {
|
||||
launch.error = null;
|
||||
vi.spyOn(Session.prototype, 'startInteractive').mockResolvedValue(undefined);
|
||||
vi.spyOn(Session.prototype, 'startShell').mockResolvedValue(undefined);
|
||||
await useApp();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await app.close();
|
||||
vi.restoreAllMocks();
|
||||
await rm(join(getDataDir(), 'docker-hosts.json'), { force: true });
|
||||
await rm(join(getDataDir(), 'docker-cases.json'), { force: true });
|
||||
// CASES_DIR sits under the (redirected) test HOME; safeRmHomeTree refuses anything else.
|
||||
safeRmHomeTree(CASES_DIR);
|
||||
});
|
||||
|
||||
describe('POST /api/sessions', () => {
|
||||
let workingDir: string;
|
||||
const createSession = (payload: Record<string, unknown>) =>
|
||||
app.inject({ method: 'POST', url: '/api/sessions', payload });
|
||||
|
||||
beforeEach(async () => {
|
||||
workingDir = await mkdtemp(join(tmpdir(), 'codeman-omp-create-'));
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await rm(workingDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('installs the module, and no Claude hook block', async () => {
|
||||
expectCreated(await createSession({ name: 'omp-create', mode: 'omp', workingDir }));
|
||||
expect(existsSync(hookFileIn(workingDir))).toBe(true);
|
||||
expect(existsSync(claudeSettingsIn(workingDir))).toBe(false);
|
||||
});
|
||||
|
||||
it('writes nothing when workspaceHooksEnabled is off', async () => {
|
||||
await useApp(false);
|
||||
expectCreated(await createSession({ name: 'omp-off', mode: 'omp', workingDir }));
|
||||
expect(existsSync(join(workingDir, '.claude'))).toBe(false);
|
||||
});
|
||||
|
||||
it('leaves nothing behind when the launch pre-flight refuses the create', async () => {
|
||||
launch.error = 'omp: command not found';
|
||||
const res = await createSession({ name: 'omp-missing', mode: 'omp', workingDir });
|
||||
expect(res.json().success).toBe(false);
|
||||
expect(existsSync(join(workingDir, '.claude'))).toBe(false);
|
||||
});
|
||||
|
||||
it('gives claude no module', async () => {
|
||||
expectCreated(await createSession({ name: 'claude-create', mode: 'claude', workingDir }));
|
||||
expect(existsSync(hookFileIn(workingDir))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('POST /api/quick-start', () => {
|
||||
const quickStart = (payload: Record<string, unknown>) =>
|
||||
app.inject({ method: 'POST', url: '/api/quick-start', payload });
|
||||
|
||||
it('installs the module into an EXISTING case, and no Claude hook block', async () => {
|
||||
const casePath = join(CASES_DIR, 'ompexisting');
|
||||
await mkdir(casePath, { recursive: true });
|
||||
expectCreated(await quickStart({ caseName: 'ompexisting', mode: 'omp' }));
|
||||
expect(existsSync(hookFileIn(casePath))).toBe(true);
|
||||
expect(existsSync(claudeSettingsIn(casePath))).toBe(false);
|
||||
});
|
||||
|
||||
it('installs the module into a case it scaffolds, and no Claude hook block', async () => {
|
||||
expectCreated(await quickStart({ caseName: 'ompnew', mode: 'omp' }));
|
||||
const casePath = join(CASES_DIR, 'ompnew');
|
||||
expect(existsSync(hookFileIn(casePath))).toBe(true);
|
||||
expect(existsSync(claudeSettingsIn(casePath))).toBe(false);
|
||||
});
|
||||
|
||||
it('writes nothing into an existing case when workspaceHooksEnabled is off', async () => {
|
||||
await useApp(false);
|
||||
const casePath = join(CASES_DIR, 'ompoff');
|
||||
await mkdir(casePath, { recursive: true });
|
||||
expectCreated(await quickStart({ caseName: 'ompoff', mode: 'omp' }));
|
||||
expect(existsSync(join(casePath, '.claude'))).toBe(false);
|
||||
});
|
||||
|
||||
it('leaves nothing behind when the launch pre-flight refuses', async () => {
|
||||
launch.error = 'omp: command not found';
|
||||
const casePath = join(CASES_DIR, 'ompmissing');
|
||||
await mkdir(casePath, { recursive: true });
|
||||
const res = await quickStart({ caseName: 'ompmissing', mode: 'omp' });
|
||||
expect(res.json().success).toBe(false);
|
||||
expect(existsSync(join(casePath, '.claude'))).toBe(false);
|
||||
});
|
||||
|
||||
it('writes neither the module nor a Claude block (or CLAUDE.md) into a Docker case', async () => {
|
||||
const ws = await mkdtemp(join(tmpdir(), 'codeman-docker-omp-'));
|
||||
try {
|
||||
await mkdir(getDataDir(), { recursive: true });
|
||||
await writeFile(
|
||||
join(getDataDir(), 'docker-hosts.json'),
|
||||
JSON.stringify([{ id: 'd1', label: 'box', image: 'codeman/agent:base' }])
|
||||
);
|
||||
await writeFile(
|
||||
join(getDataDir(), 'docker-cases.json'),
|
||||
JSON.stringify([{ name: 'dockomp', type: 'docker', hostId: 'd1', hostWorkspacePath: ws }])
|
||||
);
|
||||
expectCreated(await quickStart({ caseName: 'dockomp', mode: 'omp' }));
|
||||
expect(existsSync(hookFileIn(ws))).toBe(false);
|
||||
expect(existsSync(claudeSettingsIn(ws))).toBe(false);
|
||||
expect(existsSync(join(ws, 'CLAUDE.md'))).toBe(false);
|
||||
} finally {
|
||||
await rm(ws, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user