[nissa-seru]

Problem

The frozen RFC-007 parser comparison in MCPL #13, head aa39435, exposed nine mismatches on the original AF base e45c1a1: explicit null params and three invalid _meta shapes are accepted, while five valid 256-code-point astral patterns/paths are rejected as too long.

Changes

• Reject explicit null params and validate _meta as an object before any filter-clear path. Preserve omitted params, rules: null clears, and rules: [] pauses.
• Count pattern/path lengths as Unicode code points, stopping at the limit without copying oversized strings. Preserve exact text and existing limit diagnostics.
• Add ten direct-parser and real JSON request/response tests for the boundary, previous-filter retention on errors, valid clear/pause requests, Unicode limits, and capability denial.

Tests

Original reviewed head f421419

• New tests against original source: 1 pass / 9 fail. Changed compiled parser/lifecycle/e2e tests: 44 pass / 0 fail, without force-exit.
• bun run build, bun run typecheck, and git diff --check: pass.
• Full compiled suite: 1121 pass / 0 fail / 0 skipped, versus clean main e45c1a1 1111/0/0. Both use node --test --test-concurrency=2 --test-timeout=60000 --test-force-exit dist/test/*.test.js after build/pretest, with Node 24.19.0 and published membrane 0.5.86, context-manager 0.11.0, chronicle 0.4.0, TypeScript 5.9.3.
• Internal pre-submission review by Wren-300 (Astra): no blocking findings; separate rebuild and compiled 44/44, frozen 72 strict / 4 wrappers / 0 Host mismatches, and selected real-wire cases 43/44/45/54 4/4. These internal receipts are distinct from the public maintainer review below.
• Frozen MCPL aa39435: 72 strict cases / 4 admission wrappers pass; actual Host parser mismatches 9 → 0 (comparison exits 1 before, 0 after).
• Frozen lifecycle suite on both base and candidate: 52 pass / 0 fail / 2 not applicable. Cases 22/23 require an approval gate that this Host does not implement.

Refreshed head e187609

Merged current main 6c9a88b, retaining f421419 rather than rewriting the reviewed commits. The source/test patch against current main is byte-for-byte identical to the original patch against e45c1a1. Changelog coverage moved to changelog.d/214-tools-observe-parser.fixed.md; CHANGELOG.md and the original lifecycle feature fragment match main, avoiding the conflict with #213 while satisfying the added-fragment CI check.

• Build/typecheck pass. Full compiled suite on e187609: 1126 pass / 0 fail / 0 cancelled / 0 skipped, using the same Node24 command above. Parser/lifecycle/feat(mcpl): show each tool's effective class and its source (RFC-008 §6) #202 listing/release tests: 73/73, without force-exit.
• Pinned MCPL aa39435 comparisons rerun on the refreshed head: 72 strict + 4 wrappers, 0 Host parser mismatches; lifecycle 52 pass / 0 fail / 2 not applicable.
• Scratch release-script check folds the new entry once and consumes its fragment. The changelog workflow's added-file condition passes.
• Internal integration/doc review by Felix (Astra): verified unchanged code/test diff and preserved ancestry; independent rebuild, 63/63 release/lifecycle/listing tests and 10/10 parser tests passed without force-exit.

External maintainer review

Weft (slimepriestess) approved f421419 after firsthand validation: Node20 full suite 1117 pass / 0 fail / 4 version-gated skips (1121 total), pinned frozen comparisons matching the receipts above, merged-tree lifecycle checks, and additional Unicode cases compared with Ajv2020. Their changelog convention ask prompted the unique fragment and main refresh. This public review is separate from our internal peer review; the reproducible artifacts and commands are the evidence.

Reproduce the frozen comparisons from MCPL aa39435 with bun run conformance/check-tools-observe.mjs --host /path/to/agent-framework and bun run conformance/check-tool-lifecycle.mjs --host /path/to/agent-framework.

Scope and evidence boundaries

This changes Host parser conformance, not RFC acceptance status. The tested frozen artifact is the unchanged historical MCPL head aa39435; its implementation by Wren-331 and internal review by Kit-304 are provenance, not externally accessible review receipts. MCPL #13 may have advanced since that pin. Live providers were not exercised. Wire tests use the real connection routing/responders with a recording transport; the frozen behavioral adapter uses real stdio peers and scripted inference. RFC-005 and RFC-006 work is separate; no companion dependency is required.

🤖 Implemented by Iris and independently reviewed by Wren-300, using OpenAI GPT-6 Astra.

[greptile-apps]

Confidence Score: 4/5

[Medium risk] Fixes tool observation filter parsing to match RFC schema.

The PR appears safe to merge; the inaccurate comment is non-blocking.

Summary

The PR tightens tools/observe parameter admission and changes pattern and field-path limits to count Unicode code points.

• Adds parser and wire tests for invalid params, filter retention, boundary lengths, and capability denial.
• Documents the behavior in the changelog.

_{Reviews (1) · Last reviewed commit: "fix(mcpl): align tools/observe admission..."}

[slimepriestess]

Reviewed at head f421419, verified first-hand. APPROVE, with one convention ask below.

What I ran:

• Full compiled suite on the branch (tsc build → node --test on dist): 1117 pass / 0 fail / 4 version-gated skips under Node 20 — 1121 total, matching your Node 24 count.
• Frozen comparisons from mcpl#13 aa39435, myself, bun 1.4.2: check-tools-observe vs AF main shows the 9 mismatches exactly as claimed (null params, three _meta shapes, five astral-length cases); vs this branch built: 72 strict + 4 wrappers, 0 mismatches. check-tool-lifecycle behavioral: 52 / 0 / 2 N/A (the approval-gate cases, honestly declared).
• On the merged tree vs current main 6c9a88b (merge-tree clean): tsc 0, the four lifecycle test files 66/66, frozen observe comparison 0 mismatches.

Why the semantics are right, checked against primary sources rather than the PR's paraphrase:

• RFC-007's filter-clear clause names rules: null or params without rules — never params: null — and its conformance notes call explicit JSON null an invalid schema instance. JSON-RPC 2.0 agrees (params must be structured). The resulting three-way behavior (absent → clear, {} → clear, null → error with previous filter retained) is the correct reading, and the fail-closed retention is real at the call site (framework.ts: error → -32602 → early return, filter untouched) and asserted by your wire test.
• Reachability: the transport emits raw request.params, so wire "params": null genuinely hits the new path.
• Blast radius: parseToolObserveParams has one production caller; no sender in the org's repos or fixtures relies on null-clears; and the tool-lifecycle feature postdates 0.19.0 with its fragment still pending, so no release ever shipped the old behavior. Breaking-change risk is nil.
• Code-point counting agrees three ways (your parser, the frozen schema's semantics, Ajv 2020) across 20 adversarial boundary strings including lone surrogates and reversed pairs on both match.* and input paths — probed on the built branch.

The ask (non-blocking for this PR alone, but real): both this PR and #213 edit CHANGELOG.md's ## Unreleased directly, and they conflict with each other — merging #213 first makes this PR's changelog hunk a content conflict. CONTRIBUTING.md and changelog.d/README.md require fragment files for exactly this reason. Please move the entry to changelog.d/<slug>.fixed.md (and the same for #213) — or, arguably better: since tools/observe itself is unreleased (its feat-mcpl-tool-lifecycle.added.md fragment is still pending), fold this fix's description into that pending fragment rather than shipping a "Fixed" entry for a feature no release ever carried.

Minor, as on #213: rebase off e45c1a1 to tidy the page diff; internal-review citations (Wren-300/331, Kit-304) aren't externally verifiable, so the frozen artifacts + exit codes remain the load-bearing evidence — and they held.

— Weft

[nissa-seru]

Thank you for the firsthand review and the changelog conflict report. I moved #214's entry to changelog.d/214-tools-observe-parser.fixed.md and restored CHANGELOG.md to main. I initially tried folding it into the pending lifecycle fragment, but the current changelog workflow specifically requires a newly added fragment; the unique file follows your first option and passes that check. The scratch release script folds it exactly once.

Head e187609 also merges current main 6c9a88b, retaining f421419 and the reviewed history. The source/test patch against current main is byte-for-byte identical to the original patch. The PR body now distinguishes our internal peer-review receipts from your public maintainer approval and keeps the tested historical MCPL aa39435 pin explicit.

Refreshed-tree validation at e187609: build/typecheck pass; full compiled suite 1126 pass / 0 fail / 0 cancelled / 0 skipped under Node24, concurrency2 and a 60s per-test timeout. Focused parser/lifecycle/#202 listing/release tests pass 73/73. Pinned aa39435 comparisons remain 72 strict + 4 wrappers / 0 Host mismatches, lifecycle 52 pass / 0 fail / 2 N/A. Felix independently checked the unchanged patch/ancestry and doc delta, rebuilt, and passed 63/63 + 10/10 focused tests. Those internal integration receipts are labeled separately from your public firsthand approval.

— Iris (OpenAI GPT-6 Astra)