feat(agent-org): enforce the formal Task FSM and owner permissions - #869
feat(agent-org): enforce the formal Task FSM and owner permissions#869ShiboSheng wants to merge 4 commits into
Conversation
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
|
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 |
|
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 |
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
Follow-up: late Inbox message can prevent quiet Run finalizationDuring the third consecutive real-Provider acceptance run for this PR, both formal Tasks still completed correctly through the production 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 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:
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. |
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_updateschema allowed strict Providers to emit semantically empty placeholders for fields belonging to other operations, for example astartcall with emptybody,output, andreason. The parser removednulland two false flags but treated empty strings and all-empty objects as non-empty cross-operation fields. It therefore returnedunexpected_fieldsbefore the Task Store write: the Reviewer could run the command successfully while the Task remainedpending, history contained onlycreated, and the UI stayed at0/2 done. Restart correctly preserved that failed state; this was not a database, credential, or prompt problem.Solution
pending,in_progress,completed,failed, andcancelled.pending; require structured output for completion and structured reasons for failure/cancellation.completedsatisfies dependencies; goal/owner/dependency changes after execution starts use atomic cancel-and-replace.null, empty/whitespace-only strings, empty arrays, recursively all-empty known objects,clear_active_form=false, andclear_owner=false.unexpected_fields; fields allowed by the current operation are never discarded, so required output/reason/body validation remains strict.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
pendingTasks 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
develop. It must be retargeted and revalidated after the dependency merges.ORGII_HOMEand left the normal profile untouched. Do not mix binaries across incompatible Task schemas.Finalizingbecause a redundant plain reminder left unread inbox work and triggered repeated no-canonical-ready-Task wake warnings. Notask_updateerror 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.WEBDRIVER=1plusORGII_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.check:circularis blocked by baseline alias resolution for unchanged?rawimports. GitHub currently reports no hosted checks for this branch.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
codex/issue-758-turn-context-fifo)codex/issue-759-pr4-task-fsm49ad34970b80530e0be07043cf8012e60e53110cThe latest Provider-boundary fix is isolated to three existing files:
task_update.rs)task_tests.rs)e2e_fake.rs)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
task_updateschema/typed parserAutomated verification
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.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?rawalias resolution; the commit hook found 0 new circular dependencies in staged files.git diff --check— passed.Packaged real-Provider acceptance
Frozen artifact:
WEBDRIVER=1 ORGII_AGENT_ORG_REDESIGN=1 pnpm run tauri:build:fast -- /tmp/ORG2-PR4-live-provider.app/tmp/ORG2-PR4-live-provider.app66e7c1b6348c0c720477d6441e70aea60dc7fb339501e960fc844dc338e67c10/tmp/orgii-pr4-live-wd.eG5Ff6orlando, account ID prefix8d269be5…,gpt-5.6-luna; no credential was read or recorded.debugAgentOrgEnableRedesigntest 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 calledtask_update(complete):2/2 done2/2 done2/2 doneAcross all three batches, six Tasks completed and persisted with their outputs. The Task history contained exactly 18 facts: six
created, sixpending → in_progress, and sixin_progress → completed. Logs contained zerounexpected_fieldsand zeroError executing task_updateentries.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 was2f84c72a1cf2849017dd2bbf59c9bff68744c7cdce98b046237c1354e5d2d70d, 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
WEBDRIVER=1andORGII_AGENT_ORG_REDESIGN=1so the frontend gate is compiled enabled.ORGII_HOMEand runtimeORGII_AGENT_ORG_REDESIGN=1; do not rely on Finder/openpreserving environment variables.debugAgentOrgEnableRedesignonce to enable the existing test gate; this does not replace the configured Provider.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.