Repository navigation
Replace grid setup's overwrite warning with a plain notice and clearer prompts - #262
Merged
Merged
Conversation
sanieldalib
marked this pull request as ready for review
November 13, 2026 17:18
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are recorded for the harness-configuration messaging update.
No open issues 🎉
What changed in this PR
This PR clarifies Grid’s harness-configuration messaging: setup explains what it changes, while routine sync no longer displays an overwrite warning.
Changes:
- Replace the setup warning with a plain notice and teardown instructions.
- Use distinct setup and sync confirmation prompts, and list replaced settings in dry-run and verbose summaries.
- Update tests to check the notice, prompts, replacement summary, and silent sync behavior.
| File | Description |
|---|---|
| internal/cmd/command.grid.go | Updates setup and sync messaging, confirmation prompts, and replacement summaries. |
| internal/cmd/command.grid_test.go | Checks the revised output and sync behavior after a manual settings edit. |
Review effort: Balanced
cretz
approved these changes
Nov 13, 2026
…r words Setup warned that existing settings would be overwritten whenever a setting it manages changed value, including values an earlier setup wrote. Every sync and rerun warned, and Codex's catalog file, which Grid owns outright, did on almost every route change. Settings each harness's teardown already claims as Baseten's no longer count. The warning becomes a plain notice naming the harnesses, with a hint to preview with --dry-run or remove Grid with teardown, and the dry-run summary lists the settings it replaces. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The notice now describes what setup does instead of warning about the user's settings, so it is accurate on every setup and doesn't need to tell Baseten's own settings from the user's. Sync applies without it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pply these harness changes?" The default output lists no changes, so "these" pointed at nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sanieldalib
force-pushed
the
salib/grid-overwrite-warning
branch
from
November 13, 2026 20:57
a569b4a to
1d016de
Compare
This was referenced Nov 13, 2026
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.
🚀 What
grid setupreplaces the overwrite warning with a plain notice of what it does.grid syncapplies without it.Before (on setup and on every sync that changed a value, including values Grid wrote itself):
After (on setup, whenever it changes settings):
The confirmation asks
Set up Grid?on setup andUpdate harness settings?on sync, instead ofApply these harness changes?. The default output lists no changes, so "these" pointed at nothing.In the
--dry-run/--verbosesummary, the sentenceExisting integration settings will be overwritten; teardown does not restore them.becomes a row listing the settings whose values change, e.g.Replaces model, modelPicker.options.💻 How
The notice shows whenever any plan changes settings, so it no longer depends on telling Baseten's earlier settings from the user's. The JSON
replaced_settingsfield is unchanged.🔬 Testing
Test_Grid_Setup_ReplacesSettingsAndPickerchecks the notice and theSet up Grid?prompt on setup, the dry-run row, and that a sync which rewrites a hand-edited setting asksUpdate harness settings?and prints no notice.go test ./...passes exceptTest_Grid_Setup_RealHarnessandTest_Grid_ClaudeServedModel_RealClaudeCode, which fail the same way onmainon my machine. Not checked by hand in a terminal.🤖 Generated with Claude Code