fix: preserve CLI model override across config loads - #814
Merged
Conversation
3 tasks
Greptile SummaryPreserves CLI model overrides while validating dynamically loaded cluster configurations.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/orchestrator.js | Normalizes persisted agent models for validation, aligns same-ID proposal replacement, and shares config resolution between validation and execution. |
| src/template-resolver.js | Adds centralized named-config resolution with identifier validation and canonical containment checks. |
| src/template-validation/simulate-random-topology.js | Routes simulated load_config operations through the same resolver used at runtime. |
| tests/model-override.test.js | Covers override propagation and enforcement of authored raw-model restrictions. |
| tests/load-config-path-security.test.js | Exercises traversal and path-like input rejection across validation and execution. |
Sequence Diagram
sequenceDiagram
participant CLI
participant Orchestrator
participant Validator
participant Resolver as TemplateResolver
participant Runtime
CLI->>Orchestrator: Start cluster with modelOverride
Orchestrator->>Orchestrator: Persist override and materialize agent models
Runtime->>Orchestrator: load_config operation
Orchestrator->>Orchestrator: Clone existing configs and remove runtime model projections
Orchestrator->>Resolver: resolveConfigReference(config)
Resolver-->>Orchestrator: Loaded agent configuration
Orchestrator->>Validator: Validate proposed operation-chain state
Validator-->>Orchestrator: Valid
Orchestrator->>Runtime: Execute load_config
Runtime->>Runtime: Apply persisted modelOverride to loaded agents
Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile
|
🎉 This PR is included in version 6.8.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
Main-trunk migration
Replaces #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
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)