feat(pull-migration): pull/work-stealing coordination Phases 0–3 (dark) + pilot (#1081)#1550
Merged
Conversation
Fold the ephemeral session docs into two durable status docs and keep the wire-contract specs separate: - docs/planning/PULL_MIGRATION_STATUS.md <- ORCHESTRATOR_SESSION_CONTEXT.md + PULL_MIGRATION_SESSION_HANDOFF_2026-07-07.md + PRD.json - docs/testing/PULL_MIGRATION_TESTING.md <- PULL_MIGRATION_TEST_PLAN.md + PULL_MIGRATION_RECONCILED_TESTING.md MESSAGE_ENVELOPE_SCHEMA.md (#945) and ACTOR_MODEL_POSTCARD.md stay as separate cross-linked reference docs. Removes root-level PRD.json / ORCHESTRATOR_SESSION_CONTEXT.md clutter. Docs only.
…hase 0) Dual-track (SQLite db/migrations.py + Alembic) nullable columns on schedule_executions: - claim_token, lease_expires_at, claimed_by_worker (Alembic 0015) - redelivery_count, distinct from #678 retry_count (Alembic 0016) DDL mirrored in db/schema.py + db/tables.py + db_models.py so fresh builds stay correct (invariant #3). All columns nullable and unread until later phases wire them, so this is a true no-op for the existing fleet. Alembic revisions renumbered 0015/0016 during rebase (re-parented onto dev head 0014_agent_schedules_webhook_auth). The original PG1 env.py version_num widening is dropped — superseded by dev's #1420 (0008a_widen_alembic_version).
…r, capacity meter (#1081 Phase 1+3) Dark backend pull path (no production caller; push path unchanged): - GET /api/internal/next-task: atomic claim of the oldest queued row for an agent, stamps claim_token/lease_expires_at/claimed_by_worker, status->running. - POST /api/internal/tasks/{id}/result: CAS-applies a terminal (status precondition + claim_token match); idempotent replay. - Dual-auth on both seams: X-Internal-Secret OR the agent's own scoped MCP key (authorize_heartbeat) — no master secret in an agent container (#1159/#307). - lease-reaper (services/lease_reaper_service.py + cleanup _sweep_expired_leases): under MAX_REDELIVERY re-queue the SAME execution_id (redelivery_count++), at cap FAIL + park to the operator queue. Re-delivery preserves execution_id so effect_guard/#525 keep deduping (#1402 invariant). - lease_expires_at IS NULL exclusion on 6 non-reaper selectors so leased rows are owned exclusively by the reaper; park/requeue registered in the #1082 status-guard. - capacity shadow meter (count_active_leased_by_agent -> CapacityManager get_all_states/get_slot_state): read-only; admission stays on the ZSET. Fixes C1 (found on real PG): claim_next_queued double-claimed -> double-RAN a row up to N x under READ COMMITTED (uncorrelated InitPlan subquery + no outer status re-check). Adds FOR UPDATE SKIP LOCKED + outer status='queued' re-check; real-PG multi-process race 0/25 double-claims (was 25/25). Also keeps claim_token on park/requeue so a late SUCCESS can CAS-overwrite (B2/B5) and creates the operator alert before the park write (B3). MAX_REDELIVERY=3 and PULL_MODE_PILOT_AGENTS (default empty) in config.py.
… Phase 2) - agent_server/services/pull_worker.py (wired in agent_server/main.py): bounded pool (N = max_parallel_tasks) that short-polls next-task, runs the turn, POSTs the result with claim_token; decorrelated-jitter backoff mirroring #1083 result_callback. Persists the completed terminal to ~/.trinity/pending-pull-results/<eid>.json before delivery, re-sends leftovers on startup, and drains on shutdown so an already-billed turn is never dropped (B6). - services/agent_service/pull_mode.py: injects TRINITY_PULL_MODE / TRINITY_MAX_PARALLEL_TASKS ONLY for agents in the PULL_MODE_PILOT_AGENTS allowlist, at create (crud.py) + recreate (lifecycle.py). Default empty allowlist => no agent opts in => proven no-op. - B1: recreate pops PULL_MODE_ENV_KEYS before re-applying so a de-piloted agent clears the baked flag; the claim seam's agent-key path is allowlist-gated. - G2: recreate setdefaults TRINITY_BACKEND_URL so a legacy agent opted into pull keeps a backend URL (matches the #1098 TMPDIR idiom). Worker auths with Bearer ${TRINITY_MCP_API_KEY}; no master secret injected.
Exclude leased rows (lease_expires_at IS NOT NULL) from the S-01 slot-row bijection via an additive snapshot.running_lease_expires_at, so a pull pilot does not false-fire in_sql_only. A genuine slot-row mismatch on a non-pull row still fires. E-05/E-01 are the same class and remain open (see docs/testing/PULL_MIGRATION_TESTING.md T3.6/T3.7 — apply before opt-in or retire with the ZSET at Phase 5).
… G1)
The backend service uses an explicit environment: list (not env_file), so the
documented pilot opt-in (set PULL_MODE_PILOT_AGENTS in .env + restart) was a
no-op. Forward PULL_MODE_PILOT_AGENTS=${PULL_MODE_PILOT_AGENTS:-} so the flag
reaches the backend process from the tracked compose alone.
obasilakis
force-pushed
the
feature/target-arch-pull-migration
branch
from
July 9, 2026 10:28
469379d to
bd046ea
Compare
Tiered rollback plan for the pull-coordination change: - Tier 0: empty PULL_MODE_PILOT_AGENTS + restart (instant off-switch for a misbehaving pilot; result-report path stays open so in-flight results land). - Tier 1: redeploy previous image for a push-fleet regression; leave the additive nullable columns (expand/contract-safe, no DB step). - Tier 2: DB downgrade rarely/never needed. Plus pre-deploy checklist, detection signals (incl. the G3 canary-on-PG blind spot), scenario->response table, and the Phase-5 point-of-no-return note. Linked from PULL_MIGRATION_STATUS.md.
…dark schema columns (#1081) Review follow-ups (I1/I2): - I1: add TestClaimConcurrencyC1 — a Postgres-only, multi-thread regression for the C1 double-claim fix. N workers race N queued rows through the REAL claim_next_queued; a correct claim is an exact N-way partition, so removing the FOR UPDATE SKIP LOCKED + outer status='queued' guards (all N would claim the head row) fails the bijection. Skips on SQLite / when TEST_POSTGRES_URL is unset. Replaces the torn-down scratch harness; verified passing on real PG. The prior suite only proved SEQUENTIAL no-double-claim, which passes with or without the fix. - I2: document the 4 dark pull columns (claim_token / lease_expires_at / claimed_by_worker / redelivery_count) in architecture.md's schedule_executions schema — they ship to every instance, so the DDL block was stale.
…iew) Durable bug class from the /review of #1550: an UPDATE ... WHERE id = (scalar subquery) compiles to a once-evaluated InitPlan, so under Postgres READ COMMITTED every concurrent updater re-applies to the same id unless the mutated predicate is re-checked in the OUTER WHERE and/or FOR UPDATE SKIP LOCKED is used. Also records why a sequential claim test cannot catch it.
…dark surfaces (#1081) Address /validate-pr #1550 findings: - W1 (config packaging, #1056 class): PULL_MODE_PILOT_AGENTS + MAX_REDELIVERY were read by the backend (config.py) but the .env levers didn't reach a prod deploy — prod compose launches standalone (no env_file merge). Wire both into docker-compose.prod.yml + docker-compose.yml backend.environment and document them in .env.example, so the pilot opt-in is actually settable on prod (still default-empty = fully dark). - W2 (architecture doc): catalog the dark pull surfaces — the two internal claim/result seams on pull_router, the additive lease-reaper in the Cleanup Service, and the capacity shadow meter (metering-not-admission).
Contributor
|
Held during the sequenced merge pass (validated via Two things need doing before it can land:
No changes pushed to this branch. |
|
Resolve by running |
…-existence oracle (#1081) CSO finding (LOW): POST /api/internal/tasks/{execution_id}/result returned 404 for a missing execution but 403 for an existing-but-not-owned one — a cross-tenant existence oracle reachable by any valid agent-scoped MCP key, against all executions, even with pull-mode OFF (the endpoint's auth is inline, not allowlist-gated). Violated the self-uniform enumeration-safety invariant (#186) and deviated from the #1083 callback (agents.py:1045) it claims to mirror. Bounded to LOW by 128-bit token_urlsafe(16) execution_ids (not enumerable), but it ships live. Fold the missing-and-not-owned cases into a uniform 404. Adds a #186 regression test asserting a not-owned and an unknown execution are indistinguishable (same status + body).
#1081) Point-in-time security audit of PR #1550's new attack surface (pull seams, worker, reaper, config). 1 LOW finding (execution-existence enumeration oracle on the result seam) — fixed in 4294488 — plus clean phases + the scoped-key-over-master-secret improvement. Historical record under docs/security-reports/ (out of the enterprise-docs-guard scope).
…nto 0015_enterprise_connectors (#1081) Resolves the PR #1550 review blockers. Conflicts (all additive-vs-additive, both sides kept): - db/migrations.py — dev's enterprise_connectors_table (#118) + this branch's two pull migrations; MIGRATIONS list ordered to match the new Alembic chain. - canary/snapshot.py — dev's E-04/G-04 queued_meta collection (#1077) alongside this branch's lease_expires_at collection (#1081 Phase 3). Both columns are PRAGMA-guarded independently. - tests/test_canary_invariants.py — _add_execution helper takes both parameter sets (duration_ms/queued_at/backlog_metadata + lease_expires_at). - learnings.md — both sides' entries retained. Alembic re-chain (two heads after #1555 landed 0015_enterprise_connectors off 0014, which this branch also chained off): - 0015_schedule_executions_pull_claim_lease -> 0016, down_revision now 0015_enterprise_connectors. - 0016_schedule_executions_redelivery_count -> 0017. - Chain is linear again: 0014 -> 0015_enterprise_connectors -> 0016 -> 0017 (single head, verified via ScriptDirectory.get_heads()). - Rollback runbook downgrade target corrected: 0015_enterprise_connectors, NOT 0014 — the latter would also drop dev's enterprise_connectors table. SQLite track reconciled: enterprise_connectors_table + the two pull migrations all present in MIGRATIONS, in Alembic order.
Contributor
Author
|
@vybe both blockers addressed — pushed as a 1. Merged
2. Alembic re-chained.
One thing your comment surfaced beyond the re-chain: the rollback runbook's downgrade target was Verification (not just the guards — I reproduced the
Still yours to schedule: the dedicated |
…nto 0018 Conflicts were all "both sides appended independent fields at the same spot" (pull-coordination columns from #1081 vs. ephemeral-agent / voice-replies-v2 / source-channel work from dev) — resolved as unions, keeping both: db/schema.py, db/tables.py, db_models.py, db/schedules.py, docs/memory/architecture.md schedule_executions: claim/lease/redelivery columns + ent#117 source_channel columns db/migrations.py SQLite migration list: dev's entries, then ours services/cleanup_service.py both counter sets + both in the total services/agent_service/crud.py pull-mode env prep, then dev's ephemeral quota reservation (which must stay adjacent to the docker block so its rollback path still covers it) Alembic: dev added 0016/0017/0018 while this branch had renumbered its own revisions to 0016/0017, forking the chain (two revisions with down_revision=0015_enterprise_connectors ⇒ "Multiple heads", `upgrade head` would fail). Git does not flag this — both sides just add files. Re-chained the pull revisions onto dev's head: 0016_schedule_executions_pull_claim_lease -> 0019 (down_revision=0018_...) 0017_schedule_executions_redelivery_count -> 0020 (down_revision=0019_...) Chain is single-headed again (21 revisions, no forks, no dangling parents). Also fixes a latent sys.modules leak the merge exposed: four pull tests popped "utils" out of sys.modules to evict the tests/utils helper package, but tests/unit/conftest.py already installs src/backend/utils as the canonical "utils" via an importlib file loader for exactly that reason. Popping it left "utils" unbound, and pytest's prepend import mode puts tests/ back at sys.path[0] — so the next fresh `import utils` bound tests/utils, and every later `from utils.url_validation import ...` in backend code died at collection (15 collection errors once dev's new test modules landed in that window). The tests now use the same idempotent sys.path bootstrap as their siblings. Verification: full unit suite 3899 passed / 13 skipped, 0 collection errors, in both fixed and randomized order. The one failure (test_1474_read_boundary_z::test_schedules_summary_last_run_at_normalized) is pre-existing and reproduces identically on clean dev and on the pre-merge branch head.
…into pull-migration dev advanced while the previous merge was in flight. Same union-shaped conflicts as before — both sides appending independent fields: services/cleanup_service.py CleanupReport: keep the #1081 lease-reaper counters alongside dev's #1581 volume-reclaim and #1142 operator-queue-retention counters — in the field list, the total, and to_dict(). tests/unit/test_cleanup_inner_sweeps.py shared db-mock fixture: keep both stubs (find_expired_leases + prune_operator_queue_ terminal_items), or whichever sweep is missing its stub blows up on a Mock return value. Alembic chain unaffected (dev added no revisions): still single-headed at 0020_schedule_executions_redelivery_count, 21 revisions, no forks. Verification: full unit suite 3951 passed / 13 skipped, 0 collection errors. The single failure (test_1474_read_boundary_z::test_schedules_summary_last_run_ at_normalized) is pre-existing on dev and unrelated to this branch.
5 tasks
…nto 0019 (#1597) dev's #1597 landed 0019_agent_sync_state_git_dir_bytes (also revising 0018), colliding with this branch's 0019/0020. Re-chained: pull_claim_lease -> 0020 (down: 0019_agent_sync_state_git_dir_bytes), redelivery_count -> 0021. SQLite MIGRATIONS list keeps both tracks (git_dir_bytes entry first, matching dev apply order). Doc references updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
vybe
approved these changes
Jul 14, 2026
vybe
left a comment
vybe
left a comment
Contributor
There was a problem hiding this comment.
Validated via /validate-pr: dark by construction (PULL_MODE_PILOT_AGENTS empty ⇒ inert; push path untouched — one claim path, no fork), dual-auth pull seams keep the master internal secret out of agent containers with the pilot-allowlist consumer backstop + uniform-404 enumeration safety (Inv #8/#186), claim-token CAS terminal writes preserve #1082 status-as-projection, lease reaper disjoint via lease_expires_at IS NOT NULL. Dual-track migrations + compose/env.example wiring in-PR; CSO diff audit committed; extensive named #1081 tests. Merged dev in and re-chained Alembic onto 0019_agent_sync_state_git_dir_bytes (now 0020/0021, pg-migrations green); 90 overlap-sensitive unit tests pass locally on the merged state; head pytest green ×3 seeds (base seed-12345 leg was a cancelled hang measuring dev, not this PR — dev CI itself fully green). ✅
obasilakis
pushed a commit
that referenced
this pull request
Jul 17, 2026
…t, derived request ids, cap semantics (#1402) The MAX_REDELIVERY cap + poison-park mechanism shipped in Phase 3 (#1550); this lands the remaining #1402 deliverable: the authored contract agents receive. - Platform prompt (PLATFORM_INSTRUCTIONS → Operator Communication): rewrite around fire-and-park / never block-and-wait (park → end the turn → process responded items in a later turn), ask-before-irreversible-actions guidance, execution-id-derived request ids (approval-{execution_id}-{slug} — global-PK collision safety + re-delivery dedup), expires_at + resume-instructions guidance for agents with no next turn. Section stays MCP-tool-name-free so the Codex transform can't fork it. - _mode_guidance task mode: explicit park carve-out so "execute to completion, don't ask questions" no longer contradicts the gate on the exact autonomous path it targets. - Synced the dead-but-present reference copy (config/trinity-meta-prompt/ prompt.md, injection removed in #136) and added the agent-guide async-comms section (incl. honor-system framing). - Docs: requirements/scheduling.md §10.13 (raw-attempts cap semantics + still- blocking list for pull default-on), TARGET_ARCHITECTURE.md stale claims fixed ("all unbuilt today" → shipped split), operating-room.md (async contract, poison-park items, removed-#136 prompt-integration correction), learnings.md (prompt-delivery-path pitfall). - Tests: tests/unit/test_1402_prompt_contract.py — sentinel phrases across every runtime build, old block-inviting wording gone, section byte-identity across runtimes, carve-out, meta-prompt/guide sync (17 tests). Residual work tracked: #1629 (pull-path prompt delivery + re-delivery banner), create caps). Fixes #1402 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
obasilakis
added a commit
that referenced
this pull request
Jul 17, 2026
…ived request ids (#1402) (#1635) * feat(operator-queue): async human-gate contract — fire-and-park prompt, derived request ids, cap semantics (#1402) The MAX_REDELIVERY cap + poison-park mechanism shipped in Phase 3 (#1550); this lands the remaining #1402 deliverable: the authored contract agents receive. - Platform prompt (PLATFORM_INSTRUCTIONS → Operator Communication): rewrite around fire-and-park / never block-and-wait (park → end the turn → process responded items in a later turn), ask-before-irreversible-actions guidance, execution-id-derived request ids (approval-{execution_id}-{slug} — global-PK collision safety + re-delivery dedup), expires_at + resume-instructions guidance for agents with no next turn. Section stays MCP-tool-name-free so the Codex transform can't fork it. - _mode_guidance task mode: explicit park carve-out so "execute to completion, don't ask questions" no longer contradicts the gate on the exact autonomous path it targets. - Synced the dead-but-present reference copy (config/trinity-meta-prompt/ prompt.md, injection removed in #136) and added the agent-guide async-comms section (incl. honor-system framing). - Docs: requirements/scheduling.md §10.13 (raw-attempts cap semantics + still- blocking list for pull default-on), TARGET_ARCHITECTURE.md stale claims fixed ("all unbuilt today" → shipped split), operating-room.md (async contract, poison-park items, removed-#136 prompt-integration correction), learnings.md (prompt-delivery-path pitfall). - Tests: tests/unit/test_1402_prompt_contract.py — sentinel phrases across every runtime build, old block-inviting wording gone, section byte-identity across runtimes, carve-out, meta-prompt/guide sync (17 tests). Residual work tracked: #1629 (pull-path prompt delivery + re-delivery banner), create caps). Fixes #1402 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(1402): #1629 shipped — correct the stale pull-path prompt-delivery claims The branch predates #1633 by ~1 minute (merged to dev 14:00:26Z; this PR opened 14:01:39Z), so three docs assert a code state that is false on the merge target: - requirements/scheduling.md — #1629 listed under "Still blocking pull default-on" ("pull-claimed turns currently receive NO platform instructions at all") - TARGET_ARCHITECTURE.md — "receive no platform instructions today ... still blocking default-on", and #1629 in the residual list - feature-flows/operating-room.md — "Known gap (#1629): ... bypass platform-prompt composition entirely" On dev, pull_coordination_service.py composes the platform prompt on the claim path (fail-open) and injects the re-delivery banner. The still-blocking list gates the pull default-on decision, so a resolved blocker sitting in it costs a future reader real work. #1629 stays referenced (status-in-dev — open until the release cut), now described as shipped-pending-release rather than unbuilt. learnings.md is left as-is: it records why #1629 was filed, which remains accurate history. --------- Co-authored-by: Eugene Vyborov <eugene@beingluminous.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: obasilakis <vasileios.georgopoulos@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull / Work-Stealing Coordination — Phases 0–3 (dark) + proven pilot
Implements the local-buildable core of the pull/work-stealing dispatch migration (umbrella #1081, Epic #1045), per
docs/planning/TARGET_ARCHITECTURE.md(v2 / Direction B). Everything ships behindPULL_MODE_PILOT_AGENTS(default empty ⇒ inert) — with the flag off, the push path is byte-for-byte unchanged.One-line model
The backend owns one durable per-agent queue (
schedule_executions:queued → claimed/running → terminal). Each agent's worker pool pulls the next task when it has a free worker, runs it, and POSTs the result back under a CAS guard. Nothing is pushed at a busy or dead agent.What's in this PR
claim_token/lease_expires_at/claimed_by_worker+redelivery_countonschedule_executions(dual-track: SQLitemigrations.py+ Alembic0015/0016)GET /api/internal/next-task(atomic claim + lease) +POST /api/internal/tasks/{id}/result(CAS); scoped-key dual-auth (no master secret in agents)pull_worker.py) gated per-agent byPULL_MODE_PILOT_AGENTS; env injection at create/recreateMAX_REDELIVERY(re-queue sameexecution_id, poison-park at cap) + capacity shadow meter + canary S-01 lease-exclusionPhases 4 (sync edge adapter + fan-out join) and 5 (default-ON + delete legacy) are not in this PR — they gate on a ≥2-week zero-orphan soak (#856) and the side-effect gates.
Load-bearing invariants
execution_id(effect_guard / feat: idempotency keys at all execution trigger boundaries (RELIABILITY-006) #525 dedup depend on it).lease_expires_at IS NULLexcludes them from 6 other sweeps + canary S-01.Fixes surfaced by testing (real-PG + concurrency)
claim_next_queueddouble-claimed → double-ran rows under real-Postgres contention (uncorrelated InitPlan subquery + no outer status re-check). Fixed withFOR UPDATE SKIP LOCKED+ outerstatus='queued're-check — real-PG race 0/25 (was 25/25).claim_token(late SUCCESS lost); silent poison on crash; in-flight terminal dropped on shutdown. Fixed. (B4 budget-doubling + slow-worker double-run deferred by design to Phase 5 / effect_guard.)PULL_MODE_PILOT_AGENTS; recreate droppedTRINITY_BACKEND_URL. Fixed.devby bug: Alembic migration 0009 aborts backend boot on PostgreSQL — revision id (41 chars) exceeds alembic_version.version_num VARCHAR(32) #1420 (0008a_widen_alembic_version+env.pywidening), inherited here; pull revisions renumbered0015/0016.Testing
0010 → 0016applied cleanly; the pull worker pool authenticates (scoped key) + polls; a real task flowedqueued → claim (lease + worker) → run → CAS result → successwithZCARD(slots)=0throughout (pure-lease invariant).Rollback
Flag-gated + additive by design, so rollback is cheap (full runbook:
docs/planning/PULL_MIGRATION_ROLLBACK.md):PULL_MODE_PILOT_AGENTS+ restart backend → de-piloted workers get 403 and idle (result-report path stays open so in-flight results land); recreate the agent to stop the worker process. Handles a misbehaving pilot.redelivery_count, fleet failure rate.Docs
docs/planning/PULL_MIGRATION_STATUS.md— status / handoff (consolidated).docs/testing/PULL_MIGRATION_TESTING.md— tiered QA plan + findings + live-pilot record.docs/planning/PULL_MIGRATION_ROLLBACK.md— rollback runbook.docs/planning/MESSAGE_ENVELOPE_SCHEMA.md+ACTOR_MODEL_POSTCARD.md— wire contract + status/error taxonomy.Umbrella: #1081 · Epic #1045
🤖 Generated with Claude Code