Repository navigation
feat(subagents): add bounded same-thread batch result inspection - #6416
Conversation
willem-bd
left a comment
There was a problem hiding this comment.
Reviewed at 8eafae3. This matches the #6414 contract well: identity is resolved server-side (resolve_runtime_user_id + runtime thread_id) and never accepted from model arguments; the service layer checks batch ownership and thread equality before projecting a fixed field whitelist that excludes prompts, execution_spec and lease fields; reads are single-row, bounded, and serialize/hash off the event loop; and the response-limit derivation keeps continuation windows inline under the real ToolOutputBudgetMiddleware pass-through condition (verified against _patch_tool_message). The new suite is thorough (real submission/worker/reopen on SQLite and optional Postgres, wrong-owner/wrong-thread/missing-position denials, revision restarts, budget interactions, gating and no-global-fallback). I ran the new file plus the adjacent batch tool/service/repository and budget-middleware suites from a worktree at this head: 320 passed, 1 skipped (the Postgres param without TEST_POSTGRES_URI). Two non-blocking maintainability suggestions below.
|
Fixed the PostgreSQL CI fixture error in 86e9101, after merging current official Setup, reopen and cleanup now share a test URI adapter that maps Validation at the final commit: 25,638 passed / 0 failed in the four-shard backend suite ( The new Unit Tests run is |
Closes #6414
Scope proposal: discussion #6415.
Why
An explicit research/review batch can finish after the submitting agent turn and retain reports and acceptance verdicts in SQL. In a later turn, the owner can ask for synthesis of selected results, but the native graph only exposes submission, progress counts and cancellation. The report is available to the HTTP export consumer, not to a native model tool. Counts alone also cannot explain unmet or UNVERIFIED requirements.
This adds the missing bounded consumer to #4998's durable batches and #5289's acceptance results. It follows existing subagent result work and is independent of the still-open RAG evidence export PR #6413. It does not change retrieval, acceptance policy or batch execution.
What changed
While durable batch tools are enabled,
read_batch_resultreads one stable item position owned by the current user in the current thread. Model arguments cannot select a user or thread; private prompts, execution/authorization context and lease fields are excluded.A result document contains the existing stored report, execution/error state and separate acceptance criteria/verdict. Reads do not wait, schedule, retry or modify state. Failed/cancelled/nonterminal and legacy items retain their existing semantics; execution success does not imply acceptance.
Return bounded document windows: default 4,000 characters, requested maximum 8,192. Fit the complete escaped response under 10,000 characters and the bound host's positive per-tool externalization/fallback budgets. Metadata shares the document window; smaller budgets shorten it. Serialization and hashing run off-loop. The proposal's 8,000-character approximate ceiling is implemented as 8,192.
Continuation carries exact character offsets and a content revision. A changed completion/retry/cancellation state returns
restart_requiredrather than mixing attempts. Concatenate windows before parsing the document; each individual window can be partial JSON.result_truncatedstill describes the original storage cap, which this reader cannot undo.Wire the tool into Gateway assembly and explicit SDK runtime binding, retaining lifecycle unavailability without global fallback. Gateway tools use the assembly's
AppConfigbudget; SDK callers with a custom budget middleware should pass its matching config throughSubagentRuntime.Update lead guidance, README and module guides. Bulk results remain in the authorized JSONL export; no automatic result ingestion/polling, migration, dependency or UI change.
This is the native batch runtime's read contract, which existing public extension capabilities do not expose. Reusing its service bridge and repository projection avoids a second executor, storage system or credential-bearing HTTP self-call. No business-specific workflow is hard-coded into the harness.
Reproducible entry
Offline graph acceptance, with no provider or paid model:
In an enabled batch deployment, the owner can ask in the batch's thread to inspect selected stored items. The registered tool call starts with:
{"batch_id":"<id returned by batch_task>","position":0,"offset":0,"max_chars":4000}Decode the response JSON, accumulate
content, and follownext_offsetwith itsrevisionasexpected_revision. Stop atnext_offset: null, then parse the complete result document.next_positionnames the next stable item position withinbatch_status.total_items. Onrestart_required, discard previous windows and begin again.budget_too_smallmeans the operator must raise the tool budget; do not retry a zero-progress read. Limits below even the error envelope cannot produce a usable structured response. No live model-obedience, identity-provider, browser or PostgreSQL deployment experiment is claimed.Surface area
Frontend UI
Backend API — existing HTTP responses are unchanged
Agents / LangGraph — new opt-in batch read tool and Gateway/SDK graph assembly
<, with decoding preserving the original data.Sandbox
Skills
Dependencies
Default behavior change — existing enabled batch graphs gain explicit result inspection; batch mode remains off by default
Docs / tests / CI only
Review follow-up —
9ac7caedAddressed both non-blocking suggestions using the comment option proposed by the reviewer:
Output-budget threshold: explicitly cross-reference
ToolOutputBudgetMiddleware's_effective_trigger/_budget_contentand_patch_tool_message; changes must update the reader limit and its real-middleware budget regressions together.Position ceiling: document that
100_000matchesSubagentBatchesConfig.max_items_per_batch's schema ceiling. Retain the ceiling so lowering the configured submission cap does not hide previously accepted batch items.The follow-up adds six source-comment lines; the complete executable Python AST is identical to
8eafae30bc589778e6ea419bb18cae6a5b11c087. Official main4217b30e3bdcd2190cf4b885ca942f04ef4ef7b3was merged without conflicts. Its five frontend/changelog files are upstream changes, outside this PR's own diff, and the backend tree is identical before/after that merge. No runtime, configuration schema, permission, persistence or dependency behavior changes.| Check | SHA | Result |
|---|---|---|
| Reader + adjacent batch + real output-budget/config regression |
e18a271d| 366 passed, 1 optional PostgreSQL skip || Blocking-I/O |
e18a271d| 228 passed || Reader after upstream integration |
9ac7caed| 35 passed, 1 optional PostgreSQL skip || Format/lint, lock, diff and guidance | Follow-up heads | Passed; guidance soft warnings only |
| Independent AI review + whole-file AST equivalence |
9ac7caed| No findings; AST unchanged |Full-suite and real PostgreSQL measurements in the following section remain bound to
8eafae30bc589778e6ea419bb18cae6a5b11c087; they were not rerun or reassigned to this comment-only follow-up. No PostgreSQL server was started or contacted in this round.Validation —
86e91016Synced official
main00325790by a clean merge before this follow-up. All current results below bind to full head86e91016176e7380a2be9ab66ade2667dab9d510. The latest official baseline was also run in the same WSL/default-extra profile: 25,579 passed, 0 failed, 356 skipped; blocking-I/O 228 passed.CI connection regression and fix
The approved PostgreSQL 17 CI run at
9ac7caedreported 1 fixture errors:TypeError: connect() got an unexpected keyword argument 'sslmode'. The newly added fixture passed the libpq-styleTEST_POSTGRES_URIquery directly to the SQLAlchemy asyncpg driver. Earlier local PostgreSQL 16 acceptance used an address without that query and missed this mismatch; it did not establish CI compatibility.Both fixture setup and database reopen/cleanup now use
support.postgres.asyncpg_test_url: mapsslmodeto asyncpg'sssl, remove unsupportedchannel_binding, preserve escaped credentials/other query options, and reject conflicting TLS options. The helper only adapts test configuration; production connections, TLS policy and this PR's runtime behavior are unchanged. No workflow relaxation or PostgreSQL test skipping was added.Reproduction at the synchronized pre-fix tree with the same
?sslmode=disableURI shape: 1 PostgreSQL errors / 35 passing controls. The final PostgreSQL suite below is green. An intermediate proposal that discarded TLS modes failed 18 driver-contract cases / passed 3; the final 24-case dialect regression preserves all seven TLS choices, three URI schemes, escaped passwords, unrelated options and conflict handling.| Check at
86e91016| Result ||---|---|
| Real PostgreSQL acceptance: batch result reader and URI-driver regressions, server 18.6, CI-shaped URI | 60 passed, 0 failed |
| Focused regression, default locked extras | 218 passed, 1 skipped |
| Full backend suite, all four shards, sequential
TEST_JOBS=2| 25,638 passed, 0 failed, 357 skipped || Blocking-I/O | 228 passed |
| Ruff format / lint, dependency lock, diff and agent guidance | Passed; guidance soft warnings only |
| Independent AI review of the final fix | No remaining actionable findings |
Full-suite comparisons use default locked extras with no database URI; URI-gated cases are separately executed using the locked
postgresextra, not counted as passing when skipped. Real database acceptance ran against a disposable local PostgreSQL 18.6 cluster with UUID schemas and actual schema/reopen/teardown checks. The user's stopped PostgreSQL database was not started or accessed. GitHub uses PostgreSQL 17; its newly triggered checks are a separate result and are not claimed to pass from local tests alone. Deterministic models/executors replace paid provider work; these checks establish engineering contracts, not model quality or a live browser/provider experiment.Previously reported full-suite and PostgreSQL 16 results remain historical evidence at their original SHAs. This table supersedes them for the current head.
| Own diff category | Added | Deleted |
|---|---:|---:|
| Production code | 137 | 10 |
| Tests | 488 | 3 |
| Documentation | 25 | 2 |
| Total (16 files) | 650 | 15 |
Remote CI follow-up — 2026-10-07
Maintainer-approved GitHub Actions run Unit Tests, attempt 2 completed successfully for PR head
86e91016176e7380a2be9ab66ade2667dab9d510. All four backend test shards and default-install collection passed with the workflow's PostgreSQL 17 service and CI-shaped URI. The remote logs separately record the checkout/merge revision; these results are not inferred from local PostgreSQL 18 validation.At verification, 14 checks succeeded, 2 conditional Triage checks were skipped, and no checks were failing or pending. Lint, agent guidance, blocking-I/O, frontend unit tests, both Replay layers and CLA passed. The screenshot's partial check count reflected still-running jobs; this follow-up required no code changes or workflow relaxation. Maintainer review approval remains separate from CI success.
AI assistance
Tool(s) used: OpenAI Codex.
How you used it: Codex reproduced the issues, implemented the fixes and regression tests, ran validation, and prepared this description. A separate AI reviewer examined the changes. Human review responsibility remains with the contributor.