feat(agent-hooks): improve amp,claude state detection

[janburzinski]

summary

improves reliablity of the working/error state for amp and claude

amp would now have a plugin that tells emdash its state
claude uses hook events

[greptile-apps]

Greptile Summary

This PR improves agent state detection reliability for Amp and Claude by replacing heuristic-based state inference with explicit hook/plugin events. The change is well-structured and adds good test coverage for the new hook configurations.

• Amp receives a new plugin (amp-notifications-plugin.js) that listens to agent.start/agent.end events and POSTs working/stop/error notifications to Emdash; PLUGINS=all is injected at launch and supportsHooks is set, disabling the old regex classifier.
• Claude gains two additional hook entries — UserPromptSubmit (mapped to working) and StopFailure (mapped to error) — completing the state-transition coverage alongside the existing Stop and Notification hooks.
• event-enricher.ts adds a readString helper for safe coercion and falls back to body.error_type when body.message is absent; local-conversation.ts is fixed so runtimeEnv is available before PTY spawn.

Confidence Score: 4/5

Safe to merge with the noted inconsistencies addressed; the core hook wiring is correct and well-tested, with no data-loss or auth boundary concerns.

The agent.end handler in the Amp plugin omits the return {} that agent.start includes, which may matter once the Amp plugin API stabilises. The PLUGINS=all env var is a blanket setting on an experimental API whose scope is not fully documented. The error_type fallback for message could expose technical identifiers as user-facing text. All three are low-risk today but worth resolving before the API matures.

amp-notifications-plugin.js (return value inconsistency) and agent-provider-registry.ts (PLUGINS=all scope) deserve a second look.

Important Files Changed

Filename	Overview
src/main/core/agent-hooks/amp-notifications-plugin.js	New Amp plugin that posts working/stop/error events to the Emdash hook server; the agent.end handler omits the return {} that agent.start returns, which may be required by the Amp plugin API
src/shared/agent-provider-registry.ts	Adds supportsHooks and runtimeEnv: { PLUGINS: 'all' } to the Amp provider; the generic PLUGINS var name and 'all' value may have broader effects than enabling only the Emdash plugin
src/main/core/agent-hooks/hook-config.ts	Adds UserPromptSubmit→working and StopFailure→error Claude hooks; extracts writeStaticHookFile helper and adds writeAmpPlugin(); clean refactor with good coverage in tests
src/main/core/agent-hooks/event-enricher.ts	Adds readString() helper for safe string coercion; message now falls back to error_type field which may surface technical identifiers as user-facing text
src/main/core/conversations/impl/local-conversation.ts	Moves getProvider() call before spawnLocalPty to inject runtimeEnv; safe refactor with no logic change beyond env var injection order fix
src/renderer/features/tasks/conversations/conversation-manager.ts	Adds working event handler that calls setWorking(); correctly placed before other event type checks
src/main/core/agent-hooks/hook-config.test.ts	Comprehensive tests added for Claude hook keys and Amp plugin content; readRequiredFile helper avoids silent misses
src/shared/events/agentEvents.ts	Adds 'working' to AgentEventType union; straightforward type extension
src/main/core/agent-hooks/agent-notify-command.ts	Exports makeAmpPluginContent() and tightens makeClaudeHookCommand parameter type to AgentEventType; straightforward addition

Sequence Diagram

sequenceDiagram
    participant User
    participant Amp/Claude as Amp / Claude CLI
    participant HookServer as Emdash Hook Server
    participant ConvManager as ConversationManagerStore
    participant UI

    Note over Amp/Claude,HookServer: Amp path (plugin)
    User->>Amp/Claude: Submit prompt
    Amp/Claude->>HookServer: POST /hook (event: working) via amp-notifications-plugin
    HookServer->>ConvManager: agentEventChannel (type: working)
    ConvManager->>UI: setWorking()

    Amp/Claude->>HookServer: POST /hook (event: stop or error) via amp-notifications-plugin
    HookServer->>ConvManager: agentEventChannel (type: stop | error)
    ConvManager->>UI: setStatus('completed' | 'error')

    Note over Amp/Claude,HookServer: Claude path (hooks)
    User->>Amp/Claude: Submit prompt
    Amp/Claude->>HookServer: curl POST /hook (UserPromptSubmit to working)
    HookServer->>ConvManager: agentEventChannel (type: working)
    ConvManager->>UI: setWorking()

    Amp/Claude->>HookServer: curl POST /hook (Stop to stop)
    HookServer->>ConvManager: agentEventChannel (type: stop)
    ConvManager->>UI: setStatus('completed')

    Amp/Claude->>HookServer: curl POST /hook (StopFailure to error)
    HookServer->>ConvManager: agentEventChannel (type: error)
    ConvManager->>UI: setStatus('error')

Loading

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
src/main/core/agent-hooks/amp-notifications-plugin.js:34-38
The `agent.end` handler does not return `{}`, unlike `agent.start`. If Amp's plugin API uses the return value for event flow control (as the `agent.start` pattern suggests), omitting it here could cause `agent.end` to behave differently or be ignored in future Amp versions. The test suite only asserts that `return {};` exists somewhere in the plugin string, not that it is present in this specific handler.

```suggestion
  amp.on('agent.end', async (event) => {
    await post(event.status === 'error' ? 'error' : 'stop', {
      message: typeof event.message === 'string' ? event.message : undefined,
    });
    return {};
  });
```

### Issue 2 of 3
src/shared/agent-provider-registry.ts:211-212
**Broad `PLUGINS` env var may load unintended plugins**

`PLUGINS: 'all'` is injected into every Amp session. If Amp's plugin system interprets this as "load all discovered plugins" (beyond just those in `.amp/plugins/`), users who have other Amp plugins installed could see them activated unexpectedly when running Amp through Emdash. Since the plugin API is explicitly marked as WIP and experimental, it's worth confirming that `all` is scoped only to the project-local `.amp/plugins/` directory before shipping.

### Issue 3 of 3
src/main/core/agent-hooks/event-enricher.ts:23
Using `error_type` as a fallback for `message` may surface a raw technical identifier (e.g. `"network_error"`, `"timeout"`) as user-facing text in places that render `payload.message`. If `error_type` is an enum value rather than a human-readable string, consider only falling back to it in a dedicated field (e.g. `errorType`) rather than aliasing it onto `message`.

```suggestion
    message: readString(body.message),
    errorType: readString(body.error_type),
```

_{Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile}

[greptile-apps]

The agent.end handler does not return {}, unlike agent.start. If Amp's plugin API uses the return value for event flow control (as the agent.start pattern suggests), omitting it here could cause agent.end to behave differently or be ignored in future Amp versions. The test suite only asserts that return {}; exists somewhere in the plugin string, not that it is present in this specific handler.

Suggested change

	amp.on('agent.end', async (event) => {
	await post(event.status === 'error' ? 'error' : 'stop', {
	message: typeof event.message === 'string' ? event.message : undefined,
	});
	});
	amp.on('agent.end', async (event) => {
	await post(event.status === 'error' ? 'error' : 'stop', {
	message: typeof event.message === 'string' ? event.message : undefined,
	});
	return {};
	});

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/main/core/agent-hooks/amp-notifications-plugin.js
Line: 34-38

Comment:
The `agent.end` handler does not return `{}`, unlike `agent.start`. If Amp's plugin API uses the return value for event flow control (as the `agent.start` pattern suggests), omitting it here could cause `agent.end` to behave differently or be ignored in future Amp versions. The test suite only asserts that `return {};` exists somewhere in the plugin string, not that it is present in this specific handler.

```suggestion
  amp.on('agent.end', async (event) => {
    await post(event.status === 'error' ? 'error' : 'stop', {
      message: typeof event.message === 'string' ? event.message : undefined,
    });
    return {};
  });
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Broad PLUGINS env var may load unintended plugins

PLUGINS: 'all' is injected into every Amp session. If Amp's plugin system interprets this as "load all discovered plugins" (beyond just those in .amp/plugins/), users who have other Amp plugins installed could see them activated unexpectedly when running Amp through Emdash. Since the plugin API is explicitly marked as WIP and experimental, it's worth confirming that all is scoped only to the project-local .amp/plugins/ directory before shipping.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/shared/agent-provider-registry.ts
Line: 211-212

Comment:
**Broad `PLUGINS` env var may load unintended plugins**

`PLUGINS: 'all'` is injected into every Amp session. If Amp's plugin system interprets this as "load all discovered plugins" (beyond just those in `.amp/plugins/`), users who have other Amp plugins installed could see them activated unexpectedly when running Amp through Emdash. Since the plugin API is explicitly marked as WIP and experimental, it's worth confirming that `all` is scoped only to the project-local `.amp/plugins/` directory before shipping.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Using error_type as a fallback for message may surface a raw technical identifier (e.g. "network_error", "timeout") as user-facing text in places that render payload.message. If error_type is an enum value rather than a human-readable string, consider only falling back to it in a dedicated field (e.g. errorType) rather than aliasing it onto message.

Suggested change

	message: readString(body.message) ?? readString(body.error_type),
	message: readString(body.message),
	errorType: readString(body.error_type),

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/main/core/agent-hooks/event-enricher.ts
Line: 23

Comment:
Using `error_type` as a fallback for `message` may surface a raw technical identifier (e.g. `"network_error"`, `"timeout"`) as user-facing text in places that render `payload.message`. If `error_type` is an enum value rather than a human-readable string, consider only falling back to it in a dedicated field (e.g. `errorType`) rather than aliasing it onto `message`.

```suggestion
    message: readString(body.message),
    errorType: readString(body.error_type),
```

How can I resolve this? If you propose a fix, please make it concise.