fix: use persisted session agent for enrichment#1996
Conversation
Greptile SummaryThis PR fixes a systemic bug where enrichment and lifecycle-check paths were resolving agent identity from the current project/default config rather than from the agent that was actually recorded when the session was created. The fix adds a
Confidence Score: 5/5Safe to merge; the backfill invariant is well-tested and the consumer changes are straightforward reads of the newly-guaranteed field. All changed consumers now read a field that the session manager guarantees is populated on every read path. The recovery paths preserve the agent through metadata rewrites. The only notable trade-off is the non-null assertion in send.ts that drops the prior fallback chain, but it is covered by an outer try/catch and the session-manager invariant makes the assertion correct in every reachable production path. packages/cli/src/commands/send.ts — the non-null assertion on metadata["agent"] is load-bearing on the session-manager repair invariant; worth a second look if that path is ever tested with directly-constructed Session objects.
|
| Filename | Overview |
|---|---|
| packages/cli/src/commands/send.ts | Drops the defensive ?? project?.agent ?? config.defaults.agent fallback in favour of a TypeScript non-null assertion; safe given the session-manager repair invariant, but silently breaks if that invariant is ever bypassed. |
| packages/cli/src/commands/status.ts | Correctly refactored to resolve agent per session from session.metadata["agent"] via registry lookup instead of using the current project/default config for all sessions in a project. |
| packages/core/src/session-manager.ts | Adds repairSessionAgentMetadataOnRead that backfills missing legacy agent metadata at both list() and get() read paths; restore path also now writes agent to persisted metadata. |
| packages/core/src/lifecycle-manager.ts | Simplified: reads agent directly from session.metadata["agent"] and guards all probing with a null check, eliminating redundant project-config re-resolution for already-normalised sessions. |
| packages/core/src/recovery/validator.ts | Uses resolveAgentSelectionForSession helper and backfills agent into a normalizedMetadata view that is returned as rawMetadata on the assessment, making it available to recovery actions. |
| packages/core/src/recovery/actions.ts | Adds preserveSessionAgentPatch to carry the resolved agent through all repair/escalation/cleanup metadata writes, preventing it from being dropped when records are rewritten. |
| packages/web/src/lib/serialize.ts | Bug fix: summary enrichment now reads core.metadata["agent"] directly instead of projects[i]?.agent ?? config.defaults.agent, correctly using the session's persisted agent identity. |
| packages/core/src/agent-selection.ts | Adds resolveAgentSelectionForSession wrapper that encapsulates the role-resolution + agent-selection call, reducing boilerplate at legacy raw-metadata boundaries. |
| packages/core/src/index.ts | Exports resolveAgentSelection, resolveAgentSelectionForSession, resolveSessionRole, and their types from agent-selection.ts. |
Sequence Diagram
sequenceDiagram
participant Caller
participant SessionManager
participant Disk
participant Consumer
Caller->>SessionManager: list() / get()
SessionManager->>Disk: read raw metadata
Disk-->>SessionManager: raw record (may lack "agent")
SessionManager->>SessionManager: repairSessionAgentMetadataOnRead()
note over SessionManager: resolveAgentSelectionForSession() backfills "agent" if missing
SessionManager->>Disk: "updateMetadataPreservingMtime({ agent })"
SessionManager-->>Caller: Session with metadata["agent"] guaranteed
Caller->>Consumer: pass Session
Consumer->>Consumer: read session.metadata["agent"]
Consumer->>Consumer: registry.get("agent", agentName)
note over Consumer: serialize.ts / status.ts / send.ts / lifecycle-manager.ts
note over Caller,Consumer: Recovery path
Caller->>SessionManager: validateSession() (raw metadata)
SessionManager->>SessionManager: resolveAgentSelectionForSession()
SessionManager->>SessionManager: "normalizedMetadata = { ...raw, agent }"
SessionManager-->>Caller: "assessment.rawMetadata = normalizedMetadata"
Caller->>Caller: preserveSessionAgentPatch(rawMetadata)
Caller->>Disk: write repaired metadata with agent preserved
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/cli/src/commands/send.ts:35
The non-null assertion replaces the previous defensive fallback chain (`?? project?.agent ?? config.defaults.agent`). At runtime `!` is a no-op, so if `metadata["agent"]` is ever undefined — e.g. in a test that constructs a `Session` directly without going through `sm.get()`, or if a future code path bypasses the repair — `getAgentByNameFromRegistry` receives `undefined` silently. The outer `try/catch` will catch any resulting throw and fall back to `claude-code`, but that silent fallback is harder to diagnose than an explicit guard. Using a defined fallback keeps the behaviour observable.
```suggestion
const agentName = session.metadata["agent"] ?? config.defaults.agent;
```
Reviews (4): Last reviewed commit: "test: include normalized agent in lifecy..." | Re-trigger Greptile
There was a problem hiding this comment.
Pull request overview
This PR fixes mixed-agent session metadata enrichment by resolving the effective agent per session from persisted session metadata (instead of always using the current project/default agent), and centralizes that logic in a shared core helper.
Changes:
- Added
resolveAgentSelectionForSession()in@aoagents/ao-coreto consistently resolve role + agent selection using persisted session metadata. - Updated the web dashboard session serialization/enrichment path to use the per-session resolved agent for summary enrichment.
- Updated core and CLI call sites that perform session-specific agent probing to use the shared helper.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/web/src/lib/serialize.ts | Uses the shared helper to pick the correct per-session agent for summary enrichment. |
| packages/web/src/lib/tests/serialize.test.ts | Adds regression coverage ensuring persisted session agent beats project/default agent during enrichment. |
| packages/core/src/agent-selection.ts | Introduces resolveAgentSelectionForSession() helper wrapping role + agent selection. |
| packages/core/src/session-manager.ts | Uses the shared helper to keep session agent selection logic consistent. |
| packages/core/src/lifecycle-manager.ts | Uses the shared helper when determining agent for session polling/enrichment. |
| packages/core/src/recovery/validator.ts | Uses the shared helper during recovery validation to select the correct agent. |
| packages/core/src/index.ts | Exposes agent-selection helpers/types from the core package entrypoint. |
| packages/cli/src/commands/status.ts | Resolves the agent per session for introspection-based summaries (avoids probing with project/default agent). |
Fixes #1995
Summary
resolveAgentSelectionForSession()helper for legacy raw metadata repair/backfill boundaries.agentmetadata on session-manager reads, attached the resolved agent in recovery validation, and preserved it when recovery actions rewrite metadata.Sessionconsumers (web enrichment, CLI status/send, lifecycle polling) to usesession.metadata["agent"]as the session-agent identity instead of inferring from current project/default config.Audit findings
packages/web/src/lib/serialize.tswas the bug source: it selected the current project/default agent for dashboard summary enrichment. Fixed; web now reads the normalized persisted session agent.packages/cli/src/commands/status.tshad the same root pattern for status-summary introspection. Fixed; it no longer falls back to current project/default config for a loadedSession.packages/cli/src/commands/send.tsstill had a session-specific project/default fallback aftersessionManager.get(). Fixed to rely on normalizedsession.metadata["agent"].packages/core/src/lifecycle-manager.tswas a normalized-Session consumer and no longer re-resolves agent identity from project config.doctorand settings UI are config inspection/editing; plugin config reads are runtime launch config; fallback tmux status has no AO metadata record to resolve from.Tests
pnpm --filter @aoagents/ao-core test -- src/__tests__/session-manager/query.test.ts src/__tests__/session-manager/communication.test.ts src/__tests__/lifecycle-manager.test.ts src/__tests__/recovery-validator.test.ts src/__tests__/recovery-actions.test.tspnpm --filter @aoagents/ao-web test -- src/lib/__tests__/serialize.test.ts src/__tests__/api-routes.test.tspnpm --filter @aoagents/ao-cli test -- __tests__/commands/status.test.ts __tests__/commands/send.test.tspnpm --filter @aoagents/ao-core build && pnpm --filter './packages/plugins/*' buildpnpm --filter @aoagents/ao-core typecheck && pnpm --filter @aoagents/ao-web typecheck && pnpm --filter @aoagents/ao-cli typecheckgit diff --checkNotes