Skip to content

fix(workflow): settle fire limits atomically - #90

Merged
trvon merged 3 commits into
fix/execution-identity-authorityfrom
fix/workflow-limit-authority
Sep 5, 2026
Merged

fix(workflow): settle fire limits atomically#90
trvon merged 3 commits into
fix/execution-identity-authorityfrom
fix/workflow-limit-authority

Conversation

@trvon

@trvon trvon commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Summary

  • enforce workflow-wide and state-local fire limits under the Store lock
  • preserve terminal settlement and evidenced exits without permitting renewal
  • account for ordinary and cadence activation budgets
  • add deterministic and property-based limit coverage

Validation

  • full branch suite: 919 tests passed

Stack

3 of 6. Base: fix/execution-identity-authority.

Copilot AI lite review requested due to automatic review settings September 5, 2026 01:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

A new unconditional structuredClone in LoopStore.fireUnlocked introduces avoidable per-fire overhead on a hot path and should be made conditional/removed.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR tightens workflow fire-limit enforcement by settling both workflow-wide and state-local cadence limits atomically inside LoopStore’s lock, aiming to prevent races and preserve correct terminal settlement behavior in pi-loop’s workflow controller runtime.

Changes:

  • Updates default workflow fire budgeting to account for both ordinary (non-cadenced) activations and cadence-driven fires.
  • Settles fire-limit exhaustion during LoopStore fire handling under the store lock (including pause/delete settlement paths).
  • Adds deterministic integration, regression, and property-based tests to cover workflow/state fire caps and atomic settlement invariants.
File summaries
File Description
test/workflow-task-integration.test.ts Adds an integration regression ensuring default workflow budgets allow a normal phase + one cadence fire + terminal completion.
test/property/store.property.test.ts Adds a property test asserting persisted fireCount/state fire counts never exceed the active cap and that exhaustion pauses.
test/astra-workflow-limits.test.ts Adds targeted regressions for atomic rejection/no-write behavior after exhaustion across reopened stores.
src/tools/workflow-tools.ts Refines workflowDefaultMaxFires to reserve capacity for ordinary activations and cadence budgets.
src/store.ts Settles workflow/state fire limits under lock during fire, pausing/deleting immediately on exhaustion and preserving returned fire results.
Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/store.ts Outdated
@trvon
trvon force-pushed the fix/workflow-limit-authority branch from 82666fe to ff5a48d Compare September 5, 2026 02:19
@trvon
trvon force-pushed the fix/workflow-limit-authority branch from ff5a48d to b346e55 Compare September 5, 2026 02:51
@trvon
trvon merged commit 1e90523 into master Sep 5, 2026
5 checks passed
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