feat(harness): per-model system prompts with engine-first discovery#227
Conversation
Remove all directory::* integration from the agent loop: the system prompt no longer injects directory skills — agents learn iii from the live engine, with engine::functions::info as the mandatory API reference before every call. Delete the bootstrap skill download, skills index/body fetchers, and the system_default_skills config; re-point function_not_found hints to engine::functions::list; allow the read-only engine::* / worker::list surface in iii-permissions.yaml (mutating worker::* ops stay gated). Add opencode-style per-model prompts (src/turn-orchestrator/prompt/): anthropic (canonical sectioned voice), gpt (persistence/autonomy voice), kimi (MUST imperatives + Ultimate Reminders), default (step-by-step for small local models), routed by the run's provider/model through provider-router.decide(). A shared invariant suite enforces the same 16-rule contract on every variant; routing, style, and assembly are covered by 103 tests.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request transitions the turn-orchestrator from skill-index-based system prompts to model-specific engine-centric prompts. It removes skill bootstrapping, simplifies provisioning to build prompts from live engine context, updates permissions to use ChangesEngine-centric system prompt and discovery refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
skill-check — worker0 verified, 14 skipped (no docs/).
Four for four. Nicely done. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@harness/docs/architecture.md`:
- Around line 242-244: The docs currently list approval-gated mutating worker
ops as `worker::add`, `worker::start`, `worker::stop`, `worker::remove`,
`worker::clear` but omit `worker::update`; update the text so the approval-gated
mutating-op list includes `worker::update` alongside the other `worker::*` ops
(also keep `worker::list` and `engine::registered-triggers::*` references
unchanged) to match the prompt lifecycle contract.
In `@harness/src/turn-orchestrator/prompt/anthropic.ts`:
- Around line 43-48: The rule that mandates fetching `engine::functions::info`
"before calling ANY function" conflicts with the prohibition on introspecting
discovery calls; change the rule in the prompt text that references "before any
call" so it explicitly applies only to non-discovery target functions (e.g., any
function that is not a discovery endpoint such as `engine::functions::info` or
other `engine::*`/`worker::*` discovery calls). Update the wording around the
"before any call" requirement in the `anthropic` prompt so it: 1) excludes
discovery endpoints by name (mention `engine::functions::info` as an example),
and 2) instructs callers to skip discovery-introspection and instead use `list`
results to select actual function IDs; keep the prohibition language about not
passing discovery call IDs (e.g., `engine::functions::info`) intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab32bef1-f74d-4727-9dde-d53386370c8a
📒 Files selected for processing (22)
harness/docs/architecture.mdharness/docs/workers/turn-orchestrator.mdharness/src/turn-orchestrator/agent-trigger.tsharness/src/turn-orchestrator/bootstrap.tsharness/src/turn-orchestrator/config.tsharness/src/turn-orchestrator/prompt/anthropic.tsharness/src/turn-orchestrator/prompt/default.tsharness/src/turn-orchestrator/prompt/gpt.tsharness/src/turn-orchestrator/prompt/index.tsharness/src/turn-orchestrator/prompt/kimi.tsharness/src/turn-orchestrator/provisioning/load-skills.tsharness/src/turn-orchestrator/provisioning/ports.tsharness/src/turn-orchestrator/provisioning/process.tsharness/src/turn-orchestrator/register.tsharness/src/turn-orchestrator/system-prompt.tsharness/src/types/provider.tsharness/tests/turn-orchestrator/agent-trigger.test.tsharness/tests/turn-orchestrator/config.test.tsharness/tests/turn-orchestrator/provisioning-layer.test.tsharness/tests/turn-orchestrator/provisioning.test.tsharness/tests/turn-orchestrator/system-prompt.test.tsiii-permissions.yaml
💤 Files with no reviewable changes (4)
- harness/tests/turn-orchestrator/config.test.ts
- harness/src/turn-orchestrator/bootstrap.ts
- harness/src/turn-orchestrator/provisioning/load-skills.ts
- harness/src/turn-orchestrator/config.ts
| `engine::registered-triggers::*`), and `worker::list`. Mutating | ||
| `worker::*` ops (`add`, `start`, `stop`, `remove`, `clear`) stay | ||
| approval-gated. |
There was a problem hiding this comment.
Include worker::update in the approval-gated mutating-op list.
Line 243 omits update, but the lifecycle contract in prompt docs includes it as a mutating worker::* operation. This mismatch can mislead operators about what stays approval-gated.
Suggested docs fix
-`worker::*` ops (`add`, `start`, `stop`, `remove`, `clear`) stay
+`worker::*` ops (`add`, `start`, `stop`, `update`, `remove`, `clear`) stay
approval-gated.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `engine::registered-triggers::*`), and `worker::list`. Mutating | |
| `worker::*` ops (`add`, `start`, `stop`, `remove`, `clear`) stay | |
| approval-gated. | |
| `engine::registered-triggers::*`), and `worker::list`. Mutating | |
| `worker::*` ops (`add`, `start`, `stop`, `update`, `remove`, `clear`) stay | |
| approval-gated. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@harness/docs/architecture.md` around lines 242 - 244, The docs currently list
approval-gated mutating worker ops as `worker::add`, `worker::start`,
`worker::stop`, `worker::remove`, `worker::clear` but omit `worker::update`;
update the text so the approval-gated mutating-op list includes `worker::update`
alongside the other `worker::*` ops (also keep `worker::list` and
`engine::registered-triggers::*` references unchanged) to match the prompt
lifecycle contract.
| got from \`list\`. NEVER pass \`engine::functions::info\` itself or any \`engine::*\` / | ||
| \`worker::*\` discovery call as the id — that just returns | ||
| metadata ABOUT the info function (worker \`iii-engine-functions\`, no registered | ||
| triggers), which is useless and a sign you introspected the wrong thing. The discovery | ||
| calls are documented here; never introspect them. Omitting \`function_id\` errors with | ||
| \`missing field \`function_id\`\`. |
There was a problem hiding this comment.
Resolve the conflicting contract-discovery rule.
Line 90 says to fetch engine::functions::info before calling ANY function, but Lines 43-48 forbid introspecting discovery calls. That creates an impossible first step for discovery and can cause tool-selection deadlocks. Please explicitly scope the “before any call” rule to non-discovery target functions.
Suggested wording update
-RULE 2 — BEFORE you call ANY function, fetch its contract from the engine by passing that
-function's id as `function_id` to `engine::functions::info`.
+RULE 2 — BEFORE you call any NON-DISCOVERY target function, fetch its contract from the
+engine by passing that target function's id as `function_id` to `engine::functions::info`.
+Discovery calls documented in this prompt (for example `engine::functions::list`,
+`engine::workers::*`, `engine::triggers::*`, `engine::registered-triggers::*`, `worker::list`)
+are the explicit exception and may be called directly.Also applies to: 90-92
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@harness/src/turn-orchestrator/prompt/anthropic.ts` around lines 43 - 48, The
rule that mandates fetching `engine::functions::info` "before calling ANY
function" conflicts with the prohibition on introspecting discovery calls;
change the rule in the prompt text that references "before any call" so it
explicitly applies only to non-discovery target functions (e.g., any function
that is not a discovery endpoint such as `engine::functions::info` or other
`engine::*`/`worker::*` discovery calls). Update the wording around the "before
any call" requirement in the `anthropic` prompt so it: 1) excludes discovery
endpoints by name (mention `engine::functions::info` as an example), and 2)
instructs callers to skip discovery-introspection and instead use `list` results
to select actual function IDs; keep the prohibition language about not passing
discovery call IDs (e.g., `engine::functions::info`) intact.
Summary
Two coupled changes to the agent system prompt, following the opencode
session/prompt/*architecture:1. Remove all
directory::*integration — agents learn iii from the live engineengine::functions::infois the mandatory API reference before every call, and the prompt teaches the full engine discovery surface (engine::functions::*,engine::workers::*incl.workers::info,engine::triggers::*,worker::*lifecycle withyes: trueconsent).bootstrap.ts(skills download at boot),turn-orchestrator/config.ts+system_default_skills,provisioning/load-skills.ts, thedirectory::skills::index/getfetchers inports.ts.agent_triggertool description andfunction_not_foundhints re-pointed toengine::functions::list.iii-permissions.yaml: the 14 orphaneddirectory::*allow rules replaced with the read-onlyengine::*surface +worker::list; mutatingworker::*ops stay approval-gated.2. Per-model prompt variants (
src/turn-orchestrator/prompt/)anthropic.ts<example>blocksgpt.tskimi.tsdefault.tsRouting reuses
provider-router.decide()(prompt/index.ts, mirroring opencode'ssystem.ts:provider());buildSystemPrompt({override, mode, provider, model})is threaded through provisioning. A shared invariant suite enforces the same 16-rule contract on every variant so no battle-tested rule can silently drop from one family.Adversarially verified: instruction parity per variant against the canonical prompt, voice fidelity vs the opencode references, factual accuracy of every engine claim against the Rust handlers, and mutation-tested coverage.
Test plan
pnpm exec vitest run— 1103/1103 (103 prompt tests: invariant ×4 variants, routing heuristics, style anchors, assembly/mode)src/index.ts:75,src/models-catalog/main.ts:8)directory::/skill residualsSummary by CodeRabbit
Release Notes
New Features
Improvements
Documentation