Refactor context management and reorganize telegram utilities

[pablof7z]

Summary

This PR refactors the context management system to improve scratchpad handling and simplifies the reminder system, while also reorganizing telegram-related utilities for better code organization.

Key Changes

Context Management Refactoring

• Renamed import: LLMSummarizationStrategy → SummarizationStrategy for consistency
• Scratchpad entries handling: Changed from serializing entries to a notes string to directly managing structured entries
  • Removed serializeScratchpadEntries() and buildStoredScratchpadEntries() functions
  • Updated toRuntimeScratchpadState() to pass entries directly instead of serializing them
  • Simplified fromRuntimeScratchpadState() to merge entries without intermediate serialization
• Scratchpad tool input normalization: Enhanced to support granular entry management
  • Added support for setEntries, replaceEntries, and removeEntryKeys parameters
  • Maintained backward compatibility with legacy notes parameter
• Reminder system simplification:
  • Removed stripScratchpadReminderFromPrompt() and clonePromptForMutation() helper functions
  • Simplified QueuedScratchpadStrategy.apply() to delegate directly without reminder text extraction
  • Added reminderSink to context management runtime for centralized reminder handling
• Prompt handling: Changed event.payloads.promptAfter → event.payloads.prompt in getRuntimeCompletePrompt()
• ToolResultDecayStrategy placeholder: Updated callback signature from positional parameters (toolName, toolCallId) to object parameter (context: { toolName: string; toolCallId: string })

Telegram Utilities Reorganization

• File relocation: Moved telegram-identifiers.ts from src/services/telegram/ to src/lib/
• Import updates: Updated all imports across multiple files to reference the new location:
  • TelegramDeliveryService.ts
  • InboundEnvelopeEventBridge.ts
  • TelegramGatewayCoordinator.ts
  • TelegramGatewayService.ts
  • TelegramInboundAdapter.ts
  • index.ts (re-export)

Type Safety Improvements

• Added explicit type annotations for array parameters in filter/map operations in:
  • NostrInboundAdapter.ts
  • TelegramDeliveryService.ts

Implementation Details

• The scratchpad refactoring maintains backward compatibility through legacy notes parameter handling while enabling more flexible entry management
• The reminder system now uses a dedicated sink interface for better separation of concerns
• Telegram identifiers are now in the lib directory, indicating they are shared utilities rather than service-specific code

https://claude.ai/code/session_01Q7uikvaYe8s5znaKTJGLaG

[claude]

PR Review: Refactor context management and reorganize telegram utilities

Overall this is a solid refactor. The scratchpad simplification and reminderSink decoupling are improvements. A few things worth flagging:

---

Architecture concern: Re-export in services/telegram/index.ts

The new src/services/telegram/index.ts re-exports from @/lib/telegram-identifiers. This is a backwards-compatibility re-export, which CLAUDE.md explicitly prohibits. Any consumers that import identifiers from @/services/telegram should be updated to import directly from @/lib/telegram-identifiers instead. The re-export masks the real location and keeps the old import path working as a shim.

---

Naming: legacyNotes variable

CLAUDE.md prohibits "legacy" naming. In normalizeScratchpadToolInput, the variable legacyNotes violates this rule. If this backwards-compat handling for the notes field is intentional and needed, the variable should just be named notes. If callers of the old notes API have all been migrated, delete the handling entirely.

---

Potential bug: removeEntryKeys parsed but never applied in fromRuntimeScratchpadState

normalizeScratchpadToolInput now returns removeEntryKeys, but fromRuntimeScratchpadState merges entries via a simple spread without ever deleting the keys listed in removeEntryKeys. If this field is not consumed somewhere between tool invocation and fromRuntimeScratchpadState, the removal feature is silently a no-op. Please confirm where removeEntryKeys is applied, or add the deletion to the merge logic.

---

QueuedScratchpadStrategy.apply() -- verify prompt stripping is still handled

The old strategy explicitly stripped reminder text from the prompt via stripScratchpadReminderFromPrompt before queuing it to the system reminder context. Both that function and clonePromptForMutation are now deleted, and apply() just delegates. If the delegate still injects reminder text into the prompt, it will no longer be stripped -- the same text could appear in both the prompt and the system reminder queue. This may be intentional if the delegate was updated to not inject into the prompt at all, but worth explicitly confirming.

---

Positives

• Bug fix: event.payloads.prompt was incorrectly promptAfter -- good catch.
• reminderSink decoupling: Moving reminder queuing out of QueuedScratchpadStrategy into the runtime options is a clean architectural improvement.
• Moving telegram-identifiers.ts to lib/: Correct per the layer hierarchy -- pure identifier utilities with no service-level dependencies belong in lib/.
• Removing double computation in the old fromRuntimeScratchpadState (buildStoredScratchpadEntries was called twice) is a good fix.
• Explicit type annotations on lambda parameters in NostrInboundAdapter.ts and TelegramDeliveryService.ts are welcome.
• SummarizationStrategy rename is cleaner than LLMSummarizationStrategy.

[claude]

PR Review: Refactor context management and reorganize telegram utilities

First, a note on scope: the PR description describes extensive context management refactoring (scratchpad handling, reminder system, ToolResultDecayStrategy signature change, promptAfter → prompt rename, etc.), but the actual diff contains only 8 files with 12 additions and 12 deletions — all related to the telegram-identifiers relocation. The description is misleading either because those other changes were dropped from the branch, or the diff is incomplete. This review covers only what is in the diff.

---

What the diff does

1. Moves telegram-identifiers.ts from src/services/telegram/ to src/lib/
2. Updates all six import sites to the new path
3. Changes the re-export in src/services/telegram/index.ts from a local file re-export to an @/lib/ re-export
4. Adds explicit type annotations to array callbacks in NostrInboundAdapter.ts and TelegramDeliveryService.ts

---

Positive aspects

• Correct layer placement. telegram-identifiers.ts contains pure string-parsing/encoding utilities with zero dependencies. It belongs in lib/ per CLAUDE.md's layer hierarchy, since it is consumed by both nostr/ layer code (InboundEnvelopeEventBridge.ts) and services/ layer code. Moving it from services/telegram/ to lib/ eliminates a cross-layer awkwardness where nostr/ was importing from services/.
• No TENEX imports in the file itself. The moved file has no imports at all, which is exactly what CLAUDE.md requires for lib/: "ZERO TENEX imports."
• Clean cut. No backwards-compat shims or export { OldName } aliases were left behind — import sites were all updated.

---

Issues and observations

1. services/telegram/index.ts re-export is a backwards-compat shim (CLAUDE.md violation)

// src/services/telegram/index.ts
export * from "@/lib/telegram-identifiers";

This re-exports the identifier functions under @/services/telegram so that any caller using the old barrel path still works without updating. CLAUDE.md is explicit:

No backwards compatibility — Clean breaks only.
export { OldName as NewName } — REJECT.
export const legacyMethod = newMethod — REJECT.

All direct import sites within this PR were correctly updated to @/lib/telegram-identifiers. The re-export in index.ts should be removed. If any file elsewhere in the project still imports these functions via @/services/telegram, those imports should be updated directly rather than proxied through the barrel.

2. Explicit type annotations on NDK tag arrays are noise

// NostrInboundAdapter.ts — after the change
.map((tag: string[]) => tag[1])
.filter((value: string | undefined): value is string => !!value)

NDK defines NDKEvent.tags as string[][] and getMatchingTags() returns NDKTag[] (also string[]). TypeScript infers these types without annotations. The explicit annotations add visual clutter without adding safety, and they diverge from the surrounding code's style. This looks like it was added to silence a linter rather than to clarify intent. If the types were truly ambiguous the right fix would be to import the NDK tag type explicitly, not to inline the structural type.

3. PR description describes changes not in the diff

The summary mentions:

• Scratchpad entry serialization refactoring
• SummarizationStrategy rename
• ToolResultDecayStrategy callback signature change
• promptAfter → prompt rename
• QueuedScratchpadStrategy.apply() simplification
• reminderSink addition

None of these appear in the diff. If these are intentional and part of a separate branch/PR, the description should be scoped to what is actually being merged. If they were accidentally excluded, the PR should be updated.

---

Summary

The core change (moving the file to lib/) is architecturally correct. Two issues need addressing before merge:

1. Remove export * from "@/lib/telegram-identifiers" from src/services/telegram/index.ts. Find any remaining consumers of this via the barrel and update them to import from @/lib/telegram-identifiers directly.
2. Remove the redundant explicit type annotations in NostrInboundAdapter.ts and TelegramDeliveryService.ts, or leave them only if there is a concrete TypeScript error they fix (in which case, note why in a comment).
3. Align PR description with the actual diff to avoid confusion during review and in git history.

[claude]

PR Review: Refactor context management and reorganize telegram utilities

Overall this is a solid, well-motivated PR that fixes a real layer violation and updates the codebase to the ai-sdk-context-management 0.5.0 API. The deletions outweigh additions nearly 2:1, which is a good sign. A few things worth flagging:

---

Potential Bug: Stale fallback in toRuntimeScratchpadConversationEntries

src/agents/execution/context-management.ts lines 279–290:

function toRuntimeScratchpadConversationEntries(
    entries: StoredScratchpadEntry[] | undefined
): RuntimeScratchpadConversationEntry[] {
    return (entries ?? []).map((entry) => ({
        agentId: entry.agentId,
        agentLabel: entry.agentLabel,
        state: toRuntimeScratchpadState(entry.state) ?? {
            notes: "",         // ← old API field
            omitToolCallIds: [],
        },
    }));
}

The PR's entire purpose is to migrate away from the old notes field toward structured entries, but the fallback object still uses notes: "". If RuntimeScratchpadState in 0.5.0 no longer has notes, this is either a type error being suppressed or a runtime issue waiting to happen. The fallback should use the new shape — likely { entries: {}, omitToolCallIds: [] } — or TypeScript would have caught it if the type was properly updated.

---

Backward compatibility handling for notes

normalizeScratchpadToolInput wraps the legacy notes string as:

setEntries: { notes: legacyNotes }

This means "notes" becomes a named scratchpad entry key. It's pragmatic, but it silently promotes a legacy string value into the structured entries map using "notes" as an opaque key. This works, but it's worth being explicit in a comment or ensuring the key convention is documented somewhere — otherwise future callers won't know why notes is a magic key name.

---

Layer violation fix: correct

Moving telegram-identifiers.ts from src/services/telegram/ to src/lib/ is the right call. The file has zero imports and contains pure identifier-manipulation utilities. The nostr/InboundEnvelopeEventBridge.ts (Layer 2) previously imported from services/telegram/ (Layer 3), which was a clear downward violation. All import sites are updated correctly.

---

QueuedScratchpadStrategy.apply() simplification

async apply(
    state: ContextManagementStrategyState
): Promise<ContextManagementStrategyExecution | void> {
    return this.delegate.apply(state);
}

The simplification is clean. The reminder routing now goes through reminderSink in createContextManagementRuntime, which is the right separation. No concerns here.

---

Minor: reminderSink type coupling

reminderSink: {
    emit(reminder) {
        getSystemReminderContext().queue({
            type: reminder.kind,
            content: reminder.content,
        });
    },
},

Using reminder.kind as the queue type works, but if the library ever renames kind or changes the shape, this will silently break. A TypeScript satisfies check or a more explicit type assertion on the reminder parameter would make the contract clearer. Not blocking, but worth noting.

---

Summary

Layer fix	✅ Correct
API migration (0.5.0)	✅ Mostly correct
Dead code removal	✅ Clean
Potential bug	⚠️ Stale notes: "" fallback in toRuntimeScratchpadConversationEntries
Test coverage	No tests added or modified — given the behavioral changes to scratchpad state conversion, a regression test for toRuntimeScratchpadState/fromRuntimeScratchpadState roundtripping would reduce risk

The notes: "" fallback is the primary concern — if the type isn't enforced by TypeScript here, it could silently produce invalid state for agents with missing scratchpad entries.

[signet-bot-codex]

Review: Hephaestus (signet-bot-codex)

Scope Assessment

This PR is substantially larger than its title suggests. While it does refactor context management and reorganize telegram utilities, it actually implements the first two milestones of a transport-neutral runtime architecture (per the included RFC at docs/RFC-transport-neutral-runtime.md). The 112 changed files across ~10k additions include:

• Context management refactoring (scratchpad, decay, telemetry)
• Transport-neutral runtime abstractions (InboundEnvelope, AgentRuntimePublisher, RuntimeIngressService)
• Complete Telegram transport gateway implementation
• New identity service layer wrapping PubkeyService
• Audit scripts, doctor commands, diagnostic harnesses
• ConversationStore and ConversationResolver principal-context threading

Strengths

1. Well-structured seam introduction. The RFC-recommended approach of "add seams before moving files" is followed faithfully. AgentRuntimePublisher interface sits in src/events/runtime/ as a layer-2 contract while AgentPublisher remains in src/nostr/ as the concrete Nostr implementation.

2. Backward compatibility in scratchpad changes. normalizeScratchpadToolInput preserves the legacy notes parameter by converting to setEntries: { notes: legacyNotes }. The toRuntimeScratchpadState / fromRuntimeScratchpadState functions correctly map between stored and runtime state.

3. Good test coverage. 23 test files added/modified covering the new adapters, services, and stores.

4. Audit scripts. scripts/transport-runtime-audit.ts and scripts/identity-runtime-audit.ts enforce architectural boundaries — good engineering.

5. ConversationSummarizer improvement. Local metadata persistence now happens before the Nostr publish attempt, so relay failure no longer risks losing the local summary.

Blocking Concerns

1. buildDecayPlaceholder loses the truncate/placeholder distinction and drops description extraction.

File: src/agents/execution/context-management.ts

The old function had two branches: action === "placeholder" vs action === "truncate" with different output formats. The new function always produces placeholder-style text. More importantly, the description extraction from tool input (contextual hints like -- "Read contents of package.json") is completely removed. This degrades agent context quality during tool result decay.

Root cause: the callback signature change from DecayedToolContext to { toolName, toolCallId } drops access to input and action.

Recommendation: Either preserve description extraction by accessing tool input through the conversation store, or document why this information loss is acceptable.

2. ManagedContextWindowStatusStrategy.getContextWindow — AsyncLocalStorage may silently return undefined.

The old callback received { model, requestContext } from the strategy state deterministically. The new approach reads from modelMetadataStorage (AsyncLocalStorage), populated in the wrapping middleware's transformParams. If the ALS context is ever not set (e.g., strategy runs in a different async context from transformParams), getStore() silently returns undefined and getContextWindow returns undefined.

Recommendation: Verify the AsyncLocalStorage.run() scope in the middleware always covers strategy execution, or add a defensive fallback.

Non-blocking Concerns

3. getRuntimeCompletePrompt drops providerOptions and toolChoice from telemetry — these are now only added by buildFinalRuntimeTelemetryAttributes. If finalizeRuntimeComplete is never called, the attributes are lost.

4. process.exit(0) added unconditionally at end of main() in src/index.ts — safe only if handleCliError always calls process.exit on error. Worth verifying.

5. IdentityService.resetInstance() uses unsafe double-cast (undefined as unknown as IdentityService). Will cause runtime errors if getInstance() is called after reset without reinitialization. Appears test-only, but the method is public.

6. No integration test for the full handleChatMessage → NostrInboundAdapter.toEnvelope → RuntimeIngressService → AgentDispatchService.dispatch path.

7. InboundEnvelopeEventBridge.toEvent produces synthetic IDs (e.g., tg_1001_42) that may not round-trip safely through code expecting real Nostr event IDs. Bridge is labeled transitional, but the blast radius of synthetic IDs flowing through dispatch/resolver/store should be assessed.

8. PR size. At 112 files and ~10k additions, this bundles three distinct features (transport-neutral runtime, Telegram gateway, identity service) that would benefit from separate review cycles.

Verdict: Comment (leaning approve with minor changes)

Architecture is sound. Backward compatibility is maintained. Test coverage is good for the scope. The two blocking concerns reduce observability/context quality in ways that may be hard to debug later. If both are addressed or explained, this is ready to approve.

🔨 — Hephaestus