Skip to content

feat: make workflow runs observable and resumable - #74

Closed
SSS135 wants to merge 1 commit into
QuintinShaw:mainfrom
SSS135:feat/workflow-runtime-reliability
Closed

feat: make workflow runs observable and resumable#74
SSS135 wants to merge 1 commit into
QuintinShaw:mainfrom
SSS135:feat/workflow-runtime-reliability

Conversation

@SSS135

@SSS135 SSS135 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR hardens workflow triggering, lifecycle control, observability, persistence, and resume behavior as one runtime-reliability change:

  • disable forced keyword triggering by default; retain explicit opt-in with identifier-bounded matching and preserve slash commands
  • add the LLM-facing workflow_control tool for list/status/set_concurrency/pause/resume/stop/restart/remove
  • persist requested/effective concurrency and all execution controls; concurrency defaults to 16, accepts any positive integer without a plugin hard cap, and can be resized while a run is live or paused
  • track stable agent execution identities, queued/running/paused/terminal states, active labels, live usage estimates, and exact message-boundary usage
  • add versioned run-state migration, lease/race protection, replay-safe aggregate accounting, and restart/resume semantics
  • persist child Pi sessions by default in a private per-project workflow directory outside Pi's normal /resume picker, and reopen paused agents at the last durable message/tool-result boundary; persistAgentSessions: false remains an explicit opt-out
  • preserve isolated worktrees while paused, reuse them on resume, and clean them on completion/stop
  • distinguish finished, paused-mid-run, and never-started agents throughout the panel, navigator, and control output

Why forced keyword triggering is disabled by default

Previously, merely typing workflow or workflows in an ordinary message could rewrite the prompt and force workflow mode. That made it difficult to discuss the feature itself, mention documentation or paths containing the word, or use the word naturally without unexpectedly launching orchestration. Hidden prompt mutation based on one keyword is poor UX and produces false positives.

The lexical forcing hack is also unnecessary: current models can select the advertised workflow tool from its schema and system guidance when orchestration is appropriate. Explicit /workflows run ..., direct tool selection, saved workflow commands, and slash-command controls remain available. Users who still want keyword activation can opt in with /workflows-trigger on; when enabled, matching is Unicode/identifier-bounded rather than a raw substring check.

Resume semantics

  • Completed journal entries replay without rerunning or double charging.
  • Started-but-incomplete agents resume from their saved Pi session and preserved worktree.
  • Never-started agents remain queued and start fresh.
  • Missing/corrupt session files fall back per-agent to a fresh restart.
  • Exact mid-token/mid-tool continuation is not possible; the continuation prompt tells agents to inspect existing state and avoid repeating completed side effects.
  • Crash recovery pauses stale runs instead of auto-resuming them.
  • Resume continues the same run; restart creates a new run from the beginning.

Default behavior changes

Child session persistence is now on by default because reliable turn-boundary resume requires a durable transcript. Users can opt out with:

{ "persistAgentSessions": false }

Child transcripts live under ~/.pi/workflows/projects/<project>/agent-sessions/, not ~/.pi/agent/sessions/, so they remain available through each run artifact's sessionFile links without cluttering Pi's normal /resume picker. The README calls out that full child transcripts may contain sensitive context.

Concurrency now defaults to 16 when neither the run nor settings specify it. The workflow schema accepts any positive integer and the plugin no longer clamps concurrency to 16; provider capacity and maxAgents remain the practical ceilings. workflow_control set_concurrency and /workflows concurrency <runId> <n> resize running or paused workflows. Raising the limit releases queued agents immediately; lowering it never aborts active agents and waits for activity to drain below the new limit before starting replacements. The mutable FIFO gate is shared with nested workflows and the updated value is persisted for resume/restart.

Automated validation

  • npm test: 892/892 passed (Biome, TypeScript, 59 suites)
  • git diff --check: passed
  • real Pi createAgentSession faux-provider integration verifies reopening and appending to the same session JSONL
  • schema/runtime tests verify positive concurrency values including 128 and 10_000 are accepted without clamping, invalid/non-integer values are rejected, and the omitted default is 16
  • live-resize tests verify 1 → 3 immediately starts queued work, 3 → 1 lets existing agents drain without aborting them, a later increase releases additional work, FIFO order is retained, UI/persistence update immediately, and cold paused runs retain the new value for resume

Real-provider validation

Provider/model: openai-codex/gpt-5.6-sol through the installed local extension.

Pause/resume smoke

  • started a background child, confirmed running, paused it during a tool turn, resumed the same run, and completed with RESUME_CHILD_OK
  • persisted result: completed, one agent, 71,058 tokens, 29.9s
  • saved session contains the original tool turn, aborted boundary, resumed tool turn/result, and final stop turn
  • the test exposed a duplicate false Subagent was aborted notification; this draft includes the regression fix so lifecycle-controlled pause/stop no longer also emit error (unexpected external aborts still do)

42-agent concurrency/UI stress

  • requested/effective concurrency: 8; max agents: 42
  • observed successive states with exactly eight active labels while queued work drained (for example 3 done / 8 running / 31 queued, then 34 done / 8 running / 0 queued)
  • all 42 agents completed in 64.3s; no snapshot exceeded eight running agents
  • persisted usage: 986,874 tokens; $3.6456
  • all 42 results matched their assigned PROBE_n identifier after ignoring terminal punctuation. The workflow's own ok:false field came from an overly strict test assertion that expected a final period while the agents returned the requested identifiers without one; it was not a runtime/concurrency failure.

Process restart after 27 completed calls

Before forced process termination:

  • 27 done, one running, one queued; concurrency 1
  • aggregate usage: 658,068 tokens

After restarting Pi:

  • stale run recovered as paused rather than auto-running
  • exactly 27 agents remained done, the interrupted agent showed paused mid-run (session saved), and the never-started agent remained queued
  • requested/effective concurrency remained 1

After workflow_control resume:

  • completed with 29 unique execution IDs and no duplicate rows
  • original 27 results remained exact and were not rerun
  • interrupted agent reused the same child session and returned INTERRUPTED_OK
  • queued agent started fresh and returned QUEUED_OK
  • aggregate usage advanced monotonically from 658,068 to 705,447 tokens

Live concurrency resize

  • started six real agents at concurrency 1: one running, five queued
  • workflow_control set_concurrency changed 1 → 3; queued agents were released immediately and status showed three running
  • changed 3 → 1 while three agents were active; none were aborted, status truthfully showed three running against limit one, and no replacement started until activity drained
  • observed drain states 3 done / 1 running / 2 queued, then 4 done / 1 running / 1 queued
  • completed all six with six exact results and six unique execution IDs
  • final requested/effective concurrency persisted as 1
  • all six child sessions were stored in the private workflow agent-sessions directory outside /resume
  • persisted usage: 283,570 tokens; $0.7570; 113.0s

Remaining manual checklist

The high-scale concurrency and real process-restart scenarios are now complete. This remains a draft pending any maintainer-requested manual checks for legacy artifact migration, safe-typing permutations, and the full navigator/keybinding compatibility matrix from CONTRIBUTING.md; those paths have automated coverage.

@SSS135
SSS135 force-pushed the feat/workflow-runtime-reliability branch 2 times, most recently from 5825343 to cdfcc09 Compare July 15, 2026 09:56
@SSS135
SSS135 force-pushed the feat/workflow-runtime-reliability branch from cdfcc09 to af55f2d Compare July 15, 2026 10:16
@SSS135
SSS135 marked this pull request as ready for review July 15, 2026 10:41
@QuintinShaw

Copy link
Copy Markdown
Owner

Thanks @SSS135 — this is a substantial, carefully-tested piece of work (892 tests green here, verified). Before diving into review I want to be upfront about how I'd like to land it: right now it bundles several independent changes, and a few flip default behavior in ways that deserve their own focused review rather than riding in with the rest:

  • workflow_control tool + the richer run/agent observability — the part I'm most keen on; composes cleanly with the existing WorkflowManager, a clear win on its own.
  • Keyword-trigger default flipped to off (plus the identifier-boundary matching change) — a behavior change existing users would feel.
  • persistAgentSessions default flipped to on + moved storage location — a privacy-surface change (full child transcripts).
  • Concurrency hard cap removed (16 → soft default) — a resource-usage change.

Could you split these into separate PRs, the way #46 was split into #62 + the rest? The workflow_control/observability slice can likely go in quickly on its own; the three default-behavior changes each want their own discussion (and probably a settings opt-in rather than a silent default flip). That also keeps each diff reviewable.

One clarification so expectations line up: this PR is about manual lifecycle control and resume-safety — it doesn't include auto-resume-on-usage-limit (#27), which is a separate track. No problem, just noting it so nothing falls through the cracks.

Really appreciate the depth here — keen to get the control tool in. Let me know if you'd like a hand identifying the split boundaries.

QuintinShaw added a commit that referenced this pull request Jul 16, 2026
…age limit resets (#78)

Closes #27. A run that pauses on a provider usage/quota limit now auto-resumes
once the quota is likely refilled, instead of sitting paused until a human runs
/workflows resume.

New standalone module src/usage-limit-scheduler.ts (UsageLimitScheduler):
consumes only the manager's public event/query surface, staying decoupled from
manager/persistence internals (minimal footprint against the pending large #74
rewrite). Event-driven 'fire and watch': arm a timer on a usage_limit pause,
call resume() when it fires, and let the existing 'paused'/'complete'/'error'
subscriptions drive backoff and cleanup.

Default-on with per-run opt-out via ExecOptions.autoResume (persisted).
Exponential backoff, hard attempt cap (5), 1m delay floor, 6h ceiling.
resetHint parsed best-effort ('~3h', '1h30m', 'in 5m'), falls back to 5m.
Cold-start re-arm: a run still paused-on-usage_limit at process start is
rescheduled using time REMAINING (now - updatedAt), not a fresh full delay.

An injectable clock/timer makes the timer-fire path fully unit-tested (26
tests): normal fire, backoff-on-repause, attempt-cap give-up, cold-start
remaining-time math, opt-out skip, resetHint parsing, delay floor, and
resume()->false not consuming an attempt.

Additive-only to existing files: ExecOptions.autoResume + two PersistedRunState
fields + scheduler wiring in extensions/workflow.ts (disposed on
session_shutdown).
@SSS135
SSS135 marked this pull request as draft July 16, 2026 07:52
@SSS135

SSS135 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — I’ve started splitting #74 along the requested review boundaries and moved this monolith back to draft as the immutable reference.

Ready for review

Placeholder drafts (implementation not started)

The default-16/no-hard-cap change from #74 has been dropped. The bounded navigator and paused-script amendment work remain local-only and are not in these replacements.

I’ll keep #74 and af55f2d unchanged as the reference and close it as superseded once the replacement drafts contain their focused implementations.

@QuintinShaw

Copy link
Copy Markdown
Owner

Thanks @SSS135 — this is exactly the split I was hoping for, and I really appreciate that you also turned the three default-behavior changes into non-breaking ones: concurrency staying within the existing cap (#82), child-session resume as opt-in (#81), and keyword matching without changing defaults (#79). That removes the main thing that would have needed careful gating.

One heads-up before you polish these: main moved since you branched — two changes just landed that touch the same area as your durable-resume (#80) and child-session (#81) PRs:

Rebasing onto latest main before you finalize will save you a conflict later. Happy to review each as you mark it ready — the workflow_control/observability one (#83) looks like the cleanest standalone win to land first. Thanks again for taking the split on.

@SSS135

SSS135 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Closing this original combined PR as superseded by the focused replacement series requested in review.

All replacement PRs now contain their actual implementations, are rebased onto current main (7800cbd), pass their full Windows validation, and remain draft for now:

The original branch and af55f2d remain intact as a reference; this PR is being closed without rewriting or force-pushing it.

Dropped from the replacement policy changes:

  • no keyword default flip
  • no child-session default flip
  • no removal or increase of MAX_CONCURRENCY = 16

The local-only bounded navigator and paused-script amendment were not included in these replacements (edited-script resume is now independently present upstream via #84).

@SSS135 SSS135 closed this Jul 16, 2026
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