Skip to content

A1: propagate taskId through pilo server and core#389

Merged
MrTravisB merged 2 commits intomainfrom
travis/pilo-task-id
Apr 27, 2026
Merged

A1: propagate taskId through pilo server and core#389
MrTravisB merged 2 commits intomainfrom
travis/pilo-task-id

Conversation

@MrTravisB
Copy link
Copy Markdown
Collaborator

Summary

First PR in Stack A of the Pilo reliability plan. Adds a server-generated
taskId (UUID v4) that threads through every task submitted via
POST /pilo/run (SSE) or the WebSocket /pilo/run endpoint.

The taskId:

  • Is exposed as an x-pilo-task-id HTTP response header on every response
    (success, validation error, setup error) so callers like tabs-api can log
    the taskId with the request in their access log without parsing the SSE body
  • Is returned in the SSE start event and a new WebSocket task:accepted event
  • Is echoed in every error response (validation, setup, task-execution) as an
    optional error.taskId field
  • Threads through runTask into WebAgent options and is set as the
    pilo.task.id OTel span attribute on the root task span

Purely observational. No behavior change. Additive only: the new
error.taskId field is optional, the new task:accepted WS event and
x-pilo-task-id header are new, so existing clients are unaffected. Unblocks
downstream PRs that need per-task correlation in logs, metrics, and traces.

Changes

  • packages/core/src/webAgent.ts - optional taskId on WebAgentOptions,
    set as pilo.task.id span attribute
  • packages/server/src/taskRunner.ts - optional taskId on
    TaskRunnerOptions and ErrorResponse.error;
    createErrorResponse(message, code, taskId?) signature
  • packages/server/src/routes/pilo.ts - generate taskId at request entry,
    set x-pilo-task-id response header, include in start event and all
    error responses, pass to runTask
  • packages/server/src/routes/piloWs.ts - generate taskId on task:details,
    emit task:accepted event, include in all error responses, pass to runTask
  • Tests added: 3 in taskRunner.test.ts, 7 in pilo.test.ts, 4 in piloWs.test.ts

Test plan

  • pnpm --filter pilo-server run test (70 passing: 56 existing + 14 new)
  • pnpm --filter pilo-core run test (658 passing)
  • pnpm --filter pilo-cli run test (221 passing)
  • pnpm run typecheck green across all packages
  • pnpm run format:check green
  • Manual: curl -i -X POST /pilo/run -d '{}' returns 400 with an
    x-pilo-task-id response header and error.taskId in the body, both
    matching the same UUID v4
  • Manual: WS task:details returns task:accepted event with taskId
    before any agent events

Notes

  • Base branch: main
  • Draft PR since this is the first of a multi-PR stack (Stack A: A1-A4)
  • A2 next: replace errorToString with a sanitized structured error shape
    (class + reason enum, never error.message)
  • gitleaks is not installed on my machine - I couldn't run the recommended
    secret scan, but the diff contains only UUID propagation and test values
    like "task-abc-123", no secrets

@MrTravisB MrTravisB marked this pull request as ready for review April 21, 2026 17:20
@MrTravisB MrTravisB merged commit 51a565c into main Apr 27, 2026
11 checks passed
@MrTravisB MrTravisB deleted the travis/pilo-task-id branch April 27, 2026 16:13
MrTravisB added a commit that referenced this pull request Apr 27, 2026
**Replaces #390** (auto-closed when its base branch was deleted by the
A1 squash-merge).

## Summary
Second PR in Stack A. Stacks on the merged A1 (#389).

Adds structured classification fields to the error response so
dashboards and
programmatic retry logic can reason about failures without parsing
`message` strings. Additive only - no existing field is removed.

### What's added
- `error.class` - error constructor name (e.g. `"TypeError"`,
`"SyntaxError"`)
- `error.reason` - coarse-grained enum, safe for metric labels:
`INVALID_REQUEST`,
  `PROVIDER_UNAUTHORIZED`, `NAVIGATION_TIMEOUT`, `BROWSER_DISCONNECTED`,
  `MAX_ITERATIONS`, `MAX_ERRORS`, `TIMEOUT`, `INTERNAL_ERROR`
- `error.recoverable` - boolean derived from `error instanceof
RecoverableError`
- `error.phase` - which pipeline phase produced the error: `"setup"` or
`"execution"`

### What stays (unchanged for consumers)
- `error.message` - human-readable string, still present
- `error.timestamp` - ISO timestamp, still present
- `error.code` - fine-grained semantic code, still present
- `error.taskId` - from A1, still present

### The data-sensitivity invariant
`error.message` must never contain raw `error.message` of a thrown value
(agent errors can quote task text, URL selectors derived from page
content,
or extracted values). The new code enforces this two ways:

1. **Validation / setup errors** use hardcoded string literals at the
callsite
("Task is required", "AI provider is not configured.", etc.).
Server-controlled
   by construction.
2. **Task execution errors** go through `errorResponseFromError`, which
sets
`message` from a fixed `REASON_HINTS` map keyed by the `reason` enum. No
code
   path reads `error.message` of the thrown value into the response.

A canary test throws `new Error("DO NOT LOG THIS SENTINEL")` and asserts
the
sentinel never appears in the serialized response.

### Minor security improvements (bundled)
Dropped two WS error paths that previously echoed client-provided
strings
back in the message:
- `UNKNOWN_EVENT` no longer quotes `msg.event`
- `UNKNOWN_REQUEST_ID` no longer quotes the requestId
- `INVALID_SEARCH_PROVIDER` no longer echoes the user-provided value

### Changes
- `packages/server/src/taskRunner.ts` - new `ErrorReason` / `ErrorPhase`
types, extended `ErrorResponse`, new `createErrorResponse` (object-param
signature) and `errorResponseFromError` helpers, `classifyError` mapper,
  `REASON_HINTS` map. `errorToString` removed (no callers left).
- `packages/server/src/routes/pilo.ts` - validation errors use new shape
via
`validateTaskRequest`; setup-error path uses `createErrorResponse` with
  explicit `reason: INVALID_REQUEST`; task-execution path uses
  `errorResponseFromError`.
- `packages/server/src/routes/piloWs.ts` - all `createErrorResponse`
callsites
  migrated; task-execution error uses `errorResponseFromError`; dropped
  client-provided values from error messages.
- Tests: `taskRunner.test.ts` / `pilo.test.ts` / `piloWs.test.ts`
updated to
assert new fields alongside existing `message`/`timestamp`/`code`. Added
canary tests that assert no sentinel string ever appears in the
serialized
  response regardless of what the thrown value's `.message` is.

## Test plan
- [x] `pnpm --filter pilo-server run test` (78 passing)
- [x] `pnpm --filter pilo-core run test` (658 passing)
- [x] `pnpm --filter pilo-cli run test` (221 passing)
- [x] `pnpm run typecheck` green
- [x] `pnpm run format:check` green
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants