Skip to content

feat(agent-org): enforce the formal Task FSM and owner permissions - #869

Draft
ShiboSheng wants to merge 4 commits into
codex/issue-758-turn-context-fifofrom
codex/issue-759-pr4-task-fsm
Draft

feat(agent-org): enforce the formal Task FSM and owner permissions#869
ShiboSheng wants to merge 4 commits into
codex/issue-758-turn-context-fifofrom
codex/issue-759-pr4-task-fsm

Conversation

@ShiboSheng

@ShiboSheng ShiboSheng commented Aug 22, 2026

Copy link
Copy Markdown
Collaborator

Problem

Fixes #759

Depends on #841 and is intentionally based on codex/issue-758-turn-context-fifo.

Formal Agent Org Tasks did not have one complete lifecycle and authority contract. Creation, graph mutation, owner execution, recovery, debug helpers, and UI projection could apply overlapping rules, so invalid initial states, coordinator ownership/output, structural mutation by an owner, terminal rewrites, stale owner callbacks, and recovery-budget resets were not all rejected at the same authoritative boundary.

The root cause was that the SQLite Task Store did not own the full field-level state machine with persisted actor and Turn identity. Tool and wake paths could validate only part of the contract, while task history/results were projected without a bounded Current Work/History contract.

Live-provider acceptance then exposed a second issue at the Provider → Tool dispatcher → typed parser boundary. The unified task_update schema allowed strict Providers to emit semantically empty placeholders for fields belonging to other operations, for example a start call with empty body, output, and reason. The parser removed null and two false flags but treated empty strings and all-empty objects as non-empty cross-operation fields. It therefore returned unexpected_fields before the Task Store write: the Reviewer could run the command successfully while the Task remained pending, history contained only created, and the UI stayed at 0/2 done. Restart correctly preserved that failed state; this was not a database, credential, or prompt problem.

Solution

  • Make the Task Store the final authority for the canonical five states: pending, in_progress, completed, failed, and cancelled.
  • Force every new Task to pending; require structured output for completion and structured reasons for failure/cancellation.
  • Separate typed graph-administrator, owner-execution, and system recovery actors. Revalidate run, snapshot member, owner, session, persisted Turn binding, and generation inside each transaction.
  • Freeze terminal Task structure and results. Only completed satisfies dependencies; goal/owner/dependency changes after execution starts use atomic cancel-and-replace.
  • Make recovery attempts durable and per Task: the first three runtime failures requeue, the fourth becomes terminal, and owner/session generations cannot reset the budget.
  • Add bounded Current Work and paginated History projections with demand-loaded detail/annotations.
  • Make the Provider-facing Task contract role- and operation-aware, then normalize only known fields that are not applicable to the selected operation and are semantically empty: null, empty/whitespace-only strings, empty arrays, recursively all-empty known objects, clear_active_form=false, and clear_owner=false.
  • Keep the parser default-deny: unknown fields are rejected even when empty; non-empty cross-operation fields still return unexpected_fields; fields allowed by the current operation are never discarded, so required output/reason/body validation remains strict.
  • Exercise the real Provider → Tool dispatcher → typed parser → Task Store path in deterministic test support and in packaged-app live-provider acceptance.

The provider compatibility change is neutral to Provider/account/model identity and is fixed before persistence rather than in the UI or Task Store. It does not modify the Task state model, database schema, public Tauri/UI wire, Pause, Archive, or the general Tool Registry. Existing failed pending Tasks are not auto-completed because successful external command execution is not a persisted completion credential.

This preserves the issue boundary: it does not add new Task states, coordinator-owned formal Tasks, Member-private graphs, Writer-capability resolution, forced cancellation of an already running old execution, PR 5 Pause behavior, or PR 6 Archive behavior.

Potential risks

  • Stack dependency: this Draft PR is stacked on feat(agent-org): add transactional turn context and member FIFO #841 because PR 3 is not yet in develop. It must be retargeted and revalidated after the dependency merges.
  • Persistence compatibility: the full PR changes the canonical Task schema and intentionally does not add a backward migration for pre-contract runtime fixtures. Acceptance used a fresh isolated ORGII_HOME and left the normal profile untouched. Do not mix binaries across incompatible Task schemas.
  • Provider compatibility: normalization is deliberately narrow. A Provider that emits an unknown field, or a non-empty field for the wrong operation, will still be rejected. This preserves authority boundaries but may expose other malformed Provider output rather than guessing intent.
  • Concurrency/lifecycle: owner completion can race cancellation/replacement or recovery. Transactions, persisted Turn/generation checks, rollback tests, and late-callback rejection cover the known races, but this remains the highest-risk area.
  • Run-level convergence remains separate: all three final live-provider batches completed and persisted both formal Tasks, but the third Team stayed Finalizing because a redundant plain reminder left unread inbox work and triggered repeated no-canonical-ready-Task wake warnings. No task_update error occurred. This adjacent inbox/wake/Run-FSM issue is outside PR 4 and means the whole-run acceptance was clean in 2/3 batches even though the Task lifecycle acceptance passed in 3/3; the PR remains Draft.
  • Test artifact gate: packaged acceptance requires both the webpack-time frontend gate (WEBDRIVER=1 plus ORGII_AGENT_ORG_REDESIGN=1) and the Rust runtime gate. A normal BuildFast artifact can otherwise compile the frontend disabled while the backend watchdog is enabled.
  • Test infrastructure: the broader group-chat WDIO suite still has four adjacent failures outside this parser change. check:circular is blocked by baseline alias resolution for unchanged ?raw imports. GitHub currently reports no hosted checks for this branch.
  • Workspace selection: the isolated UI workspace was selected, but the durable session reused a cached worktree path. Acceptance commands were safe stdout-only Node commands and made no source changes; the path-selection behavior remains an independent risk.
  • Platform coverage: packaged real-app acceptance ran on macOS only. Windows and Linux packaged behavior was not manually exercised.
  • Rollback/recovery: revert this PR together with its stacked dependency state. For persisted data, restore a pre-cutover backup or use a fresh compatible ORGII_HOME; do not downgrade a database after opening it with the new canonical schema. Reverting the provider patch does not require a separate migration.

Dependency and scope

The latest Provider-boundary fix is isolated to three existing files:

File role Additions Deletions Purpose
Production parser/schema (task_update.rs) 428 69 Role/operation contract and semantic-empty normalization
Task Tool tests (task_tests.rs) 444 17 Operation matrix, rejection rules, dispatch/store/history/restart coverage
Deterministic Provider support (e2e_fake.rs) 52 6 Provider-shaped empty placeholders through the real dispatcher path
Latest fix total 924 92

Why the PR is larger than the original estimate

Issue #759 estimated 2,200 P50 / 3,800 P90 review lines across 17–27 substantive files. The implementation-time impact review later revised PR 4 to 10,800 P50 / 13,000 P90 review lines and 58–62 substantive files plus locale files. The final diff is 17,703 review lines across 121 files, but net growth is 4,631 lines. Most review volume comes from replacing the old cross-layer contract, deleting obsolete tests, and proving the replacement rather than adding unrelated product surface.

The main underestimates were the need to remove the generic mutation bypass instead of adding an FSM beside it; persisted owner-Turn, inbox/wake, recovery, run-finality, Tool, Tauri, and UI consumers that all had to agree; durable per-Task recovery and replay correctness; bounded Current Work/History wire and locale coverage; and real Provider/dispatcher evidence. The final Provider compatibility fix adds 1,016 commit-level review lines but stays inside the same Task parsing boundary.

Splitting canonical DDL, typed Store actors, Tool wire, recovery behavior, and five-state projection into independently runnable PRs would require a temporary old/new compatibility layer or retain an authority bypass. The focused commit stack and review map below are used instead; Pause, Archive, multi-Writer, and UserDirectedWork remain out of scope.

Review map

Contract Owning implementation Primary proof
Five-state FSM and field authority Task Store FSM/create/update/validation Task Store contract tests
Coordinator vs Owner authority Typed actors + Task Tool adapters Tool adapter authority tests
Provider operation contract task_update schema/typed parser Empty-placeholder and default-deny matrix
Persisted owner Turn identity Turn contexts + task-bound inbox drain/wake Stale Turn/generation and inbox tests
Recovery budget/finality Watchdog budget + Store requeue transaction Per-Task budget, rollback, replay tests
Current Work / History Task page commands + Task panel Keyset history tests + rendered UI scenario
Real dispatch path Deterministic Provider + packaged live Provider Focused WDIO and three live batches

Automated verification

  • Focused Task Tool matrix — 11 passed, including create/start/complete/fail/cancel, semantic-empty variants, non-empty cross-operation rejection, unknown-empty rejection, required-field rejection, zero-write failure, history, and restart persistence.
  • Task Store contract suite — 14 passed.
  • Deterministic Provider suite — 12 passed.
  • cargo test -p agent_core — 3,161 passed, 0 failed, 2 ignored.
  • cargo fmt --all -- --check — passed.
  • cargo clippy -p agent_core --all-targets -- -D warnings — passed; the commit hook repeated scoped Clippy successfully.
  • cargo check -p e2e-test — passed.
  • pnpm run typecheck — passed.
  • pnpm run lint — 0 errors and 5 pre-existing warnings outside this diff.
  • pnpm run check:circular — blocked by baseline ?raw alias resolution; the commit hook found 0 new circular dependencies in staged files.
  • Focused Agent Org Task FSM WDIO — 1/1 passed.
  • Broader group-chat WDIO — four adjacent failures remain outside this parser change; these are not reported as passing.
  • E2E script syntax checks and git diff --check — passed.

Packaged real-Provider acceptance

Frozen artifact:

  • Build: WEBDRIVER=1 ORGII_AGENT_ORG_REDESIGN=1 pnpm run tauri:build:fast -- /tmp/ORG2-PR4-live-provider.app
  • App: /tmp/ORG2-PR4-live-provider.app
  • Binary SHA-256: 66e7c1b6348c0c720477d6441e70aea60dc7fb339501e960fc844dc338e67c10
  • Isolated home: /tmp/orgii-pr4-live-wd.eG5Ff6
  • Provider/account/model: OpenAI, display name orlando, account ID prefix 8d269be5…, gpt-5.6-luna; no credential was read or recorded.
  • Frontend and Rust rollout gates were both enabled, and the existing debugAgentOrgEnableRedesign test entry returned {"enabled":true,"ok":true} before acceptance.

Each batch created a new Team, created two formal Tasks assigned to the Reviewer, called task_update(start), ran an actual safe Node stdout command, and called task_update(complete):

Batch Provider inputs/results Store/history UI Run convergence
P1 2 starts + 2 completes succeeded with empty cross-operation placeholders 2 completed; created + started + completed transitions 2/2 done Idle/clean
P2 2 starts + 2 completes succeeded with the same Provider-shaped placeholders 2 completed; created + started + completed transitions 2/2 done Idle/clean
P3 2 starts + 2 completes succeeded with the same Provider-shaped placeholders 2 completed; created + started + completed transitions 2/2 done Stayed Finalizing due to adjacent unread-inbox/wake behavior

Across all three batches, six Tasks completed and persisted with their outputs. The Task history contained exactly 18 facts: six created, six pending → in_progress, and six in_progress → completed. Logs contained zero unexpected_fields and zero Error executing task_update entries.

After fully quitting and restarting the same packaged app, all three Teams still showed 2/2 done; Task outputs and history were unchanged. The pre/post authoritative snapshot SHA-256 was 2f84c72a1cf2849017dd2bbf59c9bff68744c7cdce98b046237c1354e5d2d70d, proving restart neither lost results nor fabricated completion/history. The P3 run-level wake warning also persisted, which is why it is called out separately rather than hidden.

Manual verification guide

  1. Build with both WEBDRIVER=1 and ORGII_AGENT_ORG_REDESIGN=1 so the frontend gate is compiled enabled.
  2. Launch the app binary directly with a fresh ORGII_HOME and runtime ORGII_AGENT_ORG_REDESIGN=1; do not rely on Finder/open preserving environment variables.
  3. Call debugAgentOrgEnableRedesign once to enable the existing test gate; this does not replace the configured Provider.
  4. With a real Provider, create two formal Tasks, assign a Member, and have the Member start, execute a safe command, and complete both.
  5. Cross-check raw Provider tool input, tool result, Task Store status, history, logs, and Current Work/History UI.
  6. Quit and relaunch the same artifact/home, then confirm completed status, output, and history are unchanged.

Visual evidence

No UI source changed in the latest Provider fix. Current Work/History screenshots were captured during packaged acceptance, but authoritative verification is the correlated Provider input, Tool result, Store/history snapshot, logs, and restart hash. Local screenshots are not attached to the public PR because they are not portable review artifacts and add no styling evidence.

Audit

The architecture review covered the Provider wire shape, operation and actor authority, typed parsing, Task Store transaction/FSM, persistence/history, recovery/restart, Tauri projection, rendered Current/History behavior, test/debug entry parity, and default-deny failure paths. There is no resolver change. The latest fix adds no UI source, database migration, background mechanism, or general registry behavior, so no separate UI-consistency or performance audit was triggered.

Make SQLite the final authority for Task creation, mutation, and lifecycle transitions. Add the canonical five-state model, typed actors, durable provenance and results, recovery behavior, and bounded task history reads.

Separate coordinator graph administration from Owner lifecycle operations, validate persisted Turn bindings inside each transaction, and expose Current Work and History without eagerly loading full results or annotations.

Verification:
- cargo test -p agent_core: 3154 passed, 0 failed, 2 ignored
- cargo test -p agent_core section_tests -- --nocapture: 12 passed after the final mechanical fix
- cargo clippy -p agent_core --all-targets -- -D warnings: passed
- pnpm run typecheck: passed
- pnpm run lint: 0 errors; 5 pre-existing warnings outside this diff
- cargo check -p e2e-test and focused Vitest/WDIO PR4 suites: passed
- BuildFast fake-provider batches and one live-provider pending -> in_progress -> completed smoke: passed
- pnpm run check:circular: blocked by two baseline ?raw imports unchanged from PR3

Pre-commit hook ran. Total eslint: 5, total circular: 0
Pre-commit hook ran. Total eslint: 5, total circular: 0
Pre-commit hook ran. Total eslint: 5, total circular: 0
@ShiboSheng

Copy link
Copy Markdown
Collaborator Author

orgii://cloud/session/ref?v=1&org=bfa7b134-2486-45fa-81ad-a369441fafb4&owner=776dbd69-ac1d-4f72-a0d4-69cb4f2667dd&session=codexapp-rollout-2026-08-20T01-27-48-01a01b10-9b52-7f00-8af2-39711a442d02

@ShiboSheng

Copy link
Copy Markdown
Collaborator Author

orgii://cloud/session/ref?v=1&org=bfa7b134-2486-45fa-81ad-a369441fafb4&owner=776dbd69-ac1d-4f72-a0d4-69cb4f2667dd&session=codexapp-rollout-2026-08-22T12-17-41-01a027b0-5225-7080-aa68-d96e8cefdc14

@Harry19081 Harry19081 added enhancement New feature or request agent Agent runtime, behavior, memory, providers, or orchestration cloud-collaboration Cloud sync, organizations, channels, or collaboration labels Aug 23, 2026
Normalize semantically empty fields from unified provider schemas at the authoritative task_update boundary while preserving required-field validation, unknown-field rejection, and non-empty cross-operation rejection.

Move the role-aware Task operation contract onto the PR4 branch and cover provider, dispatcher, typed-parser, store, history, and restart behavior.

Verification:
- cargo test -p agent_core: 3161 passed, 0 failed, 2 ignored
- cargo fmt --all -- --check: passed
- cargo clippy -p agent_core --all-targets -- -D warnings: passed
- cargo check -p e2e-test: passed
- pnpm run typecheck and pnpm run lint: passed
- focused Task FSM WDIO: 1 passed
- BuildFast live provider acceptance: 3 rounds reached 2/2 completed and persisted across restart
- Run finalization wake warning remains deferred as an out-of-scope inbox lifecycle issue

Pre-commit hook ran. Total eslint: 5, total circular: 0
@ShiboSheng

Copy link
Copy Markdown
Collaborator Author

Follow-up: late Inbox message can prevent quiet Run finalization

During the third consecutive real-Provider acceptance run for this PR, both formal Tasks still completed correctly through the production Provider -> task_update parser -> Task Store path. The UI reached 2/2 done, Task history contained the expected created, pending -> in_progress, and in_progress -> completed facts, and the completed state/history survived a full app restart. The logs contained zero unexpected_fields responses and zero Error executing task_update entries.

The Team itself did not settle cleanly in that third run. After Task B had already started, the Coordinator sent the Reviewer one additional reminder. That Inbox row remained unread after both Tasks completed, and the Coordinator did not call org_run_complete. The Run therefore remained running/Finalizing. The watchdog then retried the Reviewer once per minute even though no canonical ready TaskExecution input existed, producing 13 warnings; the same condition remained after restart.

This is not a failure of the PR4 Provider-placeholder normalization, Task FSM, or Task persistence. It is a separate convergence edge case across Run finality, Inbox classification/consumption, and watchdog wake eligibility: finalization considers work outstanding while the canonical wake resolver finds no executable Task. PR4 Task lifecycle acceptance passed 3/3, while whole-Team quiet settlement passed 2/3.

Scope decision: do not expand PR4, PR5 Pause, or PR6 Archive to fix this incident. Track and fix it in a dedicated follow-up issue/PR at the lifecycle/Inbox/watchdog owning boundary. The regression must reproduce this exact ordering:

  1. Reviewer starts the final Task.
  2. Coordinator sends a late reminder while the Reviewer is busy.
  3. Reviewer completes the final Task.
  4. The Team converges to Idle without a repeated wake, or the remaining message is dispatched under its correct non-Task authority.
  5. Restart preserves the settled result and does not restart the warning loop.

Until that follow-up is verified, this PR must not claim three clean end-to-end settled runs; the accurate result is three successful Task lifecycles and two clean whole-Team settlements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent Agent runtime, behavior, memory, providers, or orchestration cloud-collaboration Cloud sync, organizations, channels, or collaboration enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(agent-org): [4/10] enforce the formal Task FSM and owner permissions

2 participants