(MOT-3900) feat(harness): compose system prompt from worker instructions#447
(MOT-3900) feat(harness): compose system prompt from worker instructions#447ytallo wants to merge 2 commits into
Conversation
Workers now contribute their own prompt sections: a worker publishes markdown as metadata.agent_instructions on the configuration entry it registers, and the harness appends it to every system prompt (sorted, under a per-worker heading) while that worker has functions registered. Operators get a dedicated 'instructions' configuration entry with a 'global' text plus per-worker overrides, hot-reloaded via an unfiltered configuration trigger and editable from the console configuration page. Composition order: mode paragraph, identity prompt, worker sections (worker text then operator text), global operator instructions, then the per-send enrich prompt. 'override' still serves the caller prompt verbatim. The coder/code-editing guidance moves out of the harness default prompt and all provider identity prompts into the shell worker's declared instructions, so it is only served when the shell worker is actually running and no longer duplicated across prompts.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
skill-check — worker0 verified, 39 skipped (no docs/).
Four for four. Nicely done. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a hot-reloadable instructions cache, threads worker and operator instructions into system-prompt assembly, and moves coder::* file-operation guidance into shell-worker instructions with matching prompt, config, test, and documentation updates. ChangesInstructions subsystem and prompt composition
Relocation of coder:: guidance to shell worker*
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ConfigTrigger
participant InstructionsHandler
participant IIIClient
participant InstructionsCell
participant SendStart
participant Prompt
ConfigTrigger->>InstructionsHandler: configuration registered/updated/deleted
InstructionsHandler->>IIIClient: fetch_snapshot
IIIClient-->>InstructionsHandler: worker metadata + operator entry
InstructionsHandler->>InstructionsCell: update cached snapshot
SendStart->>InstructionsCell: instructions()
SendStart->>Prompt: resolve_system_prompt(SystemPromptOpts{mode, identity, sections, user_global})
Prompt-->>SendStart: composed system prompt
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
harness/src/subagent.rs (1)
136-148: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the
FunctionsSnapshotbinding to avoid shadowing.
let functions = deps.functions().await;(line 138,Arc<FunctionsSnapshot>) is shadowed a few lines later bylet functions = match parent_record { ... };(the function-policyOption<FunctionPolicy>, line 141). Both are valid but unrelated types sharing one name in the same function — safe today, but a maintainability trap for the next edit that needs the registry snapshot after the policy shadow point.♻️ Suggested rename
let instructions = deps.instructions().await; - let functions = deps.functions().await; - let sections = crate::instructions::live_sections(&instructions, &functions); + let functions_snapshot = deps.functions().await; + let sections = crate::instructions::live_sections(&instructions, &functions_snapshot);🤖 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/subagent.rs` around lines 136 - 148, Rename the `FunctionsSnapshot` binding in `subagent::...` so it no longer shares the `functions` name with the later policy variable. Keep the `deps.functions().await` result under a distinct name, and update the `live_sections` call and any other uses in this function accordingly. Ensure the later `match parent_record` result remains clearly named as the function policy to avoid shadowing and confusion in future edits.harness/src/prompt/tests.rs (1)
89-102: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider sharing the
sections()test fixture.The same
WorkerSectionfixture is duplicated near-identically across this file,harness/src/functions/send.rs(test module), andharness/src/subagent.rs(test module). Extracting it into a shared test-support helper would reduce drift risk if the fixture shape changes.🤖 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/prompt/tests.rs` around lines 89 - 102, The WorkerSection test fixture returned by sections() is duplicated across multiple test modules, so update the tests to use a shared helper instead of maintaining separate copies. Extract the common fixture into a test-support utility that can be reused by harness/src/prompt/tests.rs, the send.rs test module, and the subagent.rs test module, and then switch sections() and the other tests to reference that shared builder/fixture so future WorkerSection shape changes stay consistent.
🤖 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/README.md`:
- Around line 174-180: The Worker sections description should say “running”
instead of “installing,” since the guidance is injected only while the worker is
live. Update the wording in the README section that describes how
`metadata.agent_instructions` is added via `configuration::register` and the `#
Notes from the \`<id>\` worker` heading so it clearly states the `coder::*`
guidance appears when an `<entry_id>::` function is registered and disappears
when it stops.
In `@harness/src/instructions.rs`:
- Around line 118-133: The `get_entry_value` helper is using substring matching
on the error text from `trigger_with_retry`, which can misclassify unrelated
failures as missing entries. Update `trigger_with_retry` and/or
`get_entry_value` to preserve and inspect the structured SDK error code, then
branch on the actual `NOT_FOUND` code instead of `contains("NOT_FOUND")`; keep
`should_seed` unchanged except for using the corrected missing-entry behavior.
---
Nitpick comments:
In `@harness/src/prompt/tests.rs`:
- Around line 89-102: The WorkerSection test fixture returned by sections() is
duplicated across multiple test modules, so update the tests to use a shared
helper instead of maintaining separate copies. Extract the common fixture into a
test-support utility that can be reused by harness/src/prompt/tests.rs, the
send.rs test module, and the subagent.rs test module, and then switch sections()
and the other tests to reference that shared builder/fixture so future
WorkerSection shape changes stay consistent.
In `@harness/src/subagent.rs`:
- Around line 136-148: Rename the `FunctionsSnapshot` binding in `subagent::...`
so it no longer shares the `functions` name with the later policy variable. Keep
the `deps.functions().await` result under a distinct name, and update the
`live_sections` call and any other uses in this function accordingly. Ensure the
later `match parent_record` result remains clearly named as the function policy
to avoid shadowing and confusion in future edits.
🪄 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: da98b81d-a86f-40f1-afb3-030dc9a221b9
📒 Files selected for processing (20)
harness/README.mdharness/prompts/cli.txtharness/prompts/default.txtharness/src/configuration.rsharness/src/deps.rsharness/src/discovery.rsharness/src/functions/send.rsharness/src/instructions.rsharness/src/lib.rsharness/src/main.rsharness/src/prompt/mod.rsharness/src/prompt/tests.rsharness/src/subagent.rsprovider-anthropic/prompts/identity.txtprovider-openai-codex/prompts/identity.txtprovider-openai/prompts/identity.txtprovider-xai/prompts/identity.txtshell/prompts/agent-instructions.txtshell/src/configuration.rsworker-readme.md
💤 Files with no reviewable changes (4)
- provider-openai/prompts/identity.txt
- provider-anthropic/prompts/identity.txt
- provider-xai/prompts/identity.txt
- provider-openai-codex/prompts/identity.txt
| 1. **Worker sections.** A worker publishes agent guidance by setting | ||
| `metadata.agent_instructions` (a markdown string) on the configuration entry | ||
| it registers (`configuration::register`). The section is injected — under a | ||
| `# Notes from the \`<id>\` worker` heading, sorted by entry id — only while | ||
| the worker is live (some `<entry_id>::` function is registered), so | ||
| installing the shell worker adds its `coder::*` guidance and stopping it | ||
| removes it. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use “running” rather than “installing.”
The guidance is injected only while the worker is live; “installing” makes the trigger condition sound broader than it is.
Suggested fix
- so installing the shell worker adds its `coder::*` guidance and stopping it removes it.
+ so running the shell worker adds its `coder::*` guidance and stopping it removes it.📝 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.
| 1. **Worker sections.** A worker publishes agent guidance by setting | |
| `metadata.agent_instructions` (a markdown string) on the configuration entry | |
| it registers (`configuration::register`). The section is injected — under a | |
| `# Notes from the \`<id>\` worker` heading, sorted by entry id — only while | |
| the worker is live (some `<entry_id>::` function is registered), so | |
| installing the shell worker adds its `coder::*` guidance and stopping it | |
| removes it. | |
| 1. **Worker sections.** A worker publishes agent guidance by setting | |
| `metadata.agent_instructions` (a markdown string) on the configuration entry | |
| it registers (`configuration::register`). The section is injected — under a | |
| `# Notes from the \`<id>\` worker` heading, sorted by entry id — only while | |
| the worker is live (some `<entry_id>::` function is registered), so | |
| running the shell worker adds its `coder::*` guidance and stopping it | |
| removes it. |
🤖 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/README.md` around lines 174 - 180, The Worker sections description
should say “running” instead of “installing,” since the guidance is injected
only while the worker is live. Update the wording in the README section that
describes how `metadata.agent_instructions` is added via
`configuration::register` and the `# Notes from the \`<id>\` worker` heading so
it clearly states the `coder::*` guidance appears when an `<entry_id>::`
function is registered and disappears when it stops.
| async fn should_seed(iii: &IIIClient) -> Result<bool, String> { | ||
| match get_entry_value(iii, INSTRUCTIONS_CONFIG_ID).await? { | ||
| None => Ok(true), | ||
| Some(v) if v.is_null() => Ok(true), | ||
| Some(_) => Ok(false), | ||
| } | ||
| } | ||
|
|
||
| /// `configuration::get` tolerant of a missing entry (codes vary in case). | ||
| async fn get_entry_value(iii: &IIIClient, id: &str) -> Result<Option<Value>, String> { | ||
| match trigger_with_retry(iii, "configuration::get", json!({ "id": id })).await { | ||
| Ok(resp) => Ok(resp.get("value").cloned()), | ||
| Err(e) if e.to_ascii_uppercase().contains("NOT_FOUND") => Ok(None), | ||
| Err(e) => Err(e), | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does the iii-sdk Rust crate's configuration::get trigger response use a stable/typed NOT_FOUND error code, or free-form error text?
💡 Result:
In the iii-sdk Rust crate, invocations (including triggers like configuration::get) are performed using the generic iii.trigger() method [1][2]. The SDK utilizes a structured message protocol for communication between the worker and the engine [3][4]. When a function invocation results in an error, the engine communicates this back to the SDK. Based on the SDK's internal message protocol—which is consistent across its Rust, Node.js, and other implementations—error responses are structured as objects typically containing both a code (string) and a message (string) [4]. Therefore, configuration::get and other triggers do not return free-form error text; they use a typed, structured error response format that includes a stable, machine-readable error code [4]. For example, when handling errors in a custom implementation, you would typically expect to handle an object with the following shape [4]: { "code": "SOME_ERROR_CODE", "message": "Human-readable description" } As configuration::get is a system-level function invoked via the standard trigger interface, any NOT_FOUND condition would return a structured error with a specific NOT_FOUND (or similarly typed) error code [5][4].
Citations:
- 1: https://crates.io/crates/iii-sdk
- 2: https://docs.rs/crate/iii-sdk/latest
- 3: https://iii.dev/docs/sdk-reference/node-sdk
- 4: https://www.mintlify.com/iii-hq/sdk/concepts/triggers
- 5: https://workers.iii.dev/workers/configuration
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant functions and any error-shape handling around trigger_with_retry.
rg -n "trigger_with_retry|get_entry_value|should_seed|NOT_FOUND|configuration::get|code.*message|message.*code" harness/src -S
# Map the file structure for the target file before reading slices.
ast-grep outline harness/src/instructions.rs --view expandedRepository: iii-hq/workers
Length of output: 4851
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant section with line numbers.
sed -n '1,220p' harness/src/instructions.rs | cat -nRepository: iii-hq/workers
Length of output: 10852
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the trigger retry wrapper and the error formatting used by the harness.
sed -n '1,260p' harness/src/configuration.rs | cat -n
printf '\n---- error.rs ----\n'
sed -n '1,220p' harness/src/error.rs | cat -n
printf '\n---- engine.rs excerpt ----\n'
sed -n '1,240p' harness/src/clients/engine.rs | cat -nRepository: iii-hq/workers
Length of output: 24355
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the retry wrapper and any helper that stringifies errors.
sed -n '300,420p' harness/src/configuration.rs | cat -n
printf '\n---- other NOT_FOUND matches ----\n'
rg -n "NOT_FOUND|to_ascii_uppercase\\(\\)\\.contains\\(\"NOT_FOUND\"\\)" harness/src -SRepository: iii-hq/workers
Length of output: 3539
🌐 Web query:
iii-sdk Rust trigger error Display format code message NOT_FOUND configuration get error shape
💡 Result:
In the iii-sdk Rust environment, a NOT_FOUND error returned during configuration or trigger registration typically indicates that the requested identifier (such as a configuration ID or trigger type) has not been correctly registered with the engine before being accessed [1][2]. Key details regarding this error: 1. Identification of the Error: The NOT_FOUND code explicitly indicates that a get or schema call was performed against an ID that has not yet been registered [1]. In the context of trigger registration, this often occurs because the required trigger worker (e.g., HTTP, cron, stream) is not active or has not been properly registered in your project [2]. 2. Troubleshooting Steps: - Verify Registration: Ensure that you have called the necessary registration functions (e.g., register_worker, register_trigger, or register_trigger_type) before attempting to invoke or configure the resource [1][2]. - Check Trigger Worker Availability: If you receive this during trigger registration, confirm that the specific trigger type (like http or cron) is supported and that the corresponding worker is active [2]. You can check available workers at workers.iii.dev [2]. - Inspect Error Payloads: If you are using the Rust SDK, the IIIError enum contains a Remote variant that carries an ErrorBody [3]. You can match on IIIError::Remote(body) to extract the code (e.g., NOT_FOUND) and the accompanying message, which often provides specific details about the missing registration [3]. 3. Common Misconfiguration: A common source of this error is attempting to use a trigger or configuration ID that has not been defined in the engine's current state [1]. Always ensure that configuration IDs are registered before use [1] and that trigger types are installed if they are not built-in [2].
Citations:
- 1: https://workers.iii.dev/workers/configuration
- 2: https://iii.dev/docs/using-iii/triggers
- 3: https://motiadev.mintlify.app/sdk-reference/rust-sdk
🌐 Web query:
iii-sdk Rust iii_sdk::errors::ErrorDisplay formatremote error ( NOT_FOUND code message
💡 Result:
In the iii-sdk Rust crate, iii_sdk::errors::IIIError is the primary error enum [1]. When a remote error occurs, it is represented by the IIIError::Remote(ErrorBody) variant [1]. The ErrorBody struct contains a code (String) and a message (String) [2][3]. While the specific Display implementation for IIIError may vary by version, the Remote variant typically formats as a string that includes the engine's error metadata [1]. When you encounter a NOT_FOUND error, it is passed within the ErrorBody payload [1]. You can access this programmatically by matching on the error variant: match err { IIIError::Remote(body) => { if body.code == "NOT_FOUND" { // Handle the specific NOT_FOUND error println!("Remote error ({}): {}", body.code, body.message); } } _ => { /* Handle other errors */ } } The ErrorBody structure is defined as: pub struct ErrorBody { pub code: String, pub message: String, pub stacktrace: Option, } [2][3] You can branch on body.code.as_str() to handle specific engine error codes such as invocation_failed, function_not_found, or NOT_FOUND [1].
Citations:
- 1: https://motiadev.mintlify.app/sdk-reference/rust-sdk
- 2: https://docs.rs/iii-sdk/latest/src/iii_sdk/protocol.rs.html
- 3: https://docs.rs/iii-sdk/latest/x86_64-pc-windows-msvc/iii_sdk/protocol/struct.ErrorBody.html
Use the SDK error code instead of substring matching. trigger_with_retry reduces the error to text, so contains("NOT_FOUND") can swallow unrelated failures that happen to mention that substring. Branch on the structured NOT_FOUND code from the SDK error body, or keep the typed error through the retry helper.
🤖 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/instructions.rs` around lines 118 - 133, The `get_entry_value`
helper is using substring matching on the error text from `trigger_with_retry`,
which can misclassify unrelated failures as missing entries. Update
`trigger_with_retry` and/or `get_entry_value` to preserve and inspect the
structured SDK error code, then branch on the actual `NOT_FOUND` code instead of
`contains("NOT_FOUND")`; keep `should_seed` unchanged except for using the
corrected missing-entry behavior.
…tions Replace the moved-verbatim coder paragraph with workflow doctrine the function contracts cannot convey: orient with tree/search (context lines), stat-probe + windowed/numbered reads before editing, anchored regex edits guarded by expect_matches, verify from the returned echoes instead of re-reading, create-file only for new files, move for renames, and coder::info + shell::fs::* for paths outside the jail.
1 participant
Fixes MOT-3900
What
Workers now contribute their own sections to the agent system prompt, and operators can add persistent custom instructions — globally, per worker, or per session — with a defined precedence.
metadata.agent_instructionson the configuration entry it already registers. The harness injects it under a# Notes from the \` workerheading while the worker is live (some<entry_id>::function registered), and drops it when the worker stops. Installing the shell worker adds itscoder::*` guidance; stopping it removes it.instructionsconfiguration entry —global(appended to every turn) plusworkers.<id>(appended inside that worker's section). Renders automatically in the console configuration page and hot-reloads via an unfilteredconfigurationtrigger; no console changes needed.options.system_promptwithenrich.Composition order (later refines earlier): mode paragraph → identity prompt → worker sections (worker text, then operator per-worker text) → global operator instructions → per-send enrich.
overrideremains absolute: caller prompt verbatim, nothing else.Deduplication
The coder/code-editing guidance moves out of
harness/prompts/default.txt,harness/prompts/cli.txt, and all four provider identity prompts intoshell/prompts/agent-instructions.txt, declared by the shell worker's config registration. It is no longer served when the shell worker isn't running, and can't drift across five prompt copies.The web worker's
web::inject-guidancepre-generate hook is the older per-step variant of this; migrating it toagent_instructionsis a possible follow-up.Implementation notes
harness/src/instructions.rs: hot-swappable snapshot (mirrors thediscovery/configurationcell pattern) — seeded at boot fromconfiguration::listmetadata + theinstructionsentry value, refreshed by aconfigurationtrigger bound without an id filter. Best-effort everywhere: a fetch failure keeps the previous snapshot and never bricks boot or a send.harness::spawnget the same sections.agent_instructionsvalues that are non-string or blank are ignored; sections over 8KB log a warning but are never truncated.Testing
cargo testgreen: harness 185, shell 1294, provider-{anthropic,openai,openai-codex,xai} all pass; clippy clean on harness + shell.shellac::vsshell::false-prefix), metadata parse tolerance, empty-snapshot prompt is byte-identical to before.Live smoke (pending)
coder::textinstructions.globalin the console lands on the next turn without restartSummary by CodeRabbit
New Features
Bug Fixes
Documentation