deps: bump github.com/charmbracelet/bubbles from 0.18.0 to 0.21.0 - #4
Conversation
LabelsThe following labels could not be found: Please fix the above issues or remove invalid values from |
57b24de to
4c323c3
Compare
74c9922 to
d3225e3
Compare
2490528 to
a0c9543
Compare
Bumps [github.com/charmbracelet/bubbles](https://github.com/charmbracelet/bubbles) from 0.18.0 to 0.21.0. - [Release notes](https://github.com/charmbracelet/bubbles/releases) - [Changelog](https://github.com/charmbracelet/bubbles/blob/master/.goreleaser.yml) - [Commits](charmbracelet/bubbles@v0.18.0...v0.21.0) --- updated-dependencies: - dependency-name: github.com/charmbracelet/bubbles dependency-version: 0.21.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
d3225e3 to
5813416
Compare
|
Superseded by #17 which updates all dependencies at once. |
|
OK, I won't notify you again about this release, but will get in touch when a new version is available. If you'd rather skip all updates until the next major or minor version, let me know by commenting If you change your mind, just re-open this PR and I'll resolve any conflicts on it. |
#1 (cli/get): runGet was hard-coding "console" / "" log options, ignoring log_format/log_level from config and flags. Call the same logOptionsFromViper() the TUI path uses so `log_format: json` actually produces JSON logs in get. #3 (terraform/getVariableDefaults): silently dropped any variable whose default expression can't evaluate with a nil context (functions, var/local/data refs). Add a Debugw entry so users tracing unresolved refs can see the default existed but couldn't be reduced. #4 (cli/validateGetConfig): the --kubeconfig empty-string check was effectively dead because the persistent flag default populates it from ~/.kube/config. Stat the file when the category requires it (BaseModel/GpuNode/DAC) so we fail fast with a clear message instead of a deep client-go error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#1 (go.mod/go.sum): `go mod tidy` moved github.com/modelcontextprotocol/go-sdk out of the //indirect block. It's a direct import from internal/mcp/server.go. #2 (internal/mcp/server.go:Run): document blocking semantics, EOF / ctx-cancel exit conditions, and the single-shot constraint (a second Run on the same Server reuses the SDK's session list). #3 (internal/cli): extract validateLoaderConfig out of validateGetConfig so runMCP shares the four-field env check. Both surfaces now report "missing required setting(s): ..." with consistent wording. Kept the kubeconfig stat in validateGetConfig — only get needs it. #4 (internal/cli/output/output.go): document FlattenWithKey's collision rule. If T's JSON encoding contains a field named groupField, the map key wins and the original value is silently overwritten. Currently safe (pool/tenant/model don't collide with any pkg/models tag), but locked in writing for future-proofing. #5 (internal/cli/output/output_test.go): add direct tests for FlattenWithKey covering nil/empty maps, single entry, sort stability across multiple groups, omitempty respected, and the collision-overwrite contract from #4. Outstanding from the latest review: an in-memory MCP integration test for the JSON-RPC framing path (#6). Coming next in a separate commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CLI and MCP surfaces had 80%-identical resolver code:
cli/mutate.go: realResolveGpuNode + realResolveGpuPool +
resolveCompartmentID
mcp/mutations.go: resolveNodeForOCIAction +
resolveGpuPoolForOCIAction +
mcpResolveCompartmentID
Same load → find-by-name → enrich-via-OCI chain, same partial-load
tolerance, same "no OCID after lookup" guard. The reviewer flagged
this as drift-prone: a fix to one will be forgotten in the other.
Hoist the chain into internal/resolve so both surfaces call the same
implementation. CLI and MCP keep their thin seam wrappers
(gpuNodeResolverFn, gpuPoolResolverFn, server method delegates) so
package-level test mocking still works as before.
Tests pass with `go test -race -shuffle=on -count=1 ./...`.
internal/resolve currently has no tests of its own; coverage for the
real resolvers comes next (review item #4).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The reviewer's #4 item: realResolveGpuNode/GpuPool and the compartment fallback were 0% covered (hidden by CLI/MCP seam-var wrappers that tests swapped out wholesale). After the previous commit's extraction, the logic lives in one place — cover it once and both surfaces inherit the verification. Eleven cases: GpuNode - ocid bypass (loader must not be called) - found by name in any pool - not found - loader error GpuPool - happy path: load + find + compartment + populate - not found in repo - partial-load tolerated when named pool is present - hard loader error not tolerated - populate fails - no OCID after populate (guard against handing UpdateInstancePool an empty ID for not-yet-applied pools) CompartmentID - happy path - empty cluster (no nodes to read CompartmentID from) - kube unreachable Package coverage: 95.0% of statements. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses the second round of review feedback. Important #1 — env_override is now an explicit opt-in: - New config field MutationEnvOverrideAllowed (default false) with matching --mutation_env_override_allowed persistent flag. - effectiveMutationEnv applies the agent's envOverride only when the operator opted in; otherwise the override is silently dropped AND audit-logged at info, preserving "operator's startup-env choice caps blast radius" as the safety story. - When the override IS applied AND deviates from startup, an info-level "level=warn" audit line records the deviation for SIEM visibility. - Tool descriptions now honestly describe the trade-off via a central mutationToolFooter constant. The previous "parity with list_*" framing understated the security cost; the new shape keeps the parity benefit (multi-realm operators can opt in) without the silent capability expansion. Important #2 — failed-phase event name unified: - Both CLI mutate.go and MCP mutations.go now emit Errorw with event="mutation", phase="failed". Aggregators filtering on a single `phase` discriminator now capture all four states (begin / done / failed / refused). Important #3 — env_override propagation coverage: - TestIntegration_MutationTool_IgnoresEnvOverride_WhenDisallowed: default config drops the agent's env_realm/env_region. - TestIntegration_MutationTool_HonorsEnvOverride_WhenAllowed: flag-set config honors override (renamed from prior test). - TestIntegration_MutationTool_PropagatesEnvOverride: reboot/terminate/scale all assert env reaches BOTH the resolver seam and the action seam — the hops where a future refactor could accidentally drop the override. Minors: - #4: gofmt double blank line in mutate.go (mutations_test.go and tools.go also picked up trailing-whitespace fixes from the same pass). - #5: mutationToolFooter centralizes the env-override clause so drift across the seven descriptions is not possible. - #7: drop the resolveNodeForOCIAction / resolveGpuPoolForOCIAction methods — they were pure passthroughs after the prior commit. The handlers now call mcpResolveGpuNodeFn / mcpResolveGpuPoolFn directly. newTestPair now accepts a variadic config-mutator option so tests that need to flip the flag don't have to fork the helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three Important + three Minor follow-ups from the post-commit review: Important #1 — MCP integration test for enrichment failure: - New TestIntegration_NotifiesOnGpuPoolEnrichmentFailure (and a tiny fixedGpuPoolsLoader helper) drives list_gpu_pools against a non-empty stub loader with KubeConfig pointed at a missing file. Asserts: tool call succeeds (IsError=false), Terraform-derived pool still rendered with placeholder status, enrichment warning appears in BOTH the StructuredContent.warnings envelope AND as a notifications/message frame. Was a real coverage gap — previous partial-load integration test returned nil pools, taking EnrichGpuPools's empty-slice fast-path. Important #2 — GpuPool JSON shape pin-test: - New TestWriteSlice_GpuPool_JSONShape asserts every key on the rendered object uses the lowercase JSON tags introduced in v0.3.0 (name / shape / actualSize / status / capacityType / …) and that no capitalized struct-field name leaks. The CHANGELOG claimed shape-unchanged; this test backs that claim. Important #3 — Structured log for enrichment failures: - EnrichGpuPools now emits a logger.Infow line at each failure point with `step=compartment_id|populate` plus the error, matching the Infow pattern resolve.GpuPool already uses for partial-load. Long-running MCP servers degrade visibly in logs instead of only via tool output. Logger interface has no Warnw — using Infow stays consistent with the existing partial-load log shape. Minor #4 — CHANGELOG calls out the NONEXIST status: - A Terraform-defined pool that hasn't been applied yet now shows `status: "NONEXIST"` after enrichment (the literal value PopulateGpuPools writes when OCI returns 200 but excludes the pool from its result). Same as TUI behavior; just wasn't documented for CLI/MCP consumers. Minor #7 — CHANGELOG calls out the offline-experience change: - `toolkit get gpupool` was previously offline-capable; one K8s lookup attempt is now made before any output renders. Added a sentence explaining the new latency on no-auth hosts. Minor #8 — CLI warning prefix symmetry: - Partial-Terraform failures previously printed `warning: <err>`; enrichment failures print `warning: gpu pool enrichment incomplete: <err>`. Both are now prefixed by their step (`warning: load gpu pools: ...` vs `warning: gpu pool enrichment incomplete: ...`) so a reader can tell at a glance which one fired. Skipped from the review's Minor list: - #5 (string→error return type for EnrichGpuPools) — pure style call, current shape reads cleanly at both call sites; defer to a future pass if/when call sites grow. - #6 (categorized warnings field) — would require restructuring the warnings array into typed entries (Terraform vs enrichment). Bigger surface change than this review warrants. Verified: go test ./..., make fmt-check, make goimports-check, golangci-lint run ./... all green. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Code-review follow-up to the four-commit ImportedModel feature (02fdba7 .. 2c1090e). All review feedback addressed: Important #1 — MCP description claimed a `source` field that was dropped in 0d194ae. Replaced with the orthogonality guidance the Go doc and CHANGELOG now use. Agents planning JSON paths against the tool description will no longer be misled. Important #2 — CHANGELOG was missing the silent BaseModel filter change (02fdba7). Added a `### Changed` entry explicit about the count delta — `toolkit get basemodel -o json | jq length` returns fewer items on clusters with tenant-scoped CBMs. Points readers at the new `importedmodel` category to recover the full set. Important #3 — Confirmed via inventory of pkg/models that ImportedModel is the only category with both Namespace and TenantID (DAC, LimitTenancyOverride, ConsolePropertyTenancyOverride each have TenantID alone). Adopting the DAC pattern: TenantID is the OCI tenant identifier (from `tenancy-id` label, populated on any source that carries the label); Namespace is the K8s scope (the authoritative source-kind indicator). They're orthogonal facets, not synonyms. Loader behavior unchanged — only documentation flips, in three places: - pkg/models/imported_model.go: expanded doc-comment with the "orthogonal facets" framing and explicit consumer guidance ("which K8s scope" vs "which OCI tenant"). - internal/mcp/tools.go (list_imported_models description): same clarification, replacing the stale `source` mention. - CHANGELOG: same framing, with the DAC-pattern reference. Important #4 — Added a deliberate comment in LoadImportedModels explaining the all-or-nothing semantics vs. the LoadGpuPools partial-error idiom. The two sources here are conceptually one catalog; a half-loaded result is more confusing than an explicit error, and the cross-GVR RBAC asymmetry (namespaced `basemodels` vs cluster-scoped `clusterbasemodels`) is the realistic failure mode the comment calls out. Minor #5 — TestList_ImportedModels_FlatShape doc-comment said the test asserts `source` is present; corrected to reflect that the test now asserts `source` is absent (the post-0d194ae contract). Skipped from the review's Minor list: - #6 (lint suppression placement) — pre-existing, no change. - #7 (split routeLoadingDataMsg following the routeList* pattern) — could land in a follow-up if the next addition pushes the message count above the cyclop ceiling again; not urgent at 10. - #8 (ResetScopedData consistency) — already correct. - #9 (StorageURI placement on BaseModel vs ImportedModel) — already the right call, no change. - #10 (CLI table readability) — passed review. Verified: go test ./..., make fmt-check, make goimports-check, golangci-lint run ./... all green. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Review follow-ups for 74723fd: Important #1 — Harmonize the orphan-tenant sentinel across DAC and ImportedModel. DAC's V1/V2 extractors previously branched on hasLabels and defaulted to the literal "missing" — different from the "UNKNOWN_TENANCY" sentinel ImportedModel and tenantIDFromLabels already use for the labels-present-but-no-tenancy-id case. tenantIDFromLabels(nil) already returns "UNKNOWN_TENANCY", so calling it unconditionally collapses the two sentinels into one and matches the policy the user picked ("same convention as DAC"). The dead-`hasLabels` variables are tagged with `_ = hasLabels` to keep the surrounding signatures unchanged. As a follow-on, the now-redundant tenantIDFromUnstructured wrapper in imported_model.go is inlined — its only purpose was to bridge the "labels absent" case, which tenantIDFromLabels(labels) handles identically. One less abstraction, one less typo surface. Important #2 — TestSetImportedModelMap added, mirroring TestSetDedicatedAIClusterMap line-for-line. Covers both the matched-by-suffix path (re-keys to Tenant.Name, sets Owner pointer) and the unmatched path (key passes through, Owner stays nil). A small generic `keys[V any]` helper feeds the diagnostic Errorf message. Important #3 — TestResetScopedData now seeds ImportedModelMap and asserts it's nil after Reset. Production code (dataset.go:78) already resets it; this test pins the contract so a future drop of the reset line fails fast. Important #4 — CHANGELOG TUI column claim corrected to the shipped order: Name, Tenant, Namespace, Display Name, Version, DAC Shape, Flags, Status. The previous text described an in-development layout that didn't match the final headers.go definition. Also added a sentence on the CLI/TUI tenant-column asymmetry (CLI shows raw OCID, TUI shows resolved Tenant.Name via SetImportedModelMap) — same as DAC's behavior, but never documented before. Skipped from the review's Minor list (filed for follow-up): - #5 (UNKNOWN_TENANCY as a `const`): three sites now share the same literal via tenantIDFromLabels; extracting a const is a micro-cleanup that's strictly post-release. - #7 (suffix-match bug in SetXxxMap): pre-existing in DAC, not introduced by this work — separate fix. Verified: go test ./..., make fmt-check, make goimports-check, golangci-lint run ./... all green. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Canonical ordering is name-first, key-second (matches TUI). CLI table output reorders for these 4 categories — intentional per spec Decision #4. DAC carries an explicit ordering invariant for export_csv.go's Name/ID substitution. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
internal/cli/get.go now consults internal/columns for all table/csv/tsv output. Per-category *Table functions and helpers deleted (~150 lines): tenantTable, baseModelTable, importedModelTable, gpuPoolTable, gpuNodeTable, dacTable, tenancyOverrideTable, limitDefinitionTable, definitionTable, definitionOverrideTable, environmentTable, serviceTenancyTable, limitRegionalOverrideTable, modelArtifactTable, tableFromSlice, tableFromGrouped, writeGroupedTable, writeMapFlat, writeMapWithKey, sortedKeys, boolStr, aliasItem. writeMap (for items carrying the group key) and writeMapWithGroupKey (for tenancy overrides that inject the tenant field for JSON) replace the old flat/key variants. writeTableLike consolidates the table/csv/tsv dispatch shared by both slice and map paths. writeAliases rewritten to pass []domain.Category through the registry — CLI Alias output changes from 1-row-per-alias to 1-row-per-category (TUI shape, spec Decision #4). selected []string param threaded through emitCategory and all callees but always nil; --columns flag lands in Task 9. tables_test.go rewritten to drive columns.RenderTable directly; get_test.go alias tests updated to match new canonical headers. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Snapshot test for every domain.Category. The 19 .csv files in testdata/snapshots/ are the canonical shape under the new column registry. 12 categories are byte-identical to pre-refactor CLI output; 7 reflect the intentional diffs (3 widened tenancy overrides + 4 grouped reordered to name-first per spec Decision #4 and Decision #9). Run UPDATE_SNAPSHOTS=1 to regenerate when adding columns or making other deliberate canonical changes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…save Resolves seven review findings across the TUI/CLI/config layers; each fix is covered by a test that fails before the change. - DAC delete (dac_actions.go): guard nil OpcRequestId in the delete error path via a new derefOr helper, and reject a nil work-request id in waitForWorkRequest instead of dereferencing/polling it; skip endpoint summaries with a nil DedicatedAiClusterId rather than panicking the whole deletion. [findings #2, #3] - GPU pool enrichment (gpu_node_actions.go): skip instance-pool summaries missing DisplayName/Id/Size (extracted applyInstancePoolSummaries) so a partial OCI response can't crash get/list/scale. [finding #5] - Lazy loading (model.go): add GPUWorkload to lazyLoadedCategories so `toolkit -c gpuworkload` issues the category load on direct startup. A contract test now asserts every kube-backed category is lazy-loaded. [finding #4] - DAC delete timeout (model_state.go, update_list_ops.go): run the multi-minute deletion under a new uncapped longOpCtx instead of the 30s opCtx, so its own internal timeout governs. [finding #6] - Atomic metadata save (metadata_save.go): writeFileAtomic writes a temp file, fsyncs, then renames, so an interrupted write can't corrupt the existing file. [finding #8] - stderr sink (redirect_stderr_unix.go): 0644 -> 0600 to keep captured auth-plugin output and panic stacks private. [finding #7] Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Records the accepted pattern for intentionally-serial tests (global function-pointer seams swapped via swap()+defer, and the Viper/Cobra singleton) and steers new code toward dependency injection so the serial-test set does not grow. Review finding #4 (migrating the existing seams to DI) is consciously deferred as low-ROI relative to its blast radius. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Bumps github.com/charmbracelet/bubbles from 0.18.0 to 0.21.0.
Release notes
Sourced from github.com/charmbracelet/bubbles's releases.
... (truncated)
Commits
8b55efbfix(textarea): placeholder with chinese chars (#767)bd2a5b0fix: golangci-lint 2 fixes (#769)cce8481ci: sync golangci-lint config (#770)ea344abfeat(viewport): horizontal scroll with mouse wheel (#761)39668ecfix(viewport): normalize method names (#763)f2434c3Revert "fix(viewport): normalize method names"c7f889efix(viewport): normalize method names9e5365edocs: add example for ValidateFunc (#705)c814ac7chore(deps): bump github.com/charmbracelet/lipgloss from 1.0.0 to 1.1.0 (#751)3befcccchore(deps): bump github.com/muesli/termenv from 0.15.2 to 0.16.0 (#740)You can trigger a rebase of this PR by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot mergewill merge this PR after your CI passes on it@dependabot squash and mergewill squash and merge this PR after your CI passes on it@dependabot cancel mergewill cancel a previously requested merge and block automerging@dependabot reopenwill reopen this PR if it is closed@dependabot closewill close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)