Skip to content

refactor(orchestration): back SubAgentJobRegistry with DetachedTaskRegistry (D11 phase 3) - #346

Merged
senamakel merged 11 commits into
mainfrom
harness-uplift/jobs-registry-removal
Oct 9, 2026
Merged

senamakel merged 11 commits into
mainfrom
harness-uplift/jobs-registry-removal

Conversation

@senamakel

Copy link
Copy Markdown
Member

Summary

D11 phase 3: SubAgentJobRegistry is now a thin adapter over tinyagents_tasks::DetachedTaskRegistry, so there is one live registry implementation. Public API of SubAgentJobRegistry and SubAgentJobsTool is unchanged.

  • Each job's SubAgentJob snapshot is the detached registry's watched status (watch::channel); ownership, snapshots, steering lookup and request-id dedupe (D7) are the detached registry's.
  • The adapter keeps only the status senders and the cancel-requested flag, behind one controls mutex that acts as the transition gate, so a settle and a cancel never interleave (first terminal state wins, as before).
  • D1 per-child cancel tokens, cancel-then-settle, and panic -> Failed (mark_aborted) behave as before. Existing tests pass unchanged except one mechanical edit: settled_jobs_release_their_cancellation_token read the private inner map, now it uses holds_live_cancellation / get.
  • Settled jobs stay queryable: the soft cap is usize::MAX, and the registry's pruning APIs (wait, cancel, sweep_terminal) are never called by the adapter (field is private to the module, invariant documented).
  • Steering handles are now deregistered when a job settles (previously held for the registry lifetime).

Minimal tinyagents-tasks additions

register_cooperative (no abort handle; jobs are stopped through their own token, never hard-aborted), cancel_cooperative (trip + release token, keep entry), release_cancellation, holds_cancellation. Entry cancellation/abort became Option. register and register_cooperative share one private register_entry.

Things to know

  • No wait verb existed on SubAgentJobRegistry/SubAgentJobsTool (verbs are query/list/cancel + the message tool), so none was kept or added. Adding one would change the tool schema.
  • TaskStore is not used: job lifecycle is process-local and synchronous here, as before, and the store is durable/async. Not a fit without changing behaviour.
  • No public type was left purposeless, so nothing was deprecated.
  • Changes are confined to invocation/ plus the tasks additions above; subagent/detached/ untouched.

Tests

New: tasks cooperative-cancel/release tests; adapter tests for registry view, steering release, 2000 settled jobs surviving, and the exact NotFound/Terminal/Cancelling/RequestIdTooLong ordering. cargo test --workspace (only known validate_repo_root_rejects_non_repo fails), clippy -D warnings, fmt clean. Reviewed by a code-reviewer pass; all Important findings fixed.

Co-authored-by: Medulla medulla@tinyhumans.ai

senamakel and others added 9 commits October 9, 2026 06:01
Add tests for cooperative registration cancellation, verifying that
cancelling only succeeds for the owning parent, leaves the entry
registered, and releases the token without aborting the task. Also cover
releasing a cancellation token without cancelling it and confirming hard
cancel still works afterwards.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Restructured the task runtime to reduce duplication and clarify the
control flow without changing observable behaviour.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce a registry that tracks subagent tool invocations so callers can look up and manage in-flight subagent runs. This provides the foundation for coordinating multiple subagent invocations within the orchestration layer.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce types describing subagent invocation so callers have a shared
shape for requests and results. This is groundwork for wiring up
subagent execution.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The subagent job registry now stores job state in the shared detached
task registry instead of its own map, so cancellation tokens, steering
handles, and request-id claims are owned in one place. A new settle
helper centralizes terminal transitions and releases the cancellation
token and steering handle together, keeping the first-terminal-state-wins
behaviour intact.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The tasks accessor on SubAgentJobRegistry is only used by tests, so it is now
compiled under cfg(test) to keep it out of non-test builds.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Document the tinyagents-orchestration crate so its purpose and usage are discoverable from the source tree.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The unused top-level import of DetachedTaskRegistry was dropped and the
test-only accessor now refers to the type through its full path, keeping
the import list limited to what the module actually uses.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-09T03:31:29.931047Z 23ffd2f New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 8 billable files and costs up to $2.00.

Or wait 17 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: dc05acfe-63a5-4797-bf2c-8336d9d62e7a
📥 Commits

Reviewing files that changed from the base of the PR and between 054bfc7 and 23ffd2f.

📒 Files selected for processing (8)
  • crates/tinyagents-orchestration/src/README.md
  • crates/tinyagents-orchestration/src/subagent/invocation/jobs.rs
  • crates/tinyagents-orchestration/src/subagent/invocation/mod.rs
  • crates/tinyagents-orchestration/src/subagent/invocation/mod_jobs_tests.rs
  • crates/tinyagents-orchestration/src/subagent/invocation/mod_registry_tests.rs
  • crates/tinyagents-orchestration/src/subagent/invocation/types.rs
  • crates/tinyagents-tasks/src/lib_tests.rs
  • crates/tinyagents-tasks/src/runtime.rs
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@tinysweeper

tinysweeper Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper reviewed this change across 6 lane(s) and found 7 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below.

State: Changes requested
Priority: high
Reviewed head: 9e76d28fd226
Updated: 1791516383 (Unix time)

Review snapshot

Change surface Files Review signal Count
Production 4 Active findings 10
Tests 3 Noted findings 0
Documentation 1 Resolved findings 42
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

The review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

  • medium · critique · Report unknown tasks when releasing cancellation — `release_cancellation_trusted` returns `Ok(())` when `task_id` is absent, so callers cannot distinguish releasing a registered task from releasing nothing. A stale or mistyped ID i (crates/tinyagents\-tasks/src/runtime\.rs:194)
  • medium · critique · Do not hide owned-job registry lookup failures — Unlike `get`, this path converts every `DetachedTaskRegistryError` into `None`. That hides failures such as a poisoned registry lock and makes them indistinguishable from an unknow (crates/tinyagents\-orchestration/src/subagent/invocation/jobs\.rs:311)
  • medium · critique · Handle task registration errors without panicking — A registration failure is converted into a panic. The registry API can reject the operation when its lock is poisoned, and the previous implementation recovered poisoned state inst (crates/tinyagents\-orchestration/src/subagent/invocation/jobs\.rs:116)
  • medium · critique · Require ownership before releasing a cancellation token — This public release operation accepts only a task ID and performs no ownership check. Any caller that can obtain a registry reference can clear another owner's live cancellation to (crates/tinyagents\-tasks/src/runtime\.rs:190)
  • medium · critique · Reject cooperative cancellation after token release — A task with a non-terminal status but `cancellation == None` passes the ownership and terminal checks, then this block skips cancellation and still returns `Ok`. This can occur aft (crates/tinyagents\-tasks/src/runtime\.rs:181)
  • high · security · Add the referenced registry test module — The revision adds cooperative registration, cooperative cancellation, optional abort handles, and cancellation-token release behavior, but the crate has no sibling registry test mo (crates/tinyagents\-tasks/src/runtime\.rs:108)
  • medium · security · Return an error for unknown cancellation entries — This method returns `Ok(())` when `task_id` is not registered, hiding a detached-registry lookup failure from its caller. A stale or mistyped task ID is indistinguishable from a su (crates/tinyagents\-tasks/src/runtime\.rs:194)
  • medium · security · Make cancellation state update atomic with task cancellation — `cancel_cooperative` can cancel the task and allow the child to finish before `control.cancellation_requested` is set below. In that window, `settle` observes the request flag as f (crates/tinyagents\-orchestration/src/subagent/invocation/jobs\.rs:292)
  • medium · security · Require ownership before releasing cancellation tokens — This public release operation accepts only a task ID and performs no ownership check, unlike the owner-aware control methods. Any caller that can access the registry can drop anoth (crates/tinyagents\-tasks/src/runtime\.rs:190)
  • medium · tests · Log registry failures in get_owned instead of returning None — `get` and the `sorted` listing now log non-`Unknown` detached-registry errors before degrading, but `get_owned` still collapses every error — including a poisoned internal lock — i (crates/tinyagents\-orchestration/src/subagent/invocation/jobs\.rs:311)

Resolved this pass

  • Add the referenced registry test module
  • Preserve stable ordering when listing all jobs
  • Sort owned jobs by job ID before returning them
  • Require ownership before releasing a cancellation token
  • Preserve stable ordering when listing jobs
  • Add the referenced registry test module
  • Do not hide detached-registry lookup failures
  • Preserve stable ordering when listing all jobs
  • Sort owned jobs by job ID before returning them
  • Require ownership before releasing a cancellation token
  • Preserve stable ordering when listing jobs
  • Add the referenced registry test module
  • Do not hide detached-registry lookup failures
  • Preserve stable ordering when listing all jobs
  • Sort owned jobs by job ID before returning them
  • Require ownership before releasing a cancellation token
  • Preserve stable ordering when listing jobs
  • Do not hide detached-registry lookup failures
  • Preserve stable ordering when listing all jobs
  • Sort owned jobs by job ID before returning them
  • Require ownership before releasing a cancellation token
  • Preserve stable ordering when listing jobs
  • Add the referenced registry test module
  • Do not hide detached-registry lookup failures
  • Preserve stable ordering when listing all jobs
  • Sort owned jobs by job ID before returning them
  • Require ownership before releasing a cancellation token
  • Preserve stable ordering when listing jobs
  • Add the referenced registry test module
  • Preserve stable ordering when listing all jobs
  • Sort owned jobs by job ID before returning them
  • Preserve stable ordering when listing jobs
  • Add the referenced registry test module
  • Preserve stable ordering when listing all jobs
  • Sort owned jobs by job ID before returning them
  • Preserve stable ordering when listing jobs
  • Add the referenced registry test module
  • Do not hide detached-registry lookup failures
  • Preserve stable ordering when listing all jobs
  • Sort owned jobs by job ID before returning them
  • Require ownership before releasing a cancellation token
  • Preserve stable ordering when listing jobs

Before merge

  • Address Add the referenced registry test module (crates/tinyagents\-tasks/src/runtime\.rs).

How this fits together

flowchart LR
  n0["new"]:::impacted
  n1["invoke_in_parent_context"]:::impacted
  n2["child_config"]:::impacted
  n3["run_child"]:::impacted
  n4["RunContext"]:::impacted
  n5["AgentRun"]:::impacted
  n1 -->|calls| n2
  n1 -->|uses| n4
  n2 -->|calls| n0
  n3 -->|uses| n4
  n3 -->|uses| n5
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Loading
Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 4 files; 5 findings. (1 observation(s) grouped into shared inline comments) _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._
  • Evidence: crates/tinyagents\-tasks/src/runtime\.rs — Report unknown tasks when releasing cancellation
  • Evidence: crates/tinyagents\-orchestration/src/subagent/invocation/jobs\.rs — Do not hide owned-job registry lookup failures
  • Evidence: crates/tinyagents\-orchestration/src/subagent/invocation/jobs\.rs — Handle task registration errors without panicking
  • Evidence: crates/tinyagents\-tasks/src/runtime\.rs — Require ownership before releasing a cancellation token
  • Evidence: crates/tinyagents\-tasks/src/runtime\.rs — Reject cooperative cancellation after token release

security

  • Conclusion: Failure
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 4 files; 4 findings. (1 observation(s) grouped into shared inline comments) _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._
  • Evidence: crates/tinyagents\-tasks/src/runtime\.rs — Add the referenced registry test module
  • Evidence: crates/tinyagents\-tasks/src/runtime\.rs — Return an error for unknown cancellation entries
  • Evidence: crates/tinyagents\-orchestration/src/subagent/invocation/jobs\.rs — Make cancellation state update atomic with task cancellation
  • Evidence: crates/tinyagents\-tasks/src/runtime\.rs — Require ownership before releasing cancellation tokens

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The registry-adapter rewrite is now backed by a real test module (mod_registry_tests.rs) that pins registration, ownership, error ordering, steering release, sweep survival and list sorting, and the tasks crate gained cooperative-cancel and release tests — the earlier missing-test findings are resolved. Sorting and logged registry failures are also fixed. One earlier concern remains: `get_owned` still converts every registry failure into `None` without logging, so a poisoned-lock error is indistinguishable from an unknown job. Otherwise this looks safe to merge. (2 earlier finding(s) still open) (1 observation(s) grouped into shared inline comments) _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._
  • Evidence: crates/tinyagents\-orchestration/src/subagent/invocation/jobs\.rs — Log registry failures in get_owned instead of returning None

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The revision completes the registry-as-adapter refactor and fixes all earlier findings: the referenced registry test module now exists (mod_registry_tests.rs), listings are sorted via `sorted()`, detached-registry errors are logged or mapped rather than silently dropped, and ownership is enforced through the registry's owner checks. The new cooperative registration/cancel APIs are covered by tests, lock ordering (controls → tasks) is consistent, and the cancel-then-settle protocol preserves the first-terminal-state-wins behaviour. The change looks sound and ready to merge. _Code retrieval was unavailable (model: ladder embeddings returned 400 Bad Request: {"error":{"message":"unknown ladder vectors; known ladders are flash (also chat-v1, flash-v1), instant (also no-think, instant-v1), reasoning (also deepseek), max-reasoning (also max-reasoning-v1), deepseek-flash (also reasoning-v1, agentic-v1), deep (also luna), scribe, uncensored, vectors-oai3 (also embeddings-oai3-v1), vision (also vision-v1, multimodal-v1), image (also images-v1, image-v1), vi), so this review saw the diff alone._ _Memory was unavailable (model: cortex: v1/recall: error sending request for url (http://cortexdb:3141/v1/recall\)\), so this review ran without it._

e2e

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: No end-to-end harness in this repository: no e2e test files and no e2e workflow.
Evidence and run details
  • Models: gpt-5.6-luna, glm-5.3-flash
  • Spend: $0.025483
  • Tokens: 454543 input · 36749 output · 39261 cached · 0 embedding
Head State Pass summary
a878424b279a changes requested 7 active finding(s), 0 resolved finding(s) (at 1791516029)
9e76d28fd226 changes requested 10 active finding(s), 42 resolved finding(s) (at 1791516383)

tinysweeper 0.1.0

@tinysweeper tinysweeper Bot 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.

Requesting changes: 2 lane(s) blocking, worst finding is critical.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.0302 · 623,973 in / 35,558 out · 66,973 cached (11%) · gpt-5.6-luna, glm-5.3-flash
critique:    $0.0158 · 316,619 in / 15,591 out · 34,570 cached (11%) · gpt-5.6-luna, glm-5.3-flash
security:    $0.0129 · 236,790 in / 11,771 out · 32,211 cached (14%) · gpt-5.6-luna
tests:       $0.0012 · 39,023 in  / 5,960 out  · 64 cached (0%)      · glm-5.3-flash
description: $0.0001 · 15,782 in  / 492 out    · 64 cached (0%)      · glm-5.3-flash

Comment thread crates/tinyagents-orchestration/src/subagent/invocation/mod.rs
Comment thread crates/tinyagents-orchestration/src/subagent/invocation/jobs.rs Outdated
Comment thread crates/tinyagents-orchestration/src/subagent/invocation/mod.rs
Comment thread crates/tinyagents-orchestration/src/subagent/invocation/jobs.rs Outdated
Comment thread crates/tinyagents-orchestration/src/subagent/invocation/jobs.rs Outdated
Comment thread crates/tinyagents-tasks/src/runtime.rs Outdated
@tinysweeper tinysweeper Bot added the priority: p0 Drop what you are doing. Data loss, a live break, or an exploitable hole. label Oct 9, 2026
…ted release API

Co-authored-by: Medulla <medulla@tinyhumans.ai>

@tinysweeper tinysweeper Bot 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.

Requesting changes: 1 lane(s) blocking, worst finding is high.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.0255 · 454,543 in / 36,749 out · 39,261 cached (9%)  · gpt-5.6-luna, glm-5.3-flash
critique:    $0.0141 · 232,715 in / 19,100 out · 18,310 cached (8%)  · gpt-5.6-luna, glm-5.3-flash
security:    $0.0110 · 172,342 in / 14,016 out · 17,879 cached (10%) · gpt-5.6-luna
tests:       $0.0001 · 16,589 in  / 1,040 out  · 1,536 cached (9%)   · glm-5.3-flash
description: $0.0001 · 16,484 in  / 801 out    · 1,408 cached (9%)   · glm-5.3-flash

Comment thread crates/tinyagents-orchestration/src/subagent/invocation/jobs.rs Outdated
Comment thread crates/tinyagents-orchestration/src/subagent/invocation/jobs.rs Outdated
Comment thread crates/tinyagents-tasks/src/runtime.rs
Comment thread crates/tinyagents-tasks/src/runtime.rs
Comment thread crates/tinyagents-tasks/src/runtime.rs
Comment thread crates/tinyagents-tasks/src/runtime.rs Outdated
Comment thread crates/tinyagents-orchestration/src/subagent/invocation/jobs.rs
@tinysweeper tinysweeper Bot added priority: p1 Next. Wrong behaviour a user will hit, or a security weakness behind a condition. and removed priority: p0 Drop what you are doing. Data loss, a live break, or an exploitable hole. labels Oct 9, 2026
…n, Unknown on release

Co-authored-by: Medulla <medulla@tinyhumans.ai>
@senamakel
senamakel merged commit 89c895e into main Oct 9, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p1 Next. Wrong behaviour a user will hit, or a security weakness behind a condition.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant