3 Commits

Author SHA1 Message Date
Ed Zynda e8e99b19a8 refactor: dedupe cross-package logic and remove dead code from audit (#58)
* Remove dead code: 5 unused symbols across internal packages

- internal/models: LoadModelSettingsFromConfig (zero refs)
- internal/prompts: PromptTemplate.ExpandWithArgs (zero refs)
- internal/app: NewMessageStore (tests migrated to NewMessageStoreWithMessages)
- internal/config: HasEnvVars (+ its test)
- internal/core: ContextWithSudoPassword (test migrated to context.WithValue)

* pkg/kit: use TreeManager alias in exported signatures

NewTreeManagerAdapter and InitTreeSession now spell their signatures with
the public kit.TreeManager alias instead of internal/session.TreeManager,
so go doc renders domain types rather than internal paths.

* Consolidate tool-kind classification into internal/extensions

coreToolKinds + toolKindFor were duplicated verbatim in
internal/extensions/wrapper.go and pkg/kit/events.go, risking silent
divergence between extension events and SDK events. Single source of
truth now lives in internal/extensions/toolkinds.go; pkg/kit re-exports
the constants.

* Consolidate Anthropic OAuth detection and usage-tracker refresh

The 'is the active Anthropic credential a stored OAuth token' check was
copy-pasted at 5 sites, all prefix-matching the magic string
'stored OAuth' produced in internal/auth. Now:

- internal/auth: new CredentialSourceOAuth constant + IsAnthropicOAuth()
- internal/ui: new UpdateUsageTrackerForModel(); CreateUsageTracker and
  SetupCLI share lookupTrackableModel (SetupCLI no longer re-inlines the
  tracker construction)
- cmd/root.go + cmd/extension_context.go: verbatim-duplicated tracker
  refresh blocks replaced with ui.UpdateUsageTrackerForModel
- pkg/kit isAnthropicOAuth delegates to auth.IsAnthropicOAuth
- internal/models compares source against the constant

* pkg/kit: consolidate model-path helpers and argument tokenizer

- ExtractModelFromPath mis-parsed model IDs containing '/' (e.g.
  'openrouter/meta/llama' -> 'meta'); it now delegates to
  RemoveProviderFromModel and is deprecated alongside
  ExtractProviderFromPath (-> GetCurrentProvider)
- parseFields delegated to prompts.ParseCommandArgs so extension argument
  parsing and builtin prompt-template parsing share one quote/escape
  grammar; ParseCommandArgs now also splits on tabs (superset of both
  previous tokenizers)

* Unify the two {{variable}} template engines

internal/skills and pkg/kit/template_bridge each had their own grammar:
skills rejected '{{ name }}' (whitespace) but allowed digit-first names;
the bridge was the opposite. A template behaved differently depending on
whether it was loaded as a skill prompt or via the extension API.

internal/skills is now the single engine using the superset grammar
(\{\{\s*(\w+)\s*\}\}); pkg/kit ParseTemplate/RenderTemplate are thin
adapters over it. Expand is now regex-based so whitespace placeholders
expand consistently; missing variables are still left as-is.

* internal/ui: extract switchModel helper for model-switch flow

The model-selector handler (ModelSelectedMsg) and /model slash command
duplicated the full switch sequence (thinking-level fallback, setModel,
display-state update, preference persistence, ModelChange emit) and had
already drifted in ordering. Both now call a single switchModel method.
Display state is still updated directly (no prog.Send from Update).

* extbridge: extract shared BaseContext for extension wiring

cmd/extension_context.go and internal/acpserver/session.go each built a
giant extensions.Context literal, duplicating ~15 delegation closures
(GetContextStats, GetMessages, AppendEntry, options, SetModel core,
Complete, SpawnSubagent, ...) that had to be kept in sync by hand. New
data-access fields had to be wired in both places or ACP-mode extensions
silently got nil function fields.

extbridge.BaseContext now provides the headless half; both call sites
overlay only their UI-specific closures. As a side effect ACP mode gains
previously-missing APIs (state, tree navigation, skills, template
parsing, model resolution) that were nil before. The interactive TUI
keeps its exact SetModel/ReloadExtensions ordering via overrides.

* internal/tools: extract withOAuthRetry and marshalToolResult helpers

ExecuteTool repeated the OAuth-error/re-auth/retry stanza verbatim twice
(sync and task-augmented paths) and the marshal-and-wrap stanza four
times. Both are now single helpers with identical error strings, so a
fix to OAuth retry or error categorization applies everywhere at once.

* internal/ui: extract buildShareFile with defer-based cleanup

handleShareCommand repeated the close/remove/print/return cleanup chain
four times across its temp-file write error paths. File assembly now
lives in buildShareFile with a single deferred cleanup on error.

* cmd: extract flag validation, preference restore, and provider-URL routing from runNormalMode

runNormalMode opened with ~150 lines of policy logic (flag-combination
validation, persisted model/thinking-level preference restoration, and
two subtle --provider-url model-rewrite rules). These are now standalone
functions (validateModeFlags, restorePersistedPreferences,
applyProviderURLRouting) so the routing policy is independently readable
and testable. Behaviour unchanged; ordering preserved.

* fix: address review findings on SDK godoc and nil guard

- pkg/kit: remove internal package paths from exported godoc on
  ParseTemplate and the ToolKind* constants (SDK doc surface must not
  reference internal packages)
- internal/tools: guard marshalToolResult against a nil CallToolResult
  (json.Marshal(nil) succeeds as 'null', then result.IsError panics if
  a client returns nil result with nil error)

Skipped the TreeNode Children deep-copy suggestion: the slice already
comes from TreeManager.GetChildren which returns a fresh copy per call
into a throwaway intermediate, so no internal state is exposed.
2026-06-11 16:13:18 +03:00
Egbert Eich ef072f6e59 Make subagent inherit tools from parent (#51)
While the tool list of the main agent could be controlled by several
options, subagent used to be equipped with all available tools (except
for the subagent tool itself).
With this change the list of tools is taken from the parent, the
subagent tool itself is removed and the remaining tool list is added
to the subagent.

Signed-off-by: Egbert Eich <eich@suse.com>
2026-06-09 16:28:01 +03:00
Ed Zynda 45689cb30d extract duplicated subagent + event conversion to internal/extbridge
The same ~40-line block — building a kit.SubagentConfig, wrapping
OnEvent through sdkEventToSubagentEvent, calling kitInstance.Subagent,
and translating the SDK result into extensions.SubagentResult — was
copy-pasted three times:

  * cmd/root.go (interactive TUI Context, line 1148)
  * cmd/root.go (post-SessionStart runtime Context, line 1446)
  * internal/acpserver/session.go (ACP server Context, line 154)

A separate sdkEventToSubagentEvent function was duplicated byte-for-byte
between cmd/root.go and internal/acpserver/session.go.

Both are now consolidated in a new internal/extbridge package which is
the only module-internal home that can legitimately import both
pkg/kit/ (the public SDK) and internal/extensions/. cmd/ and
internal/acpserver/ both import it, so SDK-event-to-extension-event
schema changes only have one site to update.

Also fixes pkg/kit/events.go godoc comment that named the underlying
LLM library, per AGENTS.md 'No Dependency Name Leakage' rule for
exported SDK symbols.

go test -race ./... passes.
2026-05-07 12:23:15 +03:00