Skip to content

docs(harness): revise Phase 1 estimates from runner exploration - #3357

Merged
PattaraS merged 2 commits into
mainfrom
harness-seam-doc-estimates
Jul 29, 2026
Merged

docs(harness): revise Phase 1 estimates from runner exploration#3357
PattaraS merged 2 commits into
mainfrom
harness-seam-doc-estimates

Conversation

@PattaraS

@PattaraS PattaraS commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Now based directly on main — the 1.1→1.2→1.3 stack this was originally stacked on has all landed, so this diff is doc-only and ready to merge on its own.

Workstream progress

Modular native-harness registry (designs/harness-modular-registry-proposal.md). This is a docs-only planning update — no PR number of its own in the phase sequence; it revises the plan the other PRs execute against.

Related issue

N/A — planning-doc revision.

Summary

Reading the runner dispatch surface (rather than guessing from the coupling inventory) materially changed the shape of the remaining work. This updates the proposal's estimates and plan to match:

  • Split PR 1.5 into a serial runner sub-stack:
    • 1.5a spawn-env (Med, 2–3d) — the bounded first measurement; collapses the two duplicated 11-arm blocks behind one build_spawn_env(session_id, *, server_client, labels) hook.
    • 1.5b launch (High, 4–6d) — the epicenter. _auto_create_<x>_terminal has 11 divergent signatures (3 common params; claude carries 9 extras), so the seam passes a NativeLaunchContext dataclass to a uniform provider.auto_create_terminal(ctx) adapter with explicit pre_launch hooks — not a single positional call.
    • 1.5c terminal-route (Med, 2d) — reuses 1.5b's context; codex/antigravity need an ownership-check + response-wrap hook.
  • Re-scope 1.6 interrupt/stop upward (Med → Med–High, 2d → 3–4d): every handler closes over app-scope state (server_client, resource_registry, _publish_event, module dicts), so extraction needs a dependency-injection context, not a plain move.
  • Revise totals: Phase 1 ~17–25 → ~20–29 eng-days; overall ~26–37 → ~29–41 eng-days across ~12 → ~14 PRs; critical path rewritten to the serial runner chain (1.1 → 1.2 → 1.5a → 1.5b → 1.5c → 1.6 → 2.2 → 2.3).
  • Add a Calibration subsection recording the learning from 1.1–1.3 (additive PRs come in under estimate; the real cost is test-shape churn; the runner is the back-loaded risk) and settle the prior "signature uniformity" open question with the confirmed finding.
  • Record harness-bench compatibility — the bench's selection/driver layer is already registry-driven, so a community native plugin auto-enumerates; the two remaining gaps close via PR 1.7 (seeding) and a small 1.8 capability field. Give 2.4 a benchable acceptance criterion.
  • Mark the progress ledger 1.1/1.2/1.3 as landed (feat(harness): add NativeHarnessProvider seam foundation (PR 1.1) #3239/refactor(harness): normalize native launcher pass-through args (PR 1.2) #3244/refactor(harness): route native resume through the provider seam (PR 1.3) #3314) now that the stack has merged.

Test Plan

Docs-only. pre-commit run --files designs/harness-modular-registry-proposal.md passes. The runner claims (divergent _auto_create_* signatures, the two duplicated spawn-env blocks, closure-bound interrupt/stop handlers, the opencode turn-path cold-boot) were verified against omnigent/runner/app.py and omnigent/runner/native/orchestration.py at the current tree.

Demo

N/A — documentation only.

Type of change

  • Bug fix
  • Feature
  • UI / frontend change
  • Refactor / chore
  • Docs
  • Test / CI
  • Breaking change

Test coverage

  • Unit tests added / updated
  • Integration tests added / updated
  • E2E tests added / updated
  • Manual verification completed
  • Existing tests cover this change
  • Not applicable

Coverage notes

Documentation-only change to a design proposal; no code paths are affected. The factual claims about the runner dispatch surface were verified by reading the current source as noted in the Test Plan.

This pull request and its description were written by Isaac.

@github-actions github-actions Bot added the size/M Pull request size: M label Jul 27, 2026
@omnigent-ci

omnigent-ci Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review

Blocking issues

None. This is a docs-only revision to a design proposal; there are no code paths, contracts, or failure modes to break.

Security vulnerabilities

None. No code, dependencies, lockfiles, or extras are touched — the diff is a single Markdown file.

Non-blocking notes

I spot-checked the load-bearing factual claims against the current tree (omnigent/runner/app.py, omnigent/runner/native/orchestration.py), since the whole point of the PR is to replace guesses with source-grounded estimates. They hold up:

  • 11 divergent _auto_create_*_terminal signatures — confirmed. orchestration.py defines exactly 11 native variants (opencode, pi, cursor, goose, hermes, kiro, qwen, kimi, codex, antigravity, claude), plus a non-native _auto_create_repl_terminal. Divergence is real.
  • Two near-identical ~11-arm spawn-env blocks — confirmed, at roughly app.py ~2532–2624 and ~6063–6098 (the doc's "~2720 / ~6236" anchors are close enough for a planning doc but drift ~100–200 lines from the actual block starts; consider loosening to "~2.5k" / "~6.0k" so they don't rot as the file moves).
  • codex/antigravity ownership-check + response-wrap hooks — confirmed: _is_runner_owned_codex_terminal, _is_runner_owned_antigravity_terminal, and _codex_ensure_response_with_policy_notice all exist and are wired into the ensure_native_terminal path.
  • Interrupt/stop handlers close over app-scope state — confirmed: the handlers are nested inside create_runner_app, closing over server_client / resource_registry / _publish_event, with _AUTO_FORWARDER_TASKS and the _session_* dicts as module/enclosing state. The "needs DI, not a plain move" characterization is accurate.

Two small precision points, neither blocking:

  • The "claude carries 9 extras" count is asserted but the diff only enumerates four (bundle_dir, skills_filter, auth_token_factory, resolve_launch_config, "…"). If the "9" is load-bearing for the 1.5b estimate, spelling out the full list (or softening to "several") would keep the doc from being contradicted by a later reader who counts.
  • The line-number anchors throughout ("~2841–3316", "~6329", "~7462–7908") are inherently fragile in a doc that outlives the current tree. They're useful now; a one-line caveat that they're pinned to the current SHA would prevent future confusion.

No visual demonstration is required — this is a pure documentation change with no user-visible surface.

Summary

A clean, well-scoped docs-only update that revises the harness-registry proposal's estimates and plan to match what a real read of the runner dispatch surface revealed (1.5 split into 1.5a/b/c, 1.6 re-scoped upward, totals bumped to ~29–41 eng-days / ~14 PRs, plus a Calibration subsection). The substantive technical claims check out against the current source, the reasoning is sound and appropriately hedged ("measure 1.5a before committing a runner timeline"), and there is no code, security, or dependency risk. Approve; the only follow-ups are optional precision tweaks (the "9 extras" count and the fragility of hard-coded line anchors).

The review is already complete and posted; the cancelled sub-agent's result is no longer needed. Draining the inbox to clear it.The

The inbox is now drained — the sub-agent was cancelled after I had already gathered sufficient verification evidence from its working history. The review is complete and posted above. No further action needed.


Automated review by Polly · workflow run

@PattaraS
PattaraS force-pushed the harness-seam-1-3-pr branch from b913aff to 1c8568d Compare July 28, 2026 14:13
@PattaraS
PattaraS force-pushed the harness-seam-doc-estimates branch from 5283ceb to f03e74e Compare July 28, 2026 14:13
@PattaraS
PattaraS force-pushed the harness-seam-1-3-pr branch from 1c8568d to ffb923e Compare July 29, 2026 03:05
@PattaraS
PattaraS force-pushed the harness-seam-doc-estimates branch from f03e74e to f4a755f Compare July 29, 2026 03:05
@PattaraS
PattaraS force-pushed the harness-seam-1-3-pr branch from ffb923e to 7cedfc1 Compare July 29, 2026 03:32
@PattaraS
PattaraS force-pushed the harness-seam-doc-estimates branch from f4a755f to 422fb40 Compare July 29, 2026 03:32
@PattaraS
PattaraS force-pushed the harness-seam-1-3-pr branch from 7cedfc1 to 59b71cb Compare July 29, 2026 03:36
@PattaraS
PattaraS force-pushed the harness-seam-doc-estimates branch from 422fb40 to d28c6d0 Compare July 29, 2026 03:36
@PattaraS
PattaraS force-pushed the harness-seam-1-3-pr branch from 59b71cb to 70f3daa Compare July 29, 2026 03:52
@PattaraS
PattaraS force-pushed the harness-seam-doc-estimates branch from d28c6d0 to b9b7c2d Compare July 29, 2026 03:52
Base automatically changed from harness-seam-1-3-pr to main July 29, 2026 04:43
PattaraS added 2 commits July 29, 2026 12:45
Reading the runner dispatch surface (not guessing) changed the shape of the
remaining work, so update the proposal's estimates and plan:

- Split PR 1.5 into a serial runner sub-stack: 1.5a spawn-env (bounded, the
  first measurement), 1.5b launch (the epicenter — _auto_create_<x>_terminal
  has 11 divergent signatures, so the seam passes a NativeLaunchContext to a
  uniform provider.auto_create_terminal(ctx) adapter with pre_launch hooks,
  not a single positional call), 1.5c terminal-route.
- Re-scope 1.6 interrupt/stop upward (Med -> Med-High, 2d -> 3-4d): every
  handler closes over app-scope state (server_client, resource_registry,
  _publish_event, module dicts), so extraction needs a DI context, not a move.
- Revise totals: Phase 1 ~17-25 -> ~20-29 eng-days; overall ~26-37 -> ~29-41
  across ~12 -> ~14 PRs; critical path rewritten to the serial runner chain.
- Add a Calibration subsection recording the learning from 1.1-1.3 (additive
  PRs come in under estimate; the real cost is test-shape churn; the runner is
  the back-loaded risk) and settle the "signature uniformity" open question
  with the confirmed finding.

Docs-only; no code paths affected.

Co-authored-by: Isaac
Signed-off-by: Pat Sukprasert <pattara.sk127@gmail.com>
The harness bench's selection + driver layer is already registry-driven:
manifest.py auto-adds every NATIVE_TUI capability as a BenchProfile and the
NativeTuiDriver is selected generically, so a community native plugin
enumerates and gets a profile with zero bench edits. Record the two remaining
gaps and where they close:

- Provisioning needs registry-driven agent seeding — closed for free by PR 1.7
  (the native driver provisions against a pre-seeded <harness>-ui agent).
- Tool-call probe metadata is hardcoded (_NATIVE_TOOL_PROVOCATION) — fold
  optional shell_tool_name / shell_tool_prompt capability fields into PR 1.8 so
  the probe reads off the registry; until then those probes skip (non-fatal).

Add a "Harness bench compatibility" subsection, extend 1.8's scope with the
tool-probe fields, and give 2.4 a benchable acceptance criterion (the example
plugin runs `python -m tests.harness_bench --harness <plugin> --live` green).
No new phase or standalone bench-migration PR.

Co-authored-by: Isaac
Signed-off-by: Pat Sukprasert <pattara.sk127@gmail.com>
@PattaraS
PattaraS force-pushed the harness-seam-doc-estimates branch from b9b7c2d to 54aaeb2 Compare July 29, 2026 04:47
Copilot AI review requested due to automatic review settings July 29, 2026 04:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@PattaraS
PattaraS merged commit 1c23bf7 into main Jul 29, 2026
62 checks passed
@PattaraS
PattaraS deleted the harness-seam-doc-estimates branch July 29, 2026 05:08
@github-actions github-actions Bot added the no-doc-update Merged PR does not need a docs update label Jul 29, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🏷️ Doc impact: no-doc-update

The change only edits an internal design/proposal doc under designs/ (planning, PR sequencing, and effort estimates) with no code, user-facing surface, integration, or built-in policy change.

Auto-classified on merge. Set the label manually before merging to override. · run

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

Labels

no-doc-update Merged PR does not need a docs update size/M Pull request size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants