chore: internal cleanups — hook event modeling, store column list, OpenCode dual-file read, dead code - #140
Merged
Merged
Conversation
EventMapping modeled agent-specific events (no cross-agent equivalent) with the placeholder convention canonical == agent, relying on the implicit invariant that no other agent table uses the same string as a canonical or agent name. The convention has spread to 23 entries across 4 tables (Windsurf 10, Hermes 4, Kiro 5, Grok 4), and Kiro/Grok name their events in the same PascalCase style as canonical names, so the collision surface grows with every new agent. Make canonical an Option: Some(...) for true cross-agent mappings, None for agent-specific events. translate() now short-circuits on None instead of depending on cross-table name uniqueness. Behavior is unchanged — the existing tests (passthrough, no-cross-translate, per-agent tables) lock the equivalence and needed no edits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
get_child_skills carried its own copy of the SELECT column list, missing the trailing scope_json and mcp_transport columns. row_to_extension reads by positional index with .ok() tolerance on those columns, so the gap never errored — every child skill silently came back with its scope reset to Global, mislabeling children of project-scoped CLIs. Extract EXTENSION_COLUMNS and use it in get_extension, list_extensions and get_child_skills so the queries cannot drift again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
OpenCode merge-loads both config files when they coexist (.json first, .jsonc second, last-wins per key), but HK read only the picked file, so MCP entries living solely in opencode.json were invisible whenever an opencode.jsonc existed next to it. Mirror the merge on the read path; the write path keeps targeting the single picked file, matching OpenCode's own write behavior. Known limit, deliberately out of scope: with both files present, an entry that lives only in .json is now listed, but disabling it edits .jsonc — OpenCode may still load the .json original. Deferred until a real-world report, per the PR #41 follow-up plan. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The dedicated pack-toggle API had no callers end to end: the invoke.ts wrapper is referenced nowhere in the frontend, and the CLI's enable/disable --pack filters extensions itself and loops toggle_extension. Remove Manager::toggle_by_pack, the desktop command and its registration, the web handler and route, the frontend wrapper, and Store::find_ids_by_pack (whose only callers were this chain). The chain also silently discarded each toggle's ToggleOutcome warnings, so wiring it up later would have reintroduced a known flaw; the CLI path remains the single implementation of batch toggling by pack. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Merged
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.
Four self-contained internal cleanups, one commit each:
1.
refactor(hooks): use Option<canonical> for agent-specific eventsEventMappingmodeled agent-specific hook events with acanonical == agentplaceholder convention, relying on the implicit invariant that no other agent table reuses the same string. The convention had spread to 23 entries across 4 tables (Windsurf 10, Hermes 4, Kiro 5, Grok 4), and Kiro/Grok name their events in canonical-style PascalCase, so the collision surface grew with every new agent.canonicalis nowOption:None= no cross-agent equivalent. Behavior-equivalent — no test changed; the existing passthrough / no-cross-translate tests lock the equivalence.2.
fix(store): share one column list across row_to_extension queriesget_child_skillscarried a private copy of the SELECT column list missing the trailingscope_json, mcp_transport.row_to_extensionreads positionally with.ok()tolerance on those columns, so this never errored — every CLI child skill silently came back scope=Global, mislabeling children of project-scoped CLIs. ExtractedEXTENSION_COLUMNS, used by all three feeding queries.3.
fix(opencode): merge opencode.json and opencode.jsonc on readOpenCode merge-loads both files when they coexist (
.jsonfirst,.jsoncsecond, last-wins per key); HK read only the picked file, dropping MCP entries that live solely inopencode.json. The read path now mirrors the merge; the write path still targets the single picked file, matching OpenCode's own write behavior. Known deferred limit (per the PR #41 follow-up plan): a.json-only entry is now visible, but disabling it edits.jsonc.4.
chore: remove the orphaned toggle_by_pack chainThe dedicated pack-toggle API had zero callers end to end — the CLI's
--packbatch toggle filters and loopstoggle_extensionitself. RemovedManager::toggle_by_pack, the desktop command + registration, the web handler + route, theinvoke.tswrapper, andStore::find_ids_by_pack(only ever called by this chain). The chain also discardedToggleOutcomewarnings, so wiring it up later would have reintroduced a known flaw.🤖 Generated with Claude Code