Skip to content

fix: preserve CLI model override across config loads#792

Open
shixi-li wants to merge 2 commits into
the-open-engine:devfrom
shixi-li:fix/model-override-provenance
Open

fix: preserve CLI model override across config loads#792
shixi-li wants to merge 2 commits into
the-open-engine:devfrom
shixi-li:fix/model-override-provenance

Conversation

@shixi-li

@shixi-li shixi-li commented Jul 24, 2026

Copy link
Copy Markdown

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 #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 #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

  • Tests pass (npm test)
  • Documentation updated (not needed; behavior-only fix)
  • Follows commit guidelines

@greptile-apps

greptile-apps Bot commented Jul 24, 2026

Copy link
Copy Markdown

Greptile Summary

Centralizes dynamic config resolution behind validated template names while preserving CLI model overrides during operation-chain validation.

  • Safely resolves static and parameterized load_config references through TemplateResolver.
  • Normalizes persisted agent configs before validating proposed add, load, update, and removal operations.
  • Adds unit, integration, security, and detached-daemon coverage for the corrected behavior.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains in the reviewed fix.

Important Files Changed

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
Loading

Reviews (2): Last reviewed commit: "fix: constrain dynamic config template p..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant