memory: github-mcp-strips-html-comments

[WillTaylor22]

One-line memory entry from PR #12 review-feedback round 2.

Captures the durable learning that the GitHub MCP update_pull_request tool silently strips HTML comment lines from the PR body field — even from inside markdown code spans. This is what blocked PR #12 from satisfying the pr-session-id-marker convention: the marker line gets persisted everywhere except where the webhook reads it.

Cross-refs ENG-25 (https://linear.app/personal12o38452384576928345/issue/ENG-25/credentials-needed-github-mcp-update-pull-request-strips-html-comments) for the proposed systemic fixes.

Side-experiment result: opening this PR via create_pull_request was also intended as the test for whether the create path shares the same filter as the update path. It does. This PR's initial body — and notice the corruption visible in the GitHub-rendered view — confirms MCP strips HTML comment tokens out of both create_pull_request and update_pull_request bodies, before markdown parsing (so even code-span escapes like the one I tried in the first version of this body get eaten, leaving stray empty backticks). The original create call also dropped the trailing marker line that should have followed this paragraph.

That kills option (2) in ENG-25's "investigation worth doing" footnote (set marker on create), so the systemic fix is one of:

1. Strip-protect the MCP body filter (whitelist the marker shape, or at least don't strip from inside code spans).
2. Change the marker shape in the convention + webhook regex to something MCP doesn't filter.
3. Provision a GITHUB_TOKEN so agents can PATCH api.github.com directly.

This PR itself does NOT carry the convention marker — by definition can't, and that's the whole point of the entry.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
self-managing-codebase	Ready	Preview, Comment	May 26, 2026 9:44am

[WillTaylor22]

AGENT_REVIEW: APPROVED — Well-formed memory entry that captures a real, reproducible MCP bug; the PR itself is the test artefact and the missing session-id marker is the expected experimental result, not a compliance failure.

What I checked:

• Real diff (three-dot, what GitHub merges): only .claude/memory/MEMORY.md (+1 line) and the new .claude/memory/learnings/2026-05-26-github-mcp-strips-html-comments.md (+1 line). The two-dot diff vs main also shows reversions in README.md and ai-manager/manager.agent.yaml, but those are an artefact of the branch being based on 32967ca (before e1d20f3 / 59c48ab landed on main). The single commit on the branch (4f45041) only touches the two memory files, and the merge-base diff confirms that — no actual revert risk on squash-merge.
• Index format matches the existing [type/slug](file.md) — one-line hook convention; filename follows the date-prefix pattern used by sibling learnings.
• MEMORY.md size: 14 lines after the change; cap is 200. learnings/ count: 4; no cap concern.
• Content: documents an empirically confirmed behaviour (two prior round-2 attempts on PR memory: pw-capture-script-needs-executable-path #12) and points at ENG-25 for the systemic fix. Saves future sessions from a guaranteed-failing round.
• CI: Vercel status is success for 4f45041.
• No code change → no build/lint/e2e needed.

On the pr-session-id-marker convention:

• The PR body deliberately omits the marker as part of the side-experiment the body describes ("If the marker below survives the create call…"). The absence of the marker in the persisted body is the experiment's result — create_pull_request shares the strip filter with update_pull_request. That is itself a useful follow-up data point for ENG-25.
• Requesting changes for "add the marker" here would just re-run the bug the PR is documenting. Approving is the right call; the fix belongs in ENG-25, not in another round on this PR.