Skip to content

Add Amp agent support and centralize agent registry - #72

Merged
wesm merged 23 commits into
kenn-io:mainfrom
sinnet3000:amp-support
Feb 28, 2026
Merged

Add Amp agent support and centralize agent registry#72
wesm merged 23 commits into
kenn-io:mainfrom
sinnet3000:amp-support

Conversation

@sinnet3000

@sinnet3000 sinnet3000 commented Feb 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Amp agent support:

  • Add Amp as a supported agent type, syncing sessions from ~/.local/share/amp/threads/ with a dedicated parser, file discovery, and sync engine wiring
  • Parse Amp's JSON thread format (T-*.json), extracting user messages, assistant responses, and tool calls (with skill name extraction from Amp's lowercase skill tool)
  • Extend shared tool content extraction to handle Amp's field name variants (path vs file_path, cmd vs command) and add display formatting for Amp-native tools (edit_file, shell_command, look_at, apply_patch, etc.)

Agent registry refactor:

  • Introduce AgentDef struct and parser.Registry as a central source of truth for all agent metadata (type, display name, env var, config key, default dirs, ID prefix, watch subdir, file-based flag, discover/findSource functions)
  • Replace per-agent fields in Config and Engine with AgentDirs map[parser.AgentType][]string driven by the registry
  • Replace NewEngine's 9 positional parameters with EngineConfig struct
  • Move all discovery and findSource functions from sync to parser so they can be referenced directly from AgentDef
  • Registry-driven loops in main.go for missing-dir warnings and file watcher setup
  • Adding a new agent now requires one Registry entry and a parser — no changes to config, engine, or main

Other fixes:

  • Use agentColor() consistently across App.svelte and CommandPalette, fixing wrong colors for Gemini and Cursor sessions
  • Fix incorrect test expectation in setProjectFilter — agent filter is preserved across project switches, not reset
  • Defensive deep-copy of AgentDirs in NewEngine to prevent post-construction mutation
  • Log warning on malformed config file array values instead of silently ignoring

Generated with Claude Code

sinnet3000 and others added 10 commits February 28, 2026 00:13
Replace hardcoded class:agent-* bindings in App.svelte with a single
dynamic style:background={agentColor()} driven by the centralized
agentColor() utility. This eliminates the need to add new CSS classes
per agent and removes dead per-agent CSS rules.

Fix CommandPalette.svelte nested ternary dot-color logic to use
agentColor(), resolving incorrect colors for Gemini and Cursor sessions.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add AgentAmp agent type and ParseAmpSession() for
~/.local/share/amp/threads/T-*.json files.

- Session ID namespaced as "amp:<thread-id>"
- Start time from epoch-ms "created" field via time.UnixMilli
- End time from meta.traces[last].endTime (zero when absent)
- Project from env.initial.trees[0].displayName, fallback "amp"
- FirstMessage prefers title field; falls back to first user message
- Calls ExtractTextContent() for all content blocks (no custom extractor)
- Returns nil for empty threads (non-interactive)
- AmpThreadID() helper for fast ID extraction without full parse

Tests cover: basic thread, tool use + thinking, no env, no title,
no meta.traces, empty thread, first message truncation, error paths.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Config:
- Add AmpDir field (default ~/.local/share/amp/threads, env AMP_DIR)

Discovery:
- Add DiscoverAmpSessions(): matches T-*.json in the threads dir
- Add FindAmpSourceFile() for single-session re-sync lookup

Engine:
- Add ampDir field and parameter to NewEngine()
- Add case parser.AgentAmp to processFile() switch
- Add processAmp() modeled after processGemini()
- Update syncAllLocked() to discover and include Amp files
- Add amp: prefix handling to FindSourceFile() and SyncSingleSession()

Server (main):
- Pass cfg.AmpDir to NewEngine()
- Watch AmpDir for file changes
- Warn on missing AmpDir
- Update help text to include Amp and AMP_DIR env var

Frontend:
- Add --accent-coral (#f34e3f) CSS variable for Amp brand color
- Add { name: "amp", color: "var(--accent-coral)" } to KNOWN_AGENTS
- Update agents.test.ts to cover amp color

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Amp uses different field names than Claude Code for the same logical
tools (cmd vs command for Bash, path vs file_path for Read) and has
agent-specific tools (edit_file, create_file, shell_command, glob,
look_at, apply_patch, undo_edit, finder, read_web_page, skill) that
were falling through to the generic [Tool: name] fallback.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Feb 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (28f545a)

Summary Verdict: The changes successfully introduce "Amp" agent support and UI refactoring, but there is one medium-severity issue regarding session end time calculation.

Medium

End time
can be dropped even when a valid trace end exists

  • File: internal/parser/amp.go:45
  • Description: EndedAt is taken only from meta.traces[last].endTime. If the last trace is missing endTime but earlier traces have one, the session is treated as not ended (zero time). This can skew ordering/analytics for completed sessions.
  • Suggested Fix: Scan traces backward and use the most
    recent non-empty, parseable endTime.

Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

When the last trace lacks an endTime but earlier traces have one,
EndedAt was incorrectly left as zero. Scan backward instead of
only checking the last trace.
@roborev-ci

roborev-ci Bot commented Feb 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (bafd66d)

Summary Verdict: All agents agree the code is clean and no security or behavioral issues were found.

No medium, high, or critical severity issues were identified in this review. The implementation of Amp support
, path validations, and frontend refactoring are secure and robust.


Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

Update all agent lists, supported agents table, env var references,
and parser/project structure descriptions to include Amp.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Feb 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (d70f836)

Summary Verdict: The code is mostly secure and well-structured, but there is one medium-severity issue regarding tool categorization that needs addressing.

Medium

  • Mismatched tool categorization for edit_file
    edit_file is rendered as an edit action but categorized as
    write, which will skew analytics and filtering.
    • References: internal/parser/content.go#L132, internal/
      parser/taxonomy.go#L34
    • Details: formatToolUse now emits "[Edit: ...]" for Amp edit _file, but NormalizeToolCategory still maps edit_file under "Write".
    • Suggested fix: Split the mapping so edit_file normalizes to "Edit" (while keeping create_file/write_file as "Write"), and update the
      tests accordingly.

Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

Render edit_file as [Write: ...] in content.go to match Gemini's
display and taxonomy mapping, keeping behavior consistent across agents.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sinnet3000

sinnet3000 commented Feb 28, 2026

Copy link
Copy Markdown
Contributor Author

roborev: Combined Review (d70f836)

Summary Verdict: The code is mostly secure and well-structured, but there is one medium-severity issue regarding tool categorization that needs addressing.

Medium

* **Mismatched tool categorization for `edit_file`**
  `edit_file` is rendered as an edit action but categorized as
  write, which will skew analytics and filtering.
  
  * **References:** [internal/parser/content.go#L132](/home/roborev/.roborev/clones/wesm/agentsview/internal/parser/content.go#L132), [internal/
    parser/taxonomy.go#L34](/home/roborev/.roborev/clones/wesm/agentsview/internal/parser/taxonomy.go#L34)
  * **Details:** `formatToolUse` now emits `"[Edit: ...]"` for Amp `edit _file`, but `NormalizeToolCategory` still maps `edit_file` under `"Write"`.
  * **Suggested fix:** Split the mapping so `edit_file` normalizes to `"Edit"` (while keeping `create_file`/`write_file` as `"Write"`), and update the
    tests accordingly.

Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

Keeping edit_file consistent with Gemini — both display and taxonomy map to "Write".

@roborev-ci

roborev-ci Bot commented Feb 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (29659a2)

Summary Verdict: All reviewers agree the
code changes are clean with no medium, high, or critical issues identified.

No issues found.


Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

@wesm

wesm commented Feb 28, 2026

Copy link
Copy Markdown
Member

Thanks, I'm going to work on some refactoring to make it easier to add new agents

wesm and others added 4 commits February 28, 2026 07:20
Replace per-agent fields across Config, Engine, and main.go with a
central AgentDef registry that drives configuration, discovery, sync,
and file watching. Move DiscoveredFile and all discover/findSource
functions from sync to parser, adding them as function fields on
AgentDef to eliminate separate dispatch tables.

Adding a new agent now requires only a Registry entry in
parser/types.go instead of modifying 13+ locations across 6+ files.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use AgentByType(AgentClaude) instead of fragile Registry[0]
  positional fallback in AgentByPrefix
- Add findContainingDir helper so processCursor validates
  containment against the correct root when multiple Cursor
  dirs are configured (was hardcoded to cursorDirs[0])
- Add integration test for multi-dir Cursor covering SyncAll,
  SyncPaths, and FindSourceFile across non-first directories

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Deep-copy cfg.AgentDirs in NewEngine to prevent callers from
  mutating the engine's state after construction
- Log a warning when a config file key can't be parsed as a
  string array instead of silently ignoring it

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add TestNewEngineDefensiveCopy: mutates the original AgentDirs
  map after NewEngine and verifies the engine still syncs correctly
- Add TestLoadFile_MalformedDirValueLogsWarning: writes a string
  instead of []string for claude_project_dirs and asserts the
  warning is logged and defaults are preserved

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@wesm wesm changed the title Add Amp agent support Add Amp agent support and centralize agent registry Feb 28, 2026
- Restore previous log.Writer() instead of hardcoding os.Stderr,
  preventing test-order-dependent logger state
- Add slice-element mutation path to TestNewEngineDefensiveCopy
  to catch regressions where the map is copied but slices are shared

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Feb 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (e23450f)

Summary: The changes safely introduce end-to-end support for Amp sessions and refactor agent handling into a centralized registry, with one medium severity issue identified regarding path classification.

Medium

  • Early return in Cursor path classification can hide valid files when roots overlap
    • File: internal/sync/engine.go (classifyOnePath, Cursor block)
    • Problem: In the Cursor branch, when a path is under a configured Cursor root but does not match the
      strict 3-part transcript shape, the code returns false immediately. With overlapping roots (or cross-agent nested roots), this can prevent checking later roots/agents and drop valid files.
    • Suggested fix: Replace those return ... false branches with continue in the Cursor loop,
      and only return false after all agent/root checks are exhausted. Also prefer the most-specific matching root when multiple roots contain the path.

Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

wesm and others added 2 commits February 28, 2026 08:26
classifyOnePath's Cursor and Claude-subagent blocks returned false
immediately when a path was under their root but didn't match the
expected structure. This prevented later agent blocks from checking
the path, silently dropping valid files when roots overlap.

Change both to continue so classification falls through to
subsequent agents. Add TestSyncPathsClassifyFallsThrough covering
an Amp file under a nested Cursor root.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cover the Claude-subagent continue path (non-agent- filename in
a subagents dir) with an overlapping-roots test that verifies
classification falls through to Amp.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Feb 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (569811f)

Summary Verdict: One medium severity issue was found regarding Amp ID validation; otherwise, the code is clean and secure.

Medium

Problem: ParseAmpSession accepts any non-empty JSON id, but source lookup/resync path for Amp requires strict T-*.json ID validation in FindAmpSourceFile. A malformed or mismatched in-file id can be ingested successfully, then later fail FindSourceFile/SyncSingleSession.

Files:

  • internal/parser/amp.go:31
  • internal/parser/discovery.go:770
  • internal/sync/engine.go:1366

Suggested fix: Validate Amp id during parse with the same rule
used by discovery/source lookup (or fall back to filename-derived ID when invalid/mismatched), and add a regression test for mismatched filename vs JSON id.


Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

ParseAmpSession accepted any non-empty JSON id, but
FindAmpSourceFile and SyncSingleSession require the T-<valid>
pattern. A malformed in-file id would be ingested successfully
but later fail source lookup and resync.

Validate the JSON id with isValidAmpThreadID at parse time and
fall back to the filename-derived ID when invalid or missing.
Add regression tests for mismatched and missing id scenarios.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Feb 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (4737a42)

Summary Verdict: One
Medium severity issue found regarding Amp session ID and filename consistency; no security vulnerabilities identified.

Medium

Amp session ID/source-file mapping inconsistency

  • Reference: [internal/parser/amp.go](/home/roborev/.roborev/clones/wesm/agents
    view/internal/parser/amp.go) (thread ID selection logic), internal/parser/discovery.go (FindAmpSourceFile)
  • ParseAmp Session only falls back to the filename when the JSON id is missing or invalid. If the JSON id is valid but differs from the T-*.json filename, the stored session ID becomes amp:<json-id>, but FindAmpSourceFile later looks for `<json-id

.json. This breaks FindSourceFile/SyncSingleSession` resolution for that session.

  • Suggested fix: If the filename has a valid Amp ID and it differs from the JSON id, prefer the filename (or reject/log and skip) so parse-time ID and source-file lookup
    stay consistent.

Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

wesm and others added 2 commits February 28, 2026 08:52
When both the filename and JSON id are valid but differ,
FindAmpSourceFile constructs the path from the session ID,
so using the JSON id would fail source lookup. Always prefer
the filename-derived ID; fall back to JSON id only when the
filename doesn't yield a valid T-<id> pattern.

Add test for valid-but-mismatched JSON id vs filename case.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add subtest for valid JSON id with non-matching filename
(bad-name.json + id:"T-from-json") to lock in the fallback
behavior where the JSON id is used when the filename doesn't
yield a valid Amp thread ID.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@roborev-ci

roborev-ci Bot commented Feb 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (e7f9d9c)

Summary Verdict: All agents reviewed the changes and agree the code is clean.

No medium, high, or critical severity issues
were found.


Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

@wesm
wesm merged commit d0f3abb into kenn-io:main Feb 28, 2026
6 checks passed
@wesm

wesm commented Feb 28, 2026

Copy link
Copy Markdown
Member

thanks!

@sinnet3000
sinnet3000 deleted the amp-support branch February 28, 2026 15:56
@carze carze mentioned this pull request Mar 1, 2026
cursor Bot pushed a commit to diazMelgarejo/periscope that referenced this pull request Jun 1, 2026
## Summary

**Amp agent support:**
- Add Amp as a supported agent type, syncing sessions from
`~/.local/share/amp/threads/` with a dedicated parser, file discovery,
and sync engine wiring
- Parse Amp's JSON thread format (`T-*.json`), extracting user messages,
assistant responses, and tool calls (with skill name extraction from
Amp's lowercase `skill` tool)
- Extend shared tool content extraction to handle Amp's field name
variants (`path` vs `file_path`, `cmd` vs `command`) and add display
formatting for Amp-native tools (`edit_file`, `shell_command`,
`look_at`, `apply_patch`, etc.)

**Agent registry refactor:**
- Introduce `AgentDef` struct and `parser.Registry` as a central source
of truth for all agent metadata (type, display name, env var, config
key, default dirs, ID prefix, watch subdir, file-based flag,
discover/findSource functions)
- Replace per-agent fields in `Config` and `Engine` with `AgentDirs
map[parser.AgentType][]string` driven by the registry
- Replace `NewEngine`'s 9 positional parameters with `EngineConfig`
struct
- Move all discovery and findSource functions from `sync` to `parser` so
they can be referenced directly from `AgentDef`
- Registry-driven loops in `main.go` for missing-dir warnings and file
watcher setup
- Adding a new agent now requires one `Registry` entry and a parser — no
changes to config, engine, or main

**Other fixes:**
- Use `agentColor()` consistently across App.svelte and CommandPalette,
fixing wrong colors for Gemini and Cursor sessions
- Fix incorrect test expectation in `setProjectFilter` — agent filter is
preserved across project switches, not reset
- Defensive deep-copy of `AgentDirs` in `NewEngine` to prevent
post-construction mutation
- Log warning on malformed config file array values instead of silently
ignoring

Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Wes McKinney <wesmckinn+git@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants