Skip to content

feat: persist workflow run records with real session stats - #31

Open
Mourey wants to merge 1 commit into
Michaelliv:mainfrom
Mourey:feat/run-store
Open

Mourey wants to merge 1 commit into
Michaelliv:mainfrom
Mourey:feat/run-store

Conversation

@Mourey

@Mourey Mourey commented Aug 21, 2026

Copy link
Copy Markdown

Implements the persistence half of the dynamic-workflows epic (#1), aligned with its vocabulary: each run lands under workflow-runs/<runId>/ as script.js (the inline script verbatim, per #3) plus task.json — written atomically (tmp+rename) on every snapshot recompute and at terminal, so a crash mid-run leaves a valid, current record.

What the record carries beyond today's snapshot:

  • Real token usage: AgentSession.getSessionStats() read in the agent's finally before dispose() (today the budget accounting estimates by JSON.length / 4); budget tracking prefers real totals and falls back to the estimate when stats are unavailable.
  • Per-agent wall-clock startedAt/endedAt captured in the tool's agent callbacks.
  • Full result text in the record; the streamed display snapshot keeps its 80-char preview.

Store behavior: failures are swallowed by design — persistence never kills a paid run. Config precedence: createWorkflowTool({runStore}) → env (PI_DYNAMIC_WORKFLOWS_RUN_STORE) → host config file under the agent dir → defaults (enabled, getAgentDir() root). Verified against pi 0.78/0.83 and the oh-my-pi shim surface.

Two deliberate deviations from the epic's draft, open to discussion:

  • run ids are wf_<16 hex> rather than ulid (sortable-enough via startedAt, and matches an existing downstream consumer contract);
  • finishedAt rather than endedAt at the record top level (same reason).

9 new unit tests (id minting, config precedence, writer atomicity, terminal-once, end-to-end record with stats); npm test green (35/35), biome + tsc clean. No behavior change when the store is disabled.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QrkRtZZJqWpnrzc3xe7V2x

Add a run store as the first slice of the persisted/resumable-workflows
epic. Every execution mints a wf_<hex> run id and writes
workflow-runs/<runId>/{script.js,task.json} under a configurable store
root (default: the host agent dir): task.json is rewritten atomically
(tmp+rename) on every snapshot update and once more at terminal
(completed/failed/killed), so external readers always see complete JSON
with a fresh lastWriteAt.

The record is enriched at source with what the display snapshot lacked:
per-agent wall-clock startedAt/endedAt from the agent callbacks, real
token usage/cost/toolCalls via AgentSession.getSessionStats() read
before dispose (replacing the JSON.length/4 estimate for budget
accounting when stats exist), and each agent's full result text — the
80-char preview stays display-only. Explicit harness (pi|omp) and
dialect stamps ride in the record; the host is detected from the agent
dir path with config/env overrides.

Config precedence: createWorkflowTool({runStore}) > env
(PI_DYNAMIC_WORKFLOWS_RUN_STORE / _HARNESS) > <agentDir>/extensions/
pi-dynamic-workflows/config.json > defaults (enabled, agent dir).
Persistence is best-effort by design: a store failure never fails a
running (paid) workflow. getSessionStats/getAgentDir verified against
pi-coding-agent 0.78.0 (peer pin), 0.83.0, and the omp 17.x shim
surface; the stats read is typeof-guarded regardless.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QrkRtZZJqWpnrzc3xe7V2x
zhangys2 added a commit to zhangys2/pi-dynamic-workflows-michaelliv that referenced this pull request Aug 26, 2026
…ixes

Adopts camerondurham's upstream model resolution in place of the simpler
one on this branch, and keeps the fixes that PR does not have.

Michaelliv#27's resolution is strictly better: model patterns rather than exact ids
only, `sonnet:low` thinking-level suffixes clamped to what the model
supports, session services built once via createAgentSessionServices
instead of per agent, the resolved model reported back for the progress
line, and — a hole this branch had — `workflow` added to excludeTools so
a subagent cannot recursively spawn workflows.

Conflict resolutions:

- agent(): kept our onUsage token reporting alongside their onModel.
- Agent events: kept our `id` and added it to their onAgentModel, which
  arrived matching agents by label. Label matching is the bug this
  branch fixed; ids make the model land on the right progress row.
- buildAgentInstructions: rejected their reintroduction of it. It
  narrates opts.isolation and opts.agentType, the no-op options this
  branch removed, and neither field exists on AgentOptions any more.
- AgentRunOptions.model: both sides added it; kept one field with their
  doc comment, which describes the richer syntax.
- resolveWorkflowModel/ModelResolverRegistry: deleted along with
  tests/workflow-agent.test.ts. Superseded by their resolveModelSpec, and
  their tests/agent-model-resolution.test.ts covers the same ground more
  thoroughly. Two model resolvers in one file would be worse than either.

Also takes the getSessionStats guard from PR Michaelliv#31: stats are read before
dispose() and behind a typeof check, since a host fork may not expose the
method, in which case the caller falls back to its own estimate instead
of failing the run.

npm test green: biome, typecheck, build, 57 unit tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
HangxiangMa pushed a commit to HangxiangMa/pi-dynamic-workflows that referenced this pull request Sep 7, 2026
…haelliv#29)

Adds a user-configurable keywordTriggerWord with /workflows-trigger set/reset, and an explicit /workflows run <prompt> manual trigger that works even when keyword triggering is off. Default workflow/workflows behavior preserved. Resolves Michaelliv#31.
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.

1 participant