Skip to content

feat(core): compaction-as-memory sink, <untrusted-data> injection framing, tool-description quality (narrow per #200) - #204

Merged
Zongwei9888 merged 1 commit into
HKUDS:mainfrom
raymondginger2018-sudo:pr/three-features
Sep 6, 2026
Merged

feat(core): compaction-as-memory sink, <untrusted-data> injection framing, tool-description quality (narrow per #200)#204
Zongwei9888 merged 1 commit into
HKUDS:mainfrom
raymondginger2018-sudo:pr/three-features

Conversation

@raymondginger2018-sudo

Copy link
Copy Markdown
Contributor

TL;DR

Narrow resubmission addressing the scope feedback on #200: only the three pieces you called out, rebased on current main (c0a6a3c), nothing outside their own files. No CLAUDE.md, no .gitleaksignore edits, no P2/P3 modules.

Summary

Three independent, merge-order-independent changes:

1. Compaction-as-memory (compaction_summary_sink)

AgentRunSpec.compaction_summary_sink field + AgentSession._make_compaction_summary_sink(): compacted sessions deposit their summary into the memory vault on a daemon thread (best-effort, never blocks the turn; both the manual /compact and auto-compaction paths pass the sink).

  • New: tests/test_compaction_memory.py
  • Files: core/agent_runtime/runner.py, core/events/session.py, core/harness/memory.py (compaction writer block)

2. Injection-regression framing (<untrusted-data>)

memory_index() now frames injected memory content with an <untrusted-data> data boundary, keeping <system-reminder> reserved for instructions. core/loop/injection_regression.py is the regression corpus/surface-checker module (pure, stdlib-only).

  • New: tests/test_injection_regression.py, core/loop/injection_regression.py
  • Updated: tests/test_memory.py (asserts the new framing)
  • Files: core/harness/memory.py
  • Note for review: this module's only production caller is the excluded P1 memory_retrieval module; memory.py deliberately does not import the loop package (layering), so the module is exercised by its regression tests. If you prefer a real runtime call site I can wire it — flagging rather than guessing.

3. Tool-description quality checks

base.description_quality_issues() / sanitize_description() static quality checks; McpToolAdapter now applies sanitize_description (remote MCP description cap 2000).

  • New: tests/test_tool_description_quality.py
  • Files: core/agent_runtime/tools/base.py, core/mcp/tools.py

Test Plan

Local (CPython 3.14, editable install):

53 passed  # test_compaction_memory.py + test_injection_regression.py + test_tool_description_quality.py + test_memory.py
76 passed  # upstream regression: test_session_compaction, test_manual_compact, test_agent_session, test_mcp_runtime, test_mcp_server, test_plugins
4 failed   # all reproduce identically on pristine upstream/main (Windows-only env): plugins symlink WinError 1314, two agent/mcp-server cases
ruff: no new violations vs baseline (diff is ruff 0.16 vs CI-pinned 0.15 drift); format clean

Dependency note

Merge-order independent of sibling splits #188/#189. emit_event() inside the sink is a best-effort no-op — core/observability/events does not exist upstream.

Related

Closes the intent behind the #200 closure feedback (narrowed to the accepted scope).

…ming, tool-description quality

Narrow subset of the GenAI-for-Beginners borrowings, rebuilt on upstream/main
(c0a6a3c) per the scope the maintainer accepted on HKUDS#200. Nothing else from the
superseded P1+P2+P3 bundle is included (no CLAUDE.md, no .gitleaksignore edits,
no P2/P3 modules).

- compaction-as-memory: AgentRunSpec.compaction_summary_sink field +
  AgentSession._make_compaction_summary_sink() deposit compaction summaries into
  the memory vault on a daemon thread (best-effort, never blocks the turn).
  New: tests/test_compaction_memory.py.
- injection regression framing: memory_index() frames injected memory with an
  <untrusted-data> data boundary (system-reminder reserved for instructions);
  core/loop/injection_regression.py is the regression corpus/surface-checker
  module, exercised by tests/test_injection_regression.py. test_memory.py asserts
  the new framing. (Module's only production consumer is the excluded P1
  memory_retrieval module; memory.py deliberately does not import the loop
  package — flagging for maintainer.)
- tool-description quality: base.description_quality_issues()/sanitize_description()
  + McpToolAdapter applies sanitize (remote description cap 2000). New:
  tests/test_tool_description_quality.py.

Local verification: 53 tests in the new/changed files pass; 76 upstream
regression tests pass (session compaction, manual compact, agent session, mcp
runtime/server, plugins). The 4 remaining failures (plugins symlink WinError
1314, two agent/mcp-server cases) reproduce identically on pristine upstream/main
and are pre-existing Windows-only environment issues. ruff: no new violations
versus baseline (diff is ruff 0.16 vs CI-pinned 0.15 drift).

Note: emit_event() in the sink is a best-effort no-op — core/observability/events
does not exist upstream.

Co-Authored-By: Claude <noreply@anthropic.com>
Zongwei9888 added a commit that referenced this pull request Sep 6, 2026
Review repairs on top of #204:

- escape closing/opening boundary and system-reminder tags inside memory
  content so a poisoned MEMORY.md cannot end the data boundary early or
  forge a reminder block, and restore the one-closing-tag assertion the PR
  had dropped from test_memory.py
- drop core/loop/injection_regression.py: nothing in the runtime called it
  and memory.py carried its own copy of the constants; its two memory
  assertions move into test_memory.py
- remove the import of the non-existent core.observability.events module
  from the compaction sink, make the sink a no-op without a workspace, and
  fix its docstring
- keep the 8,000-character allowance for remote MCP tool descriptions
  (sanitize_description gains max_chars) instead of silently cutting to 2,000
Zongwei9888 added a commit that referenced this pull request Sep 6, 2026
@Zongwei9888
Zongwei9888 merged commit 7df226b into HKUDS:main Sep 6, 2026
14 checks passed
@Zongwei9888

Copy link
Copy Markdown
Collaborator

Merged today as bf52080 ("Merge repaired PR #204") with your commit intact. The repair commit on top (803690b) does four things worth knowing about:

  • Escapes closing/opening </untrusted-data> and <system-reminder> tags inside memory content. Without that, a poisoned MEMORY.md could end the data boundary early and open a reminder block of its own; the test_memory.py assertion that pinned "exactly one closing tag" had also been relaxed, so it is restored and a poisoned-note test added.
  • Drops core/loop/injection_regression.py. Nothing at runtime called it and memory.py already carried its own copy of the constants; its two memory assertions now live in test_memory.py.
  • Removes the core.observability.events import from the compaction sink (the module does not exist upstream) and makes the sink a no-op when there is no workspace.
  • Keeps the 8,000-character allowance for remote MCP descriptions (sanitize_description gained max_chars) instead of the silent cut to 2,000.

Two follow-ups that would each make a good small PR if you are interested: pass a real session_key into the anchor (every deposit is currently session_key=default), and write one file per compaction — with parallel sub-agents sharing a workspace, the single compactions.md currently loses entries under concurrent writes.

Thanks for the narrow resubmission; it is exactly the shape that makes review fast.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants