Skip to content

refactor(harness): fork_history + shell-tool capability axes; derive gating (PR 1.8) - #3648

Merged
PattaraS merged 1 commit into
omnigent-ai:mainfrom
PattaraS:harness-seam-1-8-pr
Jul 31, 2026
Merged

refactor(harness): fork_history + shell-tool capability axes; derive gating (PR 1.8)#3648
PattaraS merged 1 commit into
omnigent-ai:mainfrom
PattaraS:harness-seam-1-8-pr

Conversation

@PattaraS

Copy link
Copy Markdown
Contributor

Related issue

N/A — final Phase-1 PR of the modular native-harness registry workstream (designs/harness-modular-registry-proposal.md).

Summary

PR 1.8 derives registry-parallel enumerations from HarnessCapabilities instead of
hand-maintained literals, and adds the two capability axes the workstream reserved for this
step. Disjoint hub (depends on 1.1).

  • New fork_history capability axis (ForkHistory enum: none/rebuild/preamble) on
    HarnessCapabilities, declared per harness in _BUILTIN_CAPABILITIES. The server's two
    fork-history gating frozensets in _sessions/common.py
    (_FORK_HISTORY_NATIVE_HARNESSES, _CURSOR_FORK_HISTORY_HARNESSES) are now derived from
    it instead of hand-listed.
  • New shell_tool_name / shell_tool_prompt capability fields (optional) carrying the
    harness bench's shell-tool provocation. The bench's hardcoded _NATIVE_TOOL_PROVOCATION
    table is deleted; native_vendor() reads the fields off capabilities (it already read
    harness_capabilities()).
  • Deleted the dead _HARNESS_MODULES literal in runtime/harnesses/__init__.py (~120
    lines, overwritten unconditionally by harness_modules() on the next line — confirmed no
    read in between).
  • Drift-guard tests extend the established test_harness_capabilities.py pattern.

ELI5

"Which harnesses carry chat history across a fork?" and "what shell command does the bench
poke each harness with?" were answered by copy-pasted lists that drift when a harness is added.
This moves both answers onto each harness's capability record and computes the lists from it,
so a new harness declares its behavior in one place.

Test Plan

uv run pytest tests/test_harness_capabilities.py tests/test_harness_plugins.py \
  tests/server/routes/ -q
uv run pre-commit run --all-files
  • Verified the derived fork-history sets are byte-equal to the prior canonical-id
    membership
    (rebuild: claude/codex/hermes/pi/qwen; preamble: cursor/opencode), and the
    bench native_vendor() yields the same (tool_name, tool_prompt) per harness as the
    deleted table (spot-checked all 9 + cursor=None).
  • New tests: test_fork_history_matches_derived_gating_sets,
    test_fork_history_derivation_preserves_prior_membership,
    test_registered_native_aliases_still_classify_fork_history,
    test_native_tui_harnesses_declare_shell_tool_provocation; updated
    test_optional_bench_capabilities_default_to_unknown for the new as_dict() keys.

Demo

N/A — internal capability-model refactor, no user-facing or UI change.

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

Behavior-preserving derivation. One subtlety worth flagging for reviewers:

Reversed native-<x> spellings must be preserved in the derived sets.
native-claude / native-codex / native-cursor are valid harness ids that
canonicalize_harness passes through unchanged (they are not registered aliases that
normalize to *-native). The read sites match on canonicalize_harness(harness_kind) in SET,
so the sets must contain those literal reversed spellings or an identically-behaving agent
silently loses fork history — there is an explicit test for exactly this
(test_sessions_fork.py::test_fork_reversed_native_spelling_carry_gating). The derivation
therefore emits both the canonical id and its native-<key> spelling per fork-history harness,
so the derived sets are a superset of the prior literals (extra reversed spellings that
canonicalize into the set are harmless at the read site). Pinned by
test_reversed_native_spellings_classify_fork_history +
test_fork_history_derivation_preserves_prior_membership.

Scope was kept tight to the doc's mandate. Other registry-parallel sets that would need
new NativeCodingAgent identity fields (_ANTIGRAVITY_FAMILY_HARNESSES,
_PROVIDER_RESOLUTION_HARNESS, the *_NATIVE_TERMINAL_ROLE constants) are left as-is — they
function and converting them touches model routing / runner hot paths. Noted as follow-up
candidates.


Workstream progress (designs/harness-modular-registry-proposal.md, ~14 PRs):
Phase 1 — 1.1 (#3239), 1.2 (#3244), 1.3 (#3314), 1.5a (#3495), 1.5b-i (#3500), 1.5b-ii
(#3501), 1.5c (#3543), 1.6 (#3568) landed; 1.7 (#3599) in review; 1.8 is this PR — the last
Phase-1 item.
Next: Phase 2 (2.1 validator flip → 2.2 /v1/harnesses native rows → 2.3 web
off endpoint → 2.4 docs + example plugin).

Copilot AI review requested due to automatic review settings July 31, 2026 04:00

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.

@github-actions github-actions Bot added the size/L Pull request size: L label Jul 31, 2026
…gating (PR 1.8)

Final Phase-1 PR of the modular native-harness registry refactor: move
registry-parallel enumerations onto HarnessCapabilities.

- Add a fork_history axis (ForkHistory enum: none/rebuild/preamble) to
  HarnessCapabilities, declared per harness in _BUILTIN_CAPABILITIES. Derive the
  server's two fork-history gating frozensets in _sessions/common.py from it
  instead of hand-listing. The derivation emits each canonical id plus its
  reversed native-<key> spelling, because native-claude/native-codex/native-cursor
  are valid ids canonicalize_harness passes through unchanged and the read sites
  match on the canonicalized id (guarded by the existing reversed-spelling fork
  test) — so the derived sets are a superset of the prior literals.
- Add optional shell_tool_name / shell_tool_prompt fields carrying the harness
  bench's shell-tool provocation; delete the bench's hardcoded
  _NATIVE_TOOL_PROVOCATION table and read the fields off capabilities in
  native_vendor() (byte-identical (tool_name, prompt) per harness).
- Delete the dead _HARNESS_MODULES literal in runtime/harnesses/__init__.py
  (~120 lines, overwritten unconditionally by harness_modules() next line).
- Extend the drift-guard tests in test_harness_capabilities.py.

Scope kept tight to the doc's mandate: sets that would need new NativeCodingAgent
identity fields (_ANTIGRAVITY_FAMILY_HARNESSES, _PROVIDER_RESOLUTION_HARNESS,
*_NATIVE_TERMINAL_ROLE) are left as-is; noted as follow-ups.

Co-authored-by: Isaac
Signed-off-by: Pat Sukprasert <pattara.sk127@gmail.com>
@PattaraS
PattaraS force-pushed the harness-seam-1-8-pr branch from a9ba65d to 3c3c0ea Compare July 31, 2026 05:42
Copilot AI review requested due to automatic review settings July 31, 2026 05:42

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

Copy link
Copy Markdown
Contributor Author

/review

@PattaraS
PattaraS enabled auto-merge (squash) July 31, 2026 05:51
@omnigent-ci

omnigent-ci Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Polly AI Review

Review: refactor(harness): fork_history + shell-tool capability axes (PR 1.8)

I verified the derivation logic, the read sites in _sessions/helpers.py, all per-harness capability declarations against the deleted tables, and the dead-code deletion in runtime/harnesses/__init__.py.

Blocking issues

None. I confirmed the two claims that matter for correctness:

  1. Derived fork-history sets preserve prior membership. REBUILD declarations = {claude, codex, pi, qwen, hermes}-native; PREAMBLE = {cursor, opencode}-native. The derivation emits each canonical id plus its native-<key> spelling, yielding a superset of the pre-1.8 literals. The extra reversed spellings the derivation adds beyond the old literals — native-pi, native-qwen — are registered aliases that canonicalize_harness folds back to pi-native/qwen-native, so they're inert dead entries at the canonicalize_harness(...) in SET read site (never matched), i.e. harmless as claimed.

  2. The un-aliased reversed spellings are covered. native-claude, native-codex, native-cursor are not in HARNESS_ALIASES, so they pass through canonicalize_harness unchanged and must be literal set members — the derivation adds exactly these (their <key>-native forms declare REBUILD/PREAMBLE), and test_reversed_native_spellings_classify_fork_history pins it. Correct.

  3. Shell-tool table move is faithful. Spot-checked all 10 NATIVE_TUI harnesses: (shell_tool_name, prompt) on capabilities equals the deleted _NATIVE_TOOL_PROVOCATION rows (claude/pi/kimi→Bash+_BASH_PROMPT; codex/kiro→shell, qwen→run_shell_command, goose→developer__shell, hermes→terminal, antigravity→run_command, all _SHELL_PROMPT), cursor-native→None (was absent). native_vendor()'s or "" fallback preserves the "empty ⇒ skip probe" gate. opencode-native is NATIVE_SERVER, so native_vendor() early-returns before touching its (None) shell fields.

  4. Dead _HARNESS_MODULES literal deletion is safe. The literal was unconditionally overwritten by _HARNESS_MODULES = harness_modules() on the following line with no intervening read; the replacement keeps the mutable-dict surface tests mutate. No callers broken.

Security vulnerabilities

None. No trust-boundary, deserialization, or injection surface is touched; this is an internal capability-model refactor.

Non-blocking notes

  • Import-time computation. _FORK_HISTORY_NATIVE_HARNESSES / _CURSOR_FORK_HISTORY_HARNESSES are now computed once at common.py import via harness_capabilities(), versus the old static literals. This is consistent with common.py's existing import-time dependency on harness_plugins, and a community plugin declaring fork_history after this module loads wouldn't be reflected — but that's no worse than the prior hand-listed literals (which couldn't reflect plugins at all). Worth a mention only if plugin-declared fork-history gating becomes a goal.
  • The derivation keys off the -native suffix to synthesize reversed spellings; a future non-native harness declaring REBUILD/PREAMBLE would get its canonical id in the set but no reversed spelling. Fine today (all such harnesses are native); a brief assertion or comment guarding that assumption would future-proof it.

Summary

A tight, well-scoped Phase-1 refactor that moves two answers ("does a fork carry history?" and "what shell tool does the bench poke?") onto each harness's capability record and derives the previously hand-maintained gating sets and bench table from them. The derivation is provably a superset of the prior literals, the tricky un-aliased reversed spellings are handled and explicitly tested, and the drift-guard tests lock the behavior-preservation contract. No user-visible surface, so the absent demo is appropriate. Ship it.


Automated review by Polly · workflow run

@PattaraS
PattaraS merged commit 9a3c905 into omnigent-ai:main Jul 31, 2026
56 checks passed
@github-actions github-actions Bot added the no-doc-update Merged PR does not need a docs update label Jul 31, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🏷️ Doc impact: no-doc-update

Internal refactor deriving fork-history/shell-tool capability axes from the harness registry (no new harness, no user-facing config or documented default changed), so no doc update is needed.

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

PattaraS added a commit that referenced this pull request Jul 31, 2026
…ospective (#3658)

Phase 1 of the modular native-harness registry refactor landed (10 PRs,
2026-07-28 → 07-31). Bring the design doc in line with what actually shipped:

- Status header, Phase 1 subtotal, effort summary, and bottom line updated from
  forward-looking ('1.1–1.3 in review') to Phase 1 complete / Phase 2 next.
- Ledger: 1.8 (#3648) landed; 1.4 marked descoped (with rationale); the 1.7
  opencode-e2e follow-up (#3656) recorded; per-PR merge dates added.
- Calibration rewritten as a Phase 1 retrospective: estimate (~20–29 eng-days)
  vs. actual (10 PRs / 4 calendar days), the real cost centers (test-shape churn
  + review-caught behavior bugs, enumerated per PR), the correct runner re-scope,
  the two intentional behavior deltas (qwen label, antigravity relay), and the
  recurring uv.lock / full-suite-only-flake operational friction.

Doc-only; no code change.

Co-authored-by: Isaac

Signed-off-by: Pat Sukprasert <pattara.sk127@gmail.com>
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/L Pull request size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants