fix: preserve CLI model override across config loads - #792
Closed
shixi-li wants to merge 2 commits into
Closed
Conversation
Greptile SummaryCentralizes dynamic config resolution behind validated template names while preserving CLI model overrides during operation-chain validation.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains in the reviewed fix.
|
| Filename | Overview |
|---|---|
| src/orchestrator.js | Normalizes persisted agent configurations, aligns same-ID proposal replacement with runtime behavior, and shares config resolution between validation and execution. |
| src/template-resolver.js | Adds identifier-only template resolution with root-bound path checks for static and parameterized config references. |
| src/template-validation/simulate-random-topology.js | Routes simulated load operations through the same validated config-reference resolver used at runtime. |
| tests/load-config-path-security.test.js | Verifies traversal and absolute-path payloads are rejected consistently during validation and execution. |
| tests/model-override.test.js | Covers model-override preservation and authored-model policy across dynamic operation chains. |
| tests/e2e/cluster-lifecycle.test.js | Exercises model-override preservation through a detached conductor-driven config load. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[load_config operation] --> B[Orchestrator validation]
B --> C[TemplateResolver.resolveConfigReference]
D[Orchestrator execution] --> C
E[Topology simulation] --> C
C --> F{Reference kind}
F -->|Static| G[Validate config name]
F -->|Parameterized| H[Validate base template name]
G --> I[Load cluster template]
H --> J[Resolve base template]
I --> K[Validated agent configuration]
J --> K
Reviews (2): Last reviewed commit: "fix: constrain dynamic config template p..." | Re-trigger Greptile
Collaborator
|
Recreated as #814 on a fresh branch from the new main trunk. The original commits and shixi-li’s authorship are preserved. |
3 tasks
shixi-li
added a commit
to shixi-li/zeroshot
that referenced
this pull request
Jul 29, 2026
…#814) ## Main-trunk migration Replaces the-open-engine#792 after the trunk cutover. Both isolated commits now start from the new `main`; original authorship and the contributor’s implementation are preserved. ## Original PR body ## Summary Preserve a validated CLI model override when a conductor dynamically loads another cluster config. Existing agent configs now drop only their runtime-materialized `model` projection before operation-chain validation, while newly authored raw `model` fields remain subject to the repository's `modelLevel` policy. ## Related Issues Addresses the-open-engine#718 ## Changes Made - Clone and normalize already-persisted agent configs before validating a proposed dynamic config. - Make same-ID proposal replacement match runtime replacement semantics, keeping add/load/update payloads visible to validation. - Reject authored raw model fields even when their value equals the active override. - Cover the conductor `load_config` path in-process and through a real detached CLI daemon. - Route validation, execution, and topology simulation through one config-reference resolver. - Reject path-like static config and base-template names before filesystem access. ## Scope Boundary This PR fixes successful CLI model-override propagation through conductor `load_config`. It does not change the generic `failureInfo` persistence or agent task/PID reconciliation behavior also described in the-open-engine#718; those lifecycle-hardening items remain follow-up work. ## Testing - `node tests/run-tests.js tests/model-override.test.js` (11 passing) - `node tests/run-tests.js tests/e2e/cluster-lifecycle.test.js` (2 passing) - `npm run lint` - `npm run typecheck` - `npm test` (1708 passing, 18 pending) - `npm run test:e2e` (25 passing) - `npm run test:slow` - `npm run dupcheck` - `npm audit --audit-level=moderate --omit=dev` - Targeted Prettier and `git diff --check origin/dev...HEAD` `npm run format:check` still reports the same 15 pre-existing files on unmodified `dev` at `ef5ed29`; none overlaps this change, and all changed files pass targeted Prettier. ## Checklist - [x] Tests pass (`npm test`) - [x] Documentation updated (not needed; behavior-only fix) - [x] Follows commit guidelines --------- Co-authored-by: shixi-li <shixi.li@outlook.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Preserve a validated CLI model override when a conductor dynamically loads another cluster config. Existing agent configs now drop only their runtime-materialized
modelprojection before operation-chain validation, while newly authored rawmodelfields remain subject to the repository'smodelLevelpolicy.Related Issues
Addresses #718
Changes Made
load_configpath in-process and through a real detached CLI daemon.Scope Boundary
This PR fixes successful CLI model-override propagation through conductor
load_config. It does not change the genericfailureInfopersistence or agent task/PID reconciliation behavior also described in #718; those lifecycle-hardening items remain follow-up work.Testing
node tests/run-tests.js tests/model-override.test.js(11 passing)node tests/run-tests.js tests/e2e/cluster-lifecycle.test.js(2 passing)npm run lintnpm run typechecknpm test(1708 passing, 18 pending)npm run test:e2e(25 passing)npm run test:slownpm run dupchecknpm audit --audit-level=moderate --omit=devgit diff --check origin/dev...HEADnpm run format:checkstill reports the same 15 pre-existing files on unmodifieddevatef5ed29; none overlaps this change, and all changed files pass targeted Prettier.Checklist
npm test)