feat: add dry-run fallback for unauthenticated model resolution#139
Merged
Conversation
**Added:** - `TestResolveModelPrecedenceDryRunWithoutCredentials` test that pins rule-4a-dry behavior: dry-run with no credentials proceeds on the manifest's top model with a warning, while a real run still aborts with env-var guidance - `runner/dryrun_credentials_test.go` - `applyManifestTopUnauthenticated` helper that applies rule 4a-dry by selecting the manifest's top model preference and emitting a warning instead of failing when no credentials exist during a dry-run - `runner/run.go` **Changed:** - `ResolveModelPrecedence` now short-circuits to `applyManifestTopUnauthenticated` when `DryRun` is true and all providers were skipped due to missing credentials, allowing dry-runs to proceed without aborting - `runner/run.go`
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
**Added:** - `TestResolveModelPrecedenceDryRunBundleBaseURL` test verifying that when neither `RunOptions` nor the selected manifest entry specify a base URL, the dry-run path correctly inherits `BaseURL` from the parent `Bundle`
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
1 participant
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.
Key Changes:
--dry-runexecutions to proceed without credentials by falling back to the manifest's top model with a warning, rather than abortingapplyManifestTopUnauthenticatedto handle BaseURL inheritance from both the manifest entry and bundle-level fallbackAdded:
applyManifestTopUnauthenticatedfunction inrunner/run.gopicks the top manifest model, inheritsBaseURLfrom the manifest entry or bundle, logs a warning, and returns a human-readable warning string instead of erroringrunner/dryrun_credentials_test.goadds two tests:TestResolveModelPrecedenceDryRunWithoutCredentialspins that a dry-run proceeds on the manifest's top model while a real run still aborts with env-var guidance, andTestResolveModelPrecedenceDryRunBundleBaseURLverifies that the bundle-levelBaseURLis correctly inherited when neither options nor the manifest entry provide oneChanged:
ResolveModelPrecedence- Added an early-exit branch inrunner/run.gothat checksopts.DryRun && len(skipped) > 0before reachingnoCredentialsError, routing unauthenticated dry-runs toapplyManifestTopUnauthenticatedwhile leaving real-run failure behavior unchanged