Merge pull request #14 from edgehero/plan-exec/2026-07-18-run-history-logs

Durable per-job run history + logs (read model for the panel)

Author: edgehero

.env.example

@@ -21,6 +21,9 @@ PI_CONCURRENCY=3       # how many jobs run in parallel
 VALKEY_URL=redis://127.0.0.1:6379
 PI_JOB_IMAGE=pi-job:latest          # the image you built:  docker build -f image/Dockerfile -t pi-job:latest .
 # PI_JOBS_DIR=                      # where per-job /job inputs live (default: your OS temp dir)
+# PI_LOGS_DIR=                      # where per-job status records (and optional raw logs) land (default: OS temp /pi-dispatch/logs)
+# PI_CAPTURE_JOB_LOGS=              # default 0; set 1 to ALSO write raw container output to logs/<jobId>.log -- PII-bearing (issue/comment text), host-only (never mounted into the container), off by default (opt-in)
+# PI_LOG_RETENTION_DAYS=            # default 30; boot-time prune of logs older than N days; 0 = keep forever
 # PI_SCHEDULES_FILE=                # ABSOLUTE path to schedules.json; unset disables cron (a relative path resolves against the worker's WorkingDirectory)
 PI_SCHEDULER_STALL_MAX=2            # tear down a scheduler after N consecutive stalls (money backstop)
 

.plan-artifacts/2026-07-18-run-history-logs-plan/drift-snapshot.json

@@ -0,0 +1,6 @@
+{
+  "generated": "2026-07-18T00:00:00Z",
+  "planSlug": "2026-07-18-run-history-logs-plan",
+  "exitCode": 0,
+  "reqStates": {}
+}

README.md

@@ -152,6 +152,16 @@ the worker handles and drains gracefully. Task Scheduler is a weaker fallback 
 hard kill, giving the worker no chance to drain; a job killed mid-flight leaves a stray container that the
 worker's **boot reaper** clears on the next start, rather than draining cleanly.
 
+## Run history
+
+The worker keeps a durable, per-job record under `PI_LOGS_DIR` (default: your OS temp dir,
+`.../pi-dispatch/logs`). Every job writes an id-only status record `logs/<jobId>.json` — stable ids only
+(the delivery GUID, `repo#issue`), never issue or comment text. Set `PI_CAPTURE_JOB_LOGS=1` to **also**
+capture the container's raw stdout/stderr to `logs/<jobId>.log`; this is **opt-in and off by default**,
+because that raw stream can contain issue and comment text (PII). Both files stay host-side, are never
+mounted into the job container, and are gitignored. A boot-time sweep prunes anything older than
+`PI_LOG_RETENTION_DAYS` (default 30; `0` keeps them forever).
+
 ## Scheduling recurring jobs
 
 A cron schedule is a trigger, not a new job kind: each entry runs a local folder through a flow on a cron

SECURITY.md

@@ -118,6 +118,13 @@ Stated openly rather than discovered later:
 - **The provider API key is broad.** Unlike the GitHub token it cannot be meaningfully scoped per job —
   the agent needs it to function. It is the one broad secret inside the container. **Set a spend limit
   on it.**
+- **Captured job logs can contain issue and comment text (PII).** By default the worker writes only an
+  id-only status record per job — `logs/<jobId>.json`, keyed on stable ids (the delivery GUID,
+  `repo#issue`) and never on issue or comment bodies. With `PI_CAPTURE_JOB_LOGS=1` (opt-in, **off by
+  default**) it also tees the container's raw stdout/stderr to `logs/<jobId>.log`, and that stream **can**
+  carry issue/comment text. Both live host-side under `PI_LOGS_DIR`, are **never mounted into the job
+  container**, and are **gitignored**; a boot-time sweep prunes them (`PI_LOG_RETENTION_DAYS`, `0` = keep
+  forever). Leave capture off unless you need it, and treat the log directory as personal data while it is on.
 - **Prompt injection is not prevented, only bounded.** Untrusted text is kept out of the trusted region
   of the prompt by *placement*, not by filtering — content-filtering natural language is not a security
   boundary and this project does not pretend otherwise. The bound is the container, the scoped token,

deploy/com.pi-dispatch.worker.plist

@@ -22,6 +22,10 @@
 
   One worker per host (DES-CONCURRENCY-3): parallelism is PI_CONCURRENCY inside the one process.
   Requires the AOF-enabled Valkey from deploy/docker-compose.yml.
+
+  PI_LOGS_DIR (run-history records; default OS-temp /pi-dispatch/logs) is created and written by the
+  worker at boot, so it must be writable by the account the daemon runs as. Set via `.env` (the wrapper),
+  not a plist change; its default is distinct from the StandardOutPath worker.out.log below.
 -->
 <plist version="1.0">
 <dict>

deploy/nssm-install.cmd

@@ -19,6 +19,10 @@ REM One worker per host (DES-CONCURRENCY-3): parallelism is PI_CONCURRENCY insid
 REM multiple services. Requires the AOF-enabled Valkey from deploy/docker-compose.yml.
 REM
 REM Per-host PLACEHOLDERS: set SERVICE / REPO / LOGDIR below for your host before running.
+REM
+REM PI_LOGS_DIR (run-history records; default OS-temp \pi-dispatch\logs) is created and written by the
+REM worker at boot, so it must be writable by the service account. Set via `.env` (the wrapper), not a
+REM change here; its default avoids colliding with the nssm LOGDIR worker.out log set below.
 
 setlocal
 

deploy/worker.service

@@ -12,6 +12,11 @@
 # Env vars come from your `.env` (see `.env.example`) via EnvironmentFile -- never commit real secrets.
 # WorkingDirectory / EnvironmentFile / User / node path below are PLACEHOLDERS: set them to wherever
 # you cloned the repo and whoever owns it.
+#
+# PI_LOGS_DIR (run-history records; default OS-temp /pi-dispatch/logs) is created and written by the
+# worker at boot, so it must be writable by User= (pi) -- do NOT pre-create it as root, or the non-root
+# worker hits EACCES at boot. Set via `.env` (EnvironmentFile), no unit change needed; the default path
+# avoids colliding with the daemon's own logs/worker.out.log.
 
 [Unit]
 Description=pi-dispatch worker (drains the job queue on the host; launches job containers via docker)

image/runner/test/compose.test.mjs

@@ -1,4 +1,5 @@
 import assert from "node:assert/strict";
+import { readFileSync } from "node:fs";
 import { test } from "node:test";
 import { captureTerminal, decideExit, EXIT_COMPLETED, EXIT_INFRA, EXIT_POLICY } from "../src/outcome.mjs";
 
@@ -43,3 +44,33 @@ test("decideExit: no terminal message and no abort is infra, not success", () =>
 	// Absence of evidence that the agent ran is not success.
 	assert.equal(decideExit({ budgetAborted: false, terminal: undefined }).code, EXIT_INFRA);
 });
+
+test("decideExit's non-abort branch carries NO turns of its own -- the premise the source-guard rests on", () => {
+	// WHY the guard below exists: on the success path, `turns` reaches the exit log SOLELY from
+	// run-job.mjs's `log("exit", { ...outcome, turns: budget.state.turns })` spread. decideExit's
+	// non-abort branch returns classifyStopReason, which has no `turns` field (only the budget-abort
+	// branch carries one -- see "a blown budget wins" above). So if that spread ever dropped `turns`,
+	// nothing in outcome.mjs would put it back, and the worker's parseExitTurns (which requires a
+	// numeric `turns` on the exit line) would silently read null on every completed run.
+	const outcome = decideExit({ budgetAborted: false, terminal: { stopReason: "stop" } });
+	assert.equal(outcome.code, EXIT_COMPLETED);
+	assert.equal(outcome.turns, undefined, "the non-abort branch of decideExit carries no turns of its own");
+});
+
+test("run-job.mjs staples `turns` onto the success-path exit line -- worker parseExitTurns depends on it", () => {
+	// A stdout-capture test would EXECUTE the runner (main() self-runs on import and `log` is
+	// unexported), so this guards the worker<->runner contract against the source instead -- the same
+	// tactic worker/test/wiring.test.mjs uses for wiring it cannot exercise at runtime. The regex
+	// matches a `log("exit", { ... turns: ... })` call whose object literal carries a `turns:` key
+	// before its closing brace: specific enough that dropping `turns` from the success spread fails
+	// it, tolerant of whitespace and property reordering.
+	const src = readFileSync(new URL("../run-job.mjs", import.meta.url), "utf8");
+	assert.match(
+		src,
+		/log\("exit",\s*\{[^}]*turns:/,
+		"run-job.mjs exit line must carry turns -- worker parseExitTurns depends on it",
+	);
+	// The catch-path exit line (classifyThrow, a preflight throw) legitimately OMITS turns: no budget
+	// exists when the agent loop never started. The regex above matches the success call and does not
+	// require every exit line to carry turns, so that omission is allowed by design.
+});

specs/design.md

@@ -137,6 +137,33 @@ money with no upstream turn limit (`REQ-RUNNER-TURN-BUDGET`).
 - **Reference** (no authority): `docs.bullmq.io/guide/rate-limiting`, `/guide/workers/pausing-queues`,
   `/guide/redis-tm-compatibility`.
 
+## DES-RUN-HISTORY-FLAT-FILES-NO-DB
+
+- **Decision**: The per-job run history is a flat `node:fs` sidecar keyed by job id — an id-only status
+  record at `logs/<jobId>.json` (written with `fs.writeFileSync`) plus an optional append-only
+  `logs/<jobId>.log` of raw container output (`fs.createWriteStream`). No database, no logging framework.
+  Retention is a boot-time age sweep (`makeLogReaper`, window `PI_LOG_RETENTION_DAYS`), not a rotation
+  library.
+- **Why**: The record must **outlive the queue entry**. BullMQ evicts completed and failed jobs by age
+  (`removeOnComplete` / `removeOnFail`), so the retained job cannot be the durable store — and it never
+  carries `exitCode`, `turns`, or `budgetReserved` in the first place. The sidecar is justified precisely
+  by what BullMQ lacks: a record that survives eviction and holds the run's outcome fields. Those records
+  are immutable, filename-keyed, and never queried across each other, so a store with query power earns
+  nothing. This is `DES-QUEUE-BULLMQ-OVER-CUSTOM`'s library-first ethos one file down, and the design
+  home for the `document-build-decision` rule that the sidecar's inline `Custom:` comment cites.
+- **Rejected**:
+  - *Querying BullMQ's retained job instead of a sidecar* — BullMQ evicts by age, so it is not the
+    *durable* store, and it never carries `exitCode` / `turns` / `budgetReserved`. It cannot be the
+    record.
+  - *An embedded database (lowdb / better-sqlite3) or a structured-logging library (pino / winston)* —
+    the records are immutable, filename-keyed, with no cross-record query in scope, so neither earns its
+    keep. A DB adds a native build (the ARM / musl / glibc pain the job image exists to avoid) and a
+    second retention authority beside the reaper for zero query benefit; a logging library brings its own
+    rotation as that same second authority. Both violate the deliberate "no database" thinness
+    (`interfaces.md` preamble) and the library-first ethos — the same reasoning as
+    `DES-QUEUE-BULLMQ-OVER-CUSTOM`.
+- **Traces to**: `DES-QUEUE-BULLMQ-OVER-CUSTOM`; implemented in `worker/src/run-history.mjs`.
+
 ## DES-PERSONA-VIA-APPEND-SYSTEM-MD
 
 - **Decision**: Bake the persona into the image at `~/.pi/agent/APPEND_SYSTEM.md`, **and** pass per-flow
@@ -531,3 +558,4 @@ a tunnel.
 | 2026-07-15 | Initial. Extracted from `DESIGN.md` v0.1 (2026-07-14, local, uncommitted) §2, §3, §4, §5, §9, §11. That document recorded "50 claims adversarially verified: 48 confirmed, 2 refuted" — **verified against documentation**. Source-verification at `earendil-works/pi @ 5e336cf` subsequently corrected ~7 points. `DES-PERSONA-VIA-APPEND-SYSTEM-MD` is materially rewritten: the source doc's decisions #1 and #2 were mutually exclusive as written. `DES-NAME-KEEP-PI-DISPATCH` is new. `pi-harness` and `pi-sentry` were absent from the source doc's alternatives and are added. §5.7's "caches roll at midnight" caveat is **dropped** — 0.80.7 removed the date from the default system prompt. |
 | 2026-07-15 | An admin panel and cross-platform (Windows/macOS/Linux + Docker) added to scope. Two new decisions and one **security correction**. `DES-PANEL-SEPARATE-FROM-RECEIVER`: the source doc mounted Bull Board on the receiver — defensible for a read-only dashboard, **not** once the same surface sets the model and rewrites flows, because the receiver is the one process that must be internet-reachable. The panel and the receiver have opposite reachability requirements and cannot share a port. `DES-FLOWS-ARE-DATA-PERSONA-IS-CODE`: the panel requirement collided with keeping flows as reviewed repo markdown; resolved by observing that one file was carrying two jobs — hard rules need immutability, task recipes need editability. Architecture diagram and repo layout updated; the public edge is now drawn explicitly. Build order extended with panel and deploy. |
 | 2026-07-16 | **Resolved a spec/code contradiction.** `DES-WORKER-ON-HOST` added and `DES-JOB-FILES-VIA-VOLUME-SUBPATH` marked SUPERSEDED: the worker runs on the host (the `docker` CLI translates host paths, the daemon does not, and the VM prefix moved between Docker Desktop versions; local-folder jobs also *require* a host bind mount a named volume cannot give). The committed spec had rejected worker-on-host while the code already did it -- caught by a spec-conformance scan. `DES-CLI-TRIGGER-FOR-LOCAL` added: the CLI producer was built (user-directed) but unspecified; recorded with the check that `CONST-BUDGET-BEFORE-TOKENS` still holds because the cap is enforced in the processor, not the trigger. Repo-layout `deploy/` line corrected (compose runs Valkey only). |
+| 2026-07-21 | Added DES-RUN-HISTORY-FLAT-FILES-NO-DB (flat node:fs sidecar over a DB / BullMQ-query for run history). |

specs/interfaces.md

@@ -496,12 +496,62 @@ Evidence convention as in `constitution.md`.
   `piDispatchConfig`-tagged error. Given a valid `local` entry, when it fires, then the emitted job's
   `data` byte-matches the shape produced by the interactive local (`enqueueLocalJob`) path.
 
+## INT-RUN-HISTORY-FILE-CONTRACT
+
+**worker → panel.**
+
+- **Contract**:
+  ```
+  <logsDir>/<sanitizedJobId>.log      append-only container stdout+stderr; untrusted, PII-bearing; written ONLY when PI_CAPTURE_JOB_LOGS=1
+  <logsDir>/<sanitizedJobId>.json     one JSON object, PII-free, overwritten on each terminal state (last-write-wins across retries)
+  (logsDir via PI_LOGS_DIR; empty/unset = <OS temp>/pi-dispatch/logs)
+  { "jobId":   "<raw job id: delivery GUID | local-<hex> | repeat:<sched>:<millis>>",
+    "kind":    "github" | "local" | null,
+    "target":  "<repo>#<issue>"  |  "local:<basename>" | null,
+    "flow":    "<flow name>" | null,
+    "startedAt": "<ISO-8601>", "endedAt": "<ISO-8601>",
+    "outcome":   "completed" | "policy" | "failed",
+    "reason":    "<fixed enum: worker-abort|over-budget|unprotected-branch|runner-policy|container-never-started|...>" | null,
+    "exitCode":  <int> | null,
+    "turns":     <int> | null,
+    "budgetReserved": <bool> | null,
+    "attempt":   <int> }
+  ```
+  Field order is the serialisation order (`JSON.stringify` emits insertion order). The filename uses the
+  **sanitized** id (`:` → `_`, because `repeat:<sched>:<millis>` is NTFS-illegal); the record **body**
+  keeps the raw `jobId`. `reason` is a fixed enum passed through from the terminal outcome — never
+  free-form and never payload text — and `turns` is `null` when the container died before emitting the
+  runner `exit` line.
+- **Why**: The panel is a separate process (`DES-PANEL-SEPARATE-FROM-RECEIVER`) that reads this as a
+  read-model it does not share memory with — the worker writes the files, the panel reads them, and
+  nothing crosses in RAM. The worker writes on both terminal paths: `worker/src/index.mjs` `makeProcessor`
+  calls `recordRun` on the success (`result`) and the failure (`error`) branch alike, and
+  `worker/src/run-history.mjs` `makeRecordWriter` serialises the record with a truncating
+  `fs.writeFileSync`, so a re-run of the same id overwrites — last-write-wins across retries. The `.json`
+  is PII-free **by construction**: `buildRecord` (`worker/src/run-history.mjs`) is an explicit object
+  literal over stable id-only fields and never spreads `job.data`, `result`, or `error`, so a GitHub
+  job's title/body and a local job's `task` or full folder path cannot leak — `target` keeps only
+  `repo#issue` or the folder `basename` (`no-pii-in-logs`, `REQ-LOCAL-JOB-VISIBILITY`). The `.log` is a
+  **separate file** from the `.json` precisely so the untrusted, PII-bearing container stream — teed off
+  each stdout/stderr chunk by the sink in `worker/src/run-container.mjs` — never contaminates the
+  structured record; it is opt-in, host-side (never mounted into the container), and written only under
+  `PI_CAPTURE_JOB_LOGS=1`. This is a **flat per-job file, not a database**: one `.json` (plus the optional
+  `.log`) keyed by the sanitized job id, no schema and no query surface — upholding this file's standing
+  invariant that **there is deliberately no database**.
+- **Traces to**: `REQ-DURABLE-RUN-HISTORY`, `REQ-LOCAL-JOB-VISIBILITY`, `INT-RUNNER-EXIT-CODE-PROTOCOL`
+- **Acceptance**: Given a job reaching a terminal state, exactly one `.json` keyed by its sanitized job
+  id exists; its `outcome` matches the queue outcome (`completed` / `policy` / `failed`); no field carries
+  issue or comment body text (`target` is `repo#issue` / `local:<basename>` only); `turns` is `null` when
+  the container died before emitting the runner `exit` line; the `.log` exists only when
+  `PI_CAPTURE_JOB_LOGS` is set.
+
 ---
 
 ## Revision History
 
 | Date | Change |
 |---|---|
+| 2026-07-21 | Added INT-RUN-HISTORY-FILE-CONTRACT (worker→panel run-history read-model files). |
 | 2026-07-17 | Added INT-SCHEDULES-FILE-CONTRACT, documenting the implemented `schedules.json` host-file shape (`PI_SCHEDULES_FILE`): `local`-only, `:`-free unique `id`, `task` as DATA, and load-time rejection of malformed/`github`/duplicate/missing-folder entries. |
 | 2026-07-15 | Initial. Extracted from `DESIGN.md` v0.1 §5.1, §5.3, §5.4, §5.5. `INT-SDK-SESSION-OPTIONS` is **new** — the source doc left the SDK option set unverified (its §10) and was wrong about the print-mode flag shape and the mode union. `PLAYWRIGHT_BROWSERS_PATH` added to the runtime contract: the source doc's Dockerfile was broken as written for non-root execution. The source doc's code sketches are deliberately **not** carried over — the real Dockerfile and handler are the truth, and a spec that mirrors them drifts on the first commit. |
 | 2026-07-16 | **Correction — "pi never throws" was FALSE**, and it was in this file for a day as the justification for forbidding `try`/`catch` outright. Adversarial re-verification refuted it: `agent-session.ts:1242-1244` is `catch (error) { preflightResult?.(false); throw error; }`, and pi's **own JSDoc** (`:1099-1100`) documents throws on no-model, no-API-key, and missing `streamingBehavior`; `agent.ts:470-471` throws `"Agent is already processing."` outside the lifecycle try entirely. The rule as written would have produced a runner that dies of an unhandled rejection on a missing API key, exiting Node's default `1` = *retryable*, so the queue pays to retry a job that can never succeed. **Both mechanisms are required and cover disjoint sets: preflight throws, the loop swallows.** Also corrected: `StopReason` has **five** values (`packages/ai/src/types.ts:380`) — the entry handled three, and a default branch silently maps `"length"` (truncated output) to success. `reload()` has **no early return** — a second call fully re-runs everything; the earlier "the `loaded` guard makes a double call safe" framing was wrong. The lesson is the file's own: this entry was written from source and still asserted an absolute from a partial read. `INT-CONTAINER-RUNTIME-CONTRACT` gained `--init`, `--shm-size` (explicitly **not** `--ipc=host`, which Playwright recommends but which would share the host IPC namespace with an adversarial container), fonts (absent ⇒ tofu-box screenshots that silently gut `REQ-FRONTEND-VISUAL-VERIFY`), and the fact that **`COPY --chown` does not fix the EACCES trap** because it skips auto-created parent dirs. |