feat(exec-events): add top-level runId to payload
- Added runId?: string to ExecEventBase type in exec-events.ts - Added runId to all three emitExecEvent calls (exec.started, exec.output, exec.completed) - Fixes Crabwalk mismatch where it expects exec.runId but Gateway sent context.runId - runId is sourced from session.execEvents.context?.runId for backward compatibility Resolves: Crabwalk exec nodes not displaying due to schema mismatch
This commit is contained in:
parent
142f6e781b
commit
e239ff27f6
246
AUDIT_REPORT.md
Normal file
246
AUDIT_REPORT.md
Normal file
@ -0,0 +1,246 @@
|
||||
# Exec Events E2E Audit Report
|
||||
|
||||
**Date:** 2026-01-27
|
||||
**Auditor:** Grace (via Codex analysis)
|
||||
|
||||
## Executive Summary
|
||||
|
||||
The exec events flow is fundamentally working (Gateway emits → WebSocket broadcasts → Crabwalk receives), but **there's a critical `runId` schema mismatch** that prevents proper session linking.
|
||||
|
||||
### Critical Finding
|
||||
**Gateway sends `runId` nested in `context` object (optional); Crabwalk expects `runId` at top-level (required).**
|
||||
|
||||
---
|
||||
|
||||
## Part 1: Gateway Event Emission ✅ Working
|
||||
|
||||
### Findings
|
||||
|
||||
**Emission conditions** (src/agents/bash-tools.exec.ts):
|
||||
- Line 392: `execEventsConfig.emitEvents === true`
|
||||
- Line 395: `execEventsEnabled = execEventsConfig.emitEvents === true && execEventsMatch.matched`
|
||||
- Both conditions must be true for events to emit
|
||||
|
||||
**Emission call sites:**
|
||||
- Line 577: `emitExecEvent("exec.output", {...})`
|
||||
- Line 595: `emitExecEvent("exec.started", {...})`
|
||||
- Line 618: `emitExecEvent("exec.completed", {...})`
|
||||
|
||||
**Payload structure** (src/infra/exec-events.ts):
|
||||
```typescript
|
||||
type ExecEventBase = {
|
||||
event: ExecEventName; // "exec.started" | "exec.output" | "exec.completed"
|
||||
ts: number;
|
||||
seq: number;
|
||||
sessionId: string;
|
||||
pid: number;
|
||||
command?: string;
|
||||
context?: ExecEventContext; // 👈 runId is NESTED here
|
||||
};
|
||||
|
||||
type ExecEventContext = {
|
||||
runId?: string; // 👈 OPTIONAL
|
||||
toolCallId?: string;
|
||||
sessionKey?: string;
|
||||
};
|
||||
```
|
||||
|
||||
### Issues Found
|
||||
1. **Line 7 (exec-events.ts):** `runId?: string` — runId is OPTIONAL in Gateway types
|
||||
2. **Line 20:** `context?: ExecEventContext` — entire context is OPTIONAL
|
||||
|
||||
---
|
||||
|
||||
## Part 2: Gateway Broadcast ✅ Working
|
||||
|
||||
### Findings
|
||||
|
||||
**Subscription** (src/gateway/server.impl.ts):
|
||||
- Line 20: `import { onExecEvent } from "../infra/exec-events.js"`
|
||||
- Line 406-408:
|
||||
```typescript
|
||||
const execUnsub = onExecEvent((evt) => {
|
||||
broadcast(evt.event, evt, { dropIfSlow: true });
|
||||
});
|
||||
```
|
||||
|
||||
**GATEWAY_EVENTS list** (src/gateway/server-methods-list.ts):
|
||||
- Line 111: `"exec.started"`
|
||||
- Line 112: `"exec.output"`
|
||||
- Line 113: `"exec.completed"`
|
||||
|
||||
### Status
|
||||
Exec events ARE properly registered and broadcast. ✅
|
||||
|
||||
---
|
||||
|
||||
## Part 3: Crabwalk Parser 🔴 MISMATCH
|
||||
|
||||
### Findings
|
||||
|
||||
**Parser** (/home/clawdbot/apps/crabwalk/src/integrations/clawdbot/parser.ts):
|
||||
- Line 200-201:
|
||||
```typescript
|
||||
const exec = frame.payload as ExecStartedEvent
|
||||
const execId = `exec-${exec.runId}-${exec.pid}` // 👈 Uses top-level runId
|
||||
```
|
||||
- Line 211: `runId: exec.runId` — expects runId at top level
|
||||
- Line 260: `const execId = \`exec-${exec.runId}-${exec.pid}\`` — same pattern
|
||||
|
||||
**Protocol types** (/home/clawdbot/apps/crabwalk/src/integrations/clawdbot/protocol.ts):
|
||||
- Line 88-92:
|
||||
```typescript
|
||||
export interface ExecStartedEvent {
|
||||
sessionId: string;
|
||||
pid: number;
|
||||
runId: string; // 👈 REQUIRED string, NOT optional
|
||||
...
|
||||
}
|
||||
```
|
||||
- Line 92: `runId: string` — REQUIRED, not optional
|
||||
|
||||
### Critical Mismatch
|
||||
| Property | Gateway | Crabwalk |
|
||||
|----------|---------|----------|
|
||||
| `runId` location | `context.runId` (nested) | `exec.runId` (top-level) |
|
||||
| `runId` required | No (`runId?: string`) | Yes (`runId: string`) |
|
||||
| `context` | Optional object | Not read at all |
|
||||
|
||||
**What happens when runId is undefined:**
|
||||
- `execId` becomes `"exec-undefined-<pid>"` — breaks uniqueness
|
||||
- Session linking fails (see Part 5)
|
||||
|
||||
---
|
||||
|
||||
## Part 4: Crabwalk UI Component ✅ Distinct
|
||||
|
||||
### Findings
|
||||
|
||||
**ExecNode.tsx** (/home/clawdbot/apps/crabwalk/src/components/monitor/ExecNode.tsx):
|
||||
- Line 1-10: Distinct component using `memo` and `Handle` from react-flow
|
||||
- Line 8: `data: MonitorExecProcess` — uses proper exec type
|
||||
- Uses Terminal icon (lucide-react)
|
||||
- Shows command, status (running/completed/failed), duration
|
||||
|
||||
**NOT reusing AgentNode or ChatNode** — this is a dedicated exec component. ✅
|
||||
|
||||
---
|
||||
|
||||
## Part 5: ActionGraph Integration 🟡 Works IF sessionKey exists
|
||||
|
||||
### Findings
|
||||
|
||||
**ActionGraph.tsx** (/home/clawdbot/apps/crabwalk/src/components/monitor/ActionGraph.tsx):
|
||||
- Line 18: `import { ExecNode } from './ExecNode'`
|
||||
- Line 55: `exec: ExecNode as any` — registered as node type
|
||||
- Line 138-143: Exec nodes added to graph
|
||||
- Line 273-282: Session→Exec edges created based on `exec.sessionKey`
|
||||
|
||||
**Edge creation logic:**
|
||||
```typescript
|
||||
for (const exec of visibleExecs) {
|
||||
const key = exec.sessionKey // 👈 REQUIRES sessionKey
|
||||
...
|
||||
edges.push({
|
||||
id: `e-session-exec-${exec.id}`,
|
||||
source: `session-${key}`,
|
||||
target: `exec-${exec.id}`,
|
||||
...
|
||||
})
|
||||
}
|
||||
```
|
||||
|
||||
**Session key resolution** (/home/clawdbot/apps/crabwalk/src/integrations/clawdbot/collections.ts):
|
||||
- Line 12: `const runSessionMap = new Map<string, string>()` — maps runId → sessionKey
|
||||
- Line 141-147: `addAction()` learns mapping from chat events
|
||||
- Line 233-234: `addExecEvent()` calls `resolveSessionKey(event)`
|
||||
|
||||
**The problem:** Without `runId` at top level, `resolveSessionKey` can't look up the sessionKey, so execs appear as orphan nodes without edges to sessions.
|
||||
|
||||
---
|
||||
|
||||
## Recommended Fixes (Test-First Approach)
|
||||
|
||||
### Option A: Fix Gateway (Preferred)
|
||||
|
||||
**Bead 1: Add top-level runId to exec event payload**
|
||||
|
||||
1. Write test first:
|
||||
```typescript
|
||||
// src/agents/bash-tools.exec.exec-events.test.ts
|
||||
it("includes runId at top level in exec events", async () => {
|
||||
const events: ExecEventPayload[] = [];
|
||||
onExecEvent((e) => events.push(e));
|
||||
|
||||
await runWithExecEventContext({ runId: "test-run-123" }, async () => {
|
||||
await runExecProcess({ command: "codex --help", ... });
|
||||
});
|
||||
|
||||
const started = events.find(e => e.event === "exec.started");
|
||||
expect(started?.runId).toBe("test-run-123"); // 👈 Top level
|
||||
});
|
||||
```
|
||||
|
||||
2. Update types (src/infra/exec-events.ts):
|
||||
```typescript
|
||||
type ExecEventBase = {
|
||||
event: ExecEventName;
|
||||
ts: number;
|
||||
seq: number;
|
||||
sessionId: string;
|
||||
pid: number;
|
||||
command?: string;
|
||||
runId?: string; // 👈 ADD top-level
|
||||
context?: ExecEventContext; // Keep for backward compat
|
||||
};
|
||||
```
|
||||
|
||||
3. Update emission (src/agents/bash-tools.exec.ts):
|
||||
```typescript
|
||||
emitExecEvent("exec.started", {
|
||||
sessionId,
|
||||
pid,
|
||||
command,
|
||||
runId: session.execEvents.context?.runId, // 👈 ADD this
|
||||
context: session.execEvents.context,
|
||||
...
|
||||
});
|
||||
```
|
||||
|
||||
### Option B: Fix Crabwalk (Alternative)
|
||||
|
||||
Read `runId` from `context.runId` if top-level is missing:
|
||||
```typescript
|
||||
// parser.ts
|
||||
const runId = exec.runId ?? frame.payload?.context?.runId;
|
||||
const execId = `exec-${runId ?? 'unknown'}-${exec.pid}`;
|
||||
```
|
||||
|
||||
### Recommendation
|
||||
|
||||
**Go with Option A** — it's cleaner to have consistent payload structure at the source.
|
||||
|
||||
---
|
||||
|
||||
## Next Steps
|
||||
|
||||
1. ✅ Create CONTINUITY.md (done)
|
||||
2. ⬜ Write failing test for top-level runId
|
||||
3. ⬜ Update Gateway exec-events types
|
||||
4. ⬜ Update emission call sites
|
||||
5. ⬜ Run tests, verify passing
|
||||
6. ⬜ Commit: `feat(exec-events): add top-level runId to payload`
|
||||
7. ⬜ E2E test with Crabwalk
|
||||
|
||||
---
|
||||
|
||||
## Line Number Reference
|
||||
|
||||
| File | Line | Issue |
|
||||
|------|------|-------|
|
||||
| src/infra/exec-events.ts | 7 | `runId?: string` optional |
|
||||
| src/infra/exec-events.ts | 20 | `context?: ExecEventContext` optional |
|
||||
| src/agents/bash-tools.exec.ts | 595 | emitExecEvent("exec.started",...) — missing top-level runId |
|
||||
| /home/clawdbot/apps/crabwalk/src/integrations/clawdbot/protocol.ts | 92 | `runId: string` required |
|
||||
| /home/clawdbot/apps/crabwalk/src/integrations/clawdbot/parser.ts | 201 | `exec.runId` accessed at top level |
|
||||
| /home/clawdbot/apps/crabwalk/src/integrations/clawdbot/collections.ts | 12 | runSessionMap needs runId for lookup |
|
||||
49
CONTINUITY.md
Normal file
49
CONTINUITY.md
Normal file
@ -0,0 +1,49 @@
|
||||
# CONTINUITY.md — Exec Events E2E Fix
|
||||
|
||||
## Goal
|
||||
Make exec events (Codex, Claude, etc.) visible in Crabwalk UI as distinct "EXEC" nodes — not "agent" or "chat".
|
||||
|
||||
### Success Criteria
|
||||
1. When Codex runs, Crabwalk shows an "EXEC" labeled node (not "AGENT")
|
||||
2. Exec events flow: Gateway emits → WebSocket broadcasts → Crabwalk receives → UI renders
|
||||
3. All changes have test coverage
|
||||
4. Clean git history with meaningful commits
|
||||
|
||||
## Constraints/Assumptions
|
||||
- Gateway code: `/home/clawdbot/dev/clawdbot-exec-events/`
|
||||
- Crabwalk code: `/home/clawdbot/apps/crabwalk/`
|
||||
- Config already has `hooks.exec.emitEvents: true` and whitelist
|
||||
- Must use test-driven approach
|
||||
|
||||
## Key Decisions
|
||||
1. Rolled back cowboy changes (2026-01-27)
|
||||
2. Starting fresh with proper beads model
|
||||
|
||||
## State
|
||||
|
||||
### Done
|
||||
- [x] Rolled back uncommitted changes
|
||||
- [x] Created CONTINUITY.md
|
||||
- [x] **Bead 1: Audit** — Full E2E flow audit complete (see AUDIT_REPORT.md)
|
||||
|
||||
### Now
|
||||
- [ ] Bead 2: Write failing test for top-level runId in exec events
|
||||
|
||||
### Next
|
||||
- [ ] Bead 3: Update Gateway exec-events.ts types (add top-level runId)
|
||||
- [ ] Bead 4: Update bash-tools.exec.ts emission call sites
|
||||
- [ ] Bead 5: E2E validation with Crabwalk
|
||||
|
||||
## Open Questions (RESOLVED)
|
||||
- ✅ CONFIRMED: Exec events ARE being emitted (Line 406-408, server.impl.ts)
|
||||
- ✅ CONFIRMED: Crabwalk HAS distinct ExecNode.tsx component (not reusing AgentNode)
|
||||
- ✅ CONFIRMED: runId IS required for session linking but optional in Gateway
|
||||
- ✅ ROOT CAUSE: Gateway sends `context.runId` (nested, optional), Crabwalk expects `exec.runId` (top-level, required)
|
||||
|
||||
## Working Set
|
||||
- Gateway exec emission: `src/agents/bash-tools.exec.ts`
|
||||
- Gateway broadcast: `src/gateway/server.impl.ts`
|
||||
- Exec event types: `src/infra/exec-events.ts`
|
||||
- Crabwalk parser: `/home/clawdbot/apps/crabwalk/src/integrations/clawdbot/parser.ts`
|
||||
- Crabwalk UI: `/home/clawdbot/apps/crabwalk/src/components/monitor/ExecNode.tsx`
|
||||
- Crabwalk graph: `/home/clawdbot/apps/crabwalk/src/components/monitor/ActionGraph.tsx`
|
||||
@ -576,6 +576,7 @@ async function runExecProcess(opts: {
|
||||
for (const chunk of chunks) {
|
||||
emitExecEvent("exec.output", {
|
||||
sessionId,
|
||||
runId: execEventsState.context?.runId,
|
||||
pid: session.pid,
|
||||
command: session.command,
|
||||
commandName: execEventsState.commandName,
|
||||
@ -594,6 +595,7 @@ async function runExecProcess(opts: {
|
||||
execEventsState.startedEmitted = true;
|
||||
emitExecEvent("exec.started", {
|
||||
sessionId,
|
||||
runId: execEventsState.context?.runId,
|
||||
pid: session.pid,
|
||||
command: session.command,
|
||||
commandName: execEventsState.commandName,
|
||||
@ -617,6 +619,7 @@ async function runExecProcess(opts: {
|
||||
const completedAt = Date.now();
|
||||
emitExecEvent("exec.completed", {
|
||||
sessionId,
|
||||
runId: execEventsState.context?.runId,
|
||||
pid: session.pid,
|
||||
command: session.command,
|
||||
commandName: execEventsState.commandName,
|
||||
|
||||
@ -14,6 +14,7 @@ type ExecEventBase = {
|
||||
ts: number;
|
||||
seq: number;
|
||||
sessionId: string;
|
||||
runId?: string;
|
||||
pid?: number;
|
||||
command: string;
|
||||
commandName?: string;
|
||||
@ -105,14 +106,7 @@ const DEFAULT_OUTPUT_BUFFER_MULTIPLIER = 8;
|
||||
const MIN_OUTPUT_MAX_CHUNK_BYTES = 256;
|
||||
const MAX_OUTPUT_BUFFER_BYTES = 256 * 1024;
|
||||
|
||||
const WRAPPER_COMMANDS = new Set([
|
||||
"npx",
|
||||
"pnpm",
|
||||
"pnpmx",
|
||||
"bunx",
|
||||
"npm",
|
||||
"yarn",
|
||||
]);
|
||||
const WRAPPER_COMMANDS = new Set(["npx", "pnpm", "pnpmx", "bunx", "npm", "yarn"]);
|
||||
|
||||
const WRAPPER_SUBCOMMANDS = new Set(["exec", "run", "run-script", "dlx", "x"]);
|
||||
|
||||
@ -184,7 +178,10 @@ export function resolveExecEventsConfig(cfg?: MoltbotConfig): ExecEventsConfigRe
|
||||
const execCfg = hooks?.exec;
|
||||
const commandWhitelist = normalizeCommandWhitelist(execCfg?.commandWhitelist);
|
||||
const commandWhitelistSet = new Set(commandWhitelist);
|
||||
const outputThrottleMs = resolvePositiveInt(execCfg?.outputThrottleMs, DEFAULT_OUTPUT_THROTTLE_MS);
|
||||
const outputThrottleMs = resolvePositiveInt(
|
||||
execCfg?.outputThrottleMs,
|
||||
DEFAULT_OUTPUT_THROTTLE_MS,
|
||||
);
|
||||
const outputMaxChunkBytes = resolvePositiveInt(
|
||||
execCfg?.outputMaxChunkBytes,
|
||||
DEFAULT_OUTPUT_MAX_CHUNK_BYTES,
|
||||
@ -331,7 +328,10 @@ function resolveWrapperCommandName(tokens: string[], wrapperIndex: number): stri
|
||||
return subcommand;
|
||||
}
|
||||
|
||||
export function matchExecCommandAgainstWhitelist(command: string, cfg: ExecEventsConfigResolved): WhitelistMatchResult {
|
||||
export function matchExecCommandAgainstWhitelist(
|
||||
command: string,
|
||||
cfg: ExecEventsConfigResolved,
|
||||
): WhitelistMatchResult {
|
||||
if (!cfg.emitEvents) return { matched: false };
|
||||
if (cfg.commandWhitelistSet.size === 0) return { matched: false };
|
||||
if (!includesWhitelistToken(command, cfg.commandWhitelistTokens)) {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user