Skip to content

CLI duplication & boundary cleanup batch #235

Description

@spxrogers

Grab-bag of smaller, mostly-mechanical consolidations from the review. Each is low-risk on its own; batching them here so they don't get lost.

Duplication:

  • Enabled-agent extraction (for name, ag := range c.Config.Agents { if ag.Enabled … } + the enabled map) is written out 7–9× (apply.go:101, status.go:153, diff.go:77, reconcile.go:140, explain.go:223, plugin.go:451, plugin_explain.go:146, plugin_poll.go ×2). One enabledAgentNames(c.Config) ([]string, map[string]bool) — the exact pair selectAgents already takes.
  • Four hand-rolled RFC 6901 escape/unescape implementations: getJSONPointer (import.go:117-141), getPointerValue (status.go:1046-1066), unescapeJSONPointer (reconcile.go:602), plus internal/jsonkeys. One home (jsonkeys), one order-of-operations bug surface.
  • Three MCP pointer-root allowlists in reconcile.go (writeBackKeyItem:1074, pointerSourceFile:971) while pluginOwnerForKeyItem:582 explains at length why hand-maintained allowlists silently break as agents are added and derives the kind from SourceID instead. Converge on the derived approach; today generic-tier agents' key items fall through the hard-coded lists.
  • importIO.note and importIO.notef are identical (import.go:926,947); the five per-component importer stanzas in import.go share a 40-line skeleton worth a table-driven pass (the component_list.go pattern).
  • The ProjectedCanonical six-field copy exists 3× (explain_model.go:419 pluginOrigins, plugin_explain.go:225, plugin_poll.go:651) — expose it from internal/marketplace.
  • Two line-based TOML table splicers with two fail-closed backstops: writeAgents (agent.go:252-336) and spliceTOMLTable (gitbackup_config.go). One comment-preserving table rewriter in internal/source.

Boundaries / structure:

  • Named-run extraction for the inline RunE monoliths: diff (~200-line closure), check (101), doctor (92), revert (flag triangulation mutating all/args in place) — matching the mcp.go/agent.go pattern.
  • Relocate shared helpers out of misleading files: dest decoding (readDestFile/decodeDestBytes/jsonUnmarshalLoose) out of import.go; stateFileKey/hashFile/hashContent out of status.go; scope resolution + project discovery (resolveScopeFlags, discoverProjectTree, loadProjectedForScope) out of apply.go into e.g. scope.go/load.go.
  • Move cobra-free domain logic to domain packages: vault read/modify/write semantics out of secrets.go into internal/secrets (incl. replacing the hardcoded os.Exit(130) signal goroutine at secrets.go:314-321 with an injectable cleanup hook, and retiring loadSecretsConfig's second config-load path); the subagent migration out of migrate.go; plugin_drift.go is already move-ready.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions