Trailing HTML comment parsing

[dwin-gharibi]

Summary

trailingHtmlComments can return an "HTML comment" that spans visible prose. It
scans backward from the end of a comment body, pairing each --> with the nearest
preceding <!--, without checking that the two actually delimit one comment. A
stray --> in prose therefore bridges back to an earlier opener and swallows
everything in between — including any real ClawSweeper marker in that span.

Impact

This is the parser that recovers ClawSweeper's durable state markers from its own
published comment. When the bridge happens, the marker inside the swallowed span is
no longer independently recoverable:

<!-- clawsweeper-verdict:needs-human item=321 sha=head -->
The rust operator `-` then `->` renders as -->
<!-- clawsweeper-review item=321 -->

returns

["<!-- clawsweeper-verdict:needs-human item=321 sha=head -->\nrenders as -->",
 "<!-- clawsweeper-review item=321 -->"]

The first entry is not a comment. It contains an interior --> and visible text.

--> is not exotic in these bodies: Mermaid flowchart edges are literally A --> B,
and ClawSweeper renders a Mermaid architecture diagram in its own reviews. Rust,
Haskell, and ASCII diagrams produce it too.

Reachability — please read before rating severity

I could not construct a body from the real renderer that triggers this, and I
tried hard. The trigger needs all three of:

1. an earlier <!-- in the body to bridge back to;
2. visible prose ending in --> after it;
3. the trailing marker block immediately after that prose.

Four realistic bodies — a Mermaid diagram in a closed fence, the same with an
unclosed fence, prose ending in an arrow, and a table cell ending in an arrow —
all parse correctly even today, because condition 1 is missing. The durable
comment's only mid-body opener is the <!-- clawsweeper-review-history --> marker
that renderReviewHistorySection emits, and </details> always follows it before
any trailing prose.

Both consumers are also fail-safe today:

• clawsweeper-review-comment-state.ts anchors both ends with [^>]*, so a blob
  containing > fails to match and the marker is ignored;
• review-recovery-label-backfill.ts uses a start-anchored matcher and then scans
  the whole entry for name=value, but its canonical check is strictly anchored,
  so a blob makes the recovery label be retained rather than wrongly cleared.

So this is a latent parser defect that is currently safe by accident of its
consumers' regexes, not a live incident. The reason it is still worth fixing:
review-recovery-label-backfill.ts harvests attributes from the entire entry with
a loose matcher. If a blob ever satisfies its canonical check — one regex
refactor away — prose-derived attributes become authoritative for a
label-clearing decision.

Rate it accordingly; I would not call it more than a P3.

Root cause

src/review-comment-markers.ts:

if (!value.endsWith("-->", end)) break;
const commentStart = value.lastIndexOf("<!--", end - 3);
if (commentStart < 0) break;
trailing.push(value.slice(commentStart, end));   // never checks what is between

An HTML comment ends at its first -->. The pairing above never verifies that.

Suggested fix

Reject the candidate unless the opener's first terminator is the one being
matched:

if (value.indexOf("-->", commentStart + 4) !== end - 3) break;

Anything else means prose lies between them, so the trailing block has ended. This
makes the function honor its documented contract — "the final contiguous comment
block" — and changes nothing for bodies with a clean trailing block.

Existing coverage gap

test/review-comment-markers.test.ts covers the mirror case (prose containing an
unmatched opener) and an adversarial unterminated suffix. The unmatched
closer is the untested sibling.

Environment

Repo	openclaw/clawsweeper @ 0588bda9
Node	v24.19.0 (Crabbox local-container)

[clawsweeper]

Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve. Reviewed August 14, 2026, 10:13 PM ET / August 15, 2026, 02:13 UTC.

Summary
Keep open: current main still has the unguarded backward pairing, and the focused open PR #1070 explicitly owns the repair.

Reproducibility: yes. Current source pairs the final terminator with the nearest earlier opener without rejecting an interior terminator, and the issue supplies a focused input shape.

Next step
An open linked PR already owns the focused repair, so dispatching another fix would duplicate contributor work.

Review details

Best possible solution:

Review and land the narrow parser-boundary guard with regression coverage through #1070, preserving valid trailing-marker recovery.

Do we have a high-confidence way to reproduce the issue?

Yes. Current source pairs the final terminator with the nearest earlier opener without rejecting an interior terminator, and the issue supplies a focused input shape.

Is this the best way to solve the issue?

Yes. The linked PR's narrow boundary check and targeted regression coverage are the maintainable repair path, subject to that PR's own review.

AGENTS.md: found and applied where relevant.

Remaining risk / open question:

• Current consumers reject the malformed entry today, but the parser can still emit a non-comment blob that a future consumer could accidentally treat as authoritative.

Codex review notes: model internal, reasoning high; reviewed against 9fb6d6bfe562.

Label changes

Label justifications:

• P3: This is a source-proven latent parser defect whose current consumers fail safely for the malformed entry.
• impact:other: The affected surface is durable review-marker recovery and label reconciliation, outside the more specific impact categories.

Evidence reviewed

What I checked:

• Current parser still permits the malformed span: The backward scan takes the nearest preceding opener for a trailing terminator without verifying that it is the opener's first terminator. (src/review-comment-markers.ts:10, 51ac499c741b)
• Durable-state consumers use this helper: Review-state parsing and recovery-label backfill consume the returned trailing comments, so the defect is on an owned automation path. (src/clawsweeper-review-comment-state.ts:340, 9fb6d6bfe562)
• Existing coverage does not cover a stray closer: Current tests cover an unterminated suffix and an unmatched opener, but not a visible-prose --> between an opener and the final marker. (test/review-comment-markers.test.ts:25, 51ac499c741b)
• Feature provenance: Blame attributes the current helper and its initial tests to the commit that introduced the implementation now on main. (src/review-comment-markers.ts:10, 51ac499c741b)
• Open implementation candidate: Provided GitHub context shows fix: stop trailing comment parsing from spanning visible prose #1070 is open, cleanly mergeable, and explicitly closes this issue. (40dcf942af70)
• Maturity classification unavailable: The required shortlist helper reported that qa/maturity-scores.yaml is absent, so no stable-maturity label is asserted. (qa/maturity-scores.yaml)

Likely related people:

• Martin Cleary: Current-main blame and history attribute the extracted parser and its original tests to this commit. (role: introduced current marker-recovery behavior; confidence: high; commits: 51ac499c741b; files: src/review-comment-markers.ts, test/review-comment-markers.test.ts)

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.