Skip to content

Keep ACP responsive with lazy Honcho tools memory#19102

Open
wo-o wants to merge 2 commits into
NousResearch:mainfrom
wo-o:fix/acp-honcho-tools-memory
Open

Keep ACP responsive with lazy Honcho tools memory#19102
wo-o wants to merge 2 commits into
NousResearch:mainfrom
wo-o:fix/acp-honcho-tools-memory

Conversation

@wo-o

@wo-o wo-o commented May 3, 2026

Copy link
Copy Markdown

Summary

  • keep Honcho tools-only lazy sessions responsive for ACP/Zed while still mirroring completed turns and explicit memory writes
  • skip nonessential Honcho context/config reads on tools-only startup/write paths to avoid launch stalls
  • dedupe exact adjacent Honcho turn syncs to prevent duplicate durable memory rows
  • tolerate ACP permission API drift (allow_permanent, option_id/optionId)

Why

ACP/Zed sessions need Honcho memory enabled, but eager/hybrid Honcho paths can block startup/first prompts. The existing tools-only lazy path avoided the latency, but normal completed turns were not written until a tool initialized the session. This change preserves the low-latency tools-only path while ensuring Honcho still observes the conversation.

Verification

  • uv run --extra dev --extra honcho --extra acp python -m pytest tests/honcho_plugin/test_session.py tests/acp/test_server.py -q → 171 passed
  • uv run --extra dev --extra honcho --extra acp python -m py_compile acp_adapter/permissions.py plugins/memory/honcho/__init__.py plugins/memory/honcho/session.py
  • Manual remote ACP E2E with Honcho enabled:
    • initialize/session/new/load/prompt passed
    • Honcho messages had one user row and one assistant row for the unique E2E marker
    • Honcho message_embeddings had both rows in synced state
    • no wrapper SIGABRT/Fatal/EOF/Traceback/error lines in final scan

Notes

The manual E2E was run against a self-hosted Honcho stack with deriver/api/redis/postgres healthy.

Honcho tools-only lazy initialization avoided expensive context reads during ACP startup, but it also skipped completed-turn and explicit memory-write mirroring. Initialize the Honcho session inside background write paths, skip nonessential tools-mode context/config reads, and dedupe exact adjacent syncs so ACP sessions can keep Honcho enabled without launch stalls or duplicate memory rows.\n\nACP permission approval also accepts the newer allow_permanent argument and both snake/camel option id spellings so schema drift does not crash approval callbacks.\n\nConstraint: ACP clients need first prompt latency bounded while preserving durable Honcho observation.\nRejected: Disable Honcho in ACP profiles | fixes launch but loses required memory behavior.\nRejected: Keep tools-only writes lazy until first tool call | drops normal conversation turns.\nConfidence: high\nScope-risk: moderate\nDirective: Do not reintroduce synchronous context/dialectic reads on tools-only ACP startup without latency E2E tests.\nTested: uv run --extra dev --extra honcho --extra acp python -m pytest tests/honcho_plugin/test_session.py tests/acp/test_server.py -q\nTested: uv run --extra dev --extra honcho --extra acp python -m py_compile acp_adapter/permissions.py plugins/memory/honcho/__init__.py plugins/memory/honcho/session.py\nTested: remote ACP E2E initialize/session/new/load/prompt with Honcho enabled; DB messages and message_embeddings synced once each; no SIGABRT/EOF/errors\nNot-tested: full repository test suite
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/acp Agent Communication Protocol adapter comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers labels May 3, 2026
Zed treats streamed chunks as incomplete until session/prompt returns, so ACP must not run auxiliary title generation before committing the final assistant message. Move the final message update onto the hot path and defer auto-title generation to a daemon thread so a slow title LLM cannot hide completed answers from the editor UI.\n\nConstraint: ACP clients need the session/prompt response to finalize a visible turn\nRejected: Disable title generation entirely | preserves useful titles as best-effort background work\nConfidence: high\nScope-risk: narrow\nDirective: Do not place auxiliary LLM calls before ACP final message update or prompt response\nTested: python3 -m py_compile acp_adapter/server.py\nTested: Live ACP finalization smoke test returned final JSON-RPC response in 2689ms\nNot-tested: Full upstream CI
@wo-o

wo-o commented May 3, 2026

Copy link
Copy Markdown
Author

Follow-up ACP finalization fix pushed in acfefa252.\n\nRoot cause found during Zed ACP smoke testing: the agent streamed agent_message_chunk, then ACP ran auxiliary title generation before sending the final assistant update / session/prompt response. If title generation or provider I/O stalled, Zed saw chunks but never finalized the visible turn.\n\nFix: send the final assistant message to the ACP client first, return session/prompt normally, and run auto-title as best-effort daemon background work.\n\nValidation:\n- python3 -m py_compile acp_adapter/server.py\n- Live ACP smoke test via the Zed wrapper: first token 2689ms; final JSON-RPC session/prompt response also 2689ms; saw_token=True, saw_final=True.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/acp Agent Communication Protocol adapter comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants