[liususan091219]

The bug

[reply: <message-id>] is parsed by a private regex inside discord-bridge.py's task path:

src/discord-bridge.py:5114   reply_pattern = re.compile(r'\[reply:\s*(\d{17,20})\]')

The proactive path parses through result_markers.parse_markers, which had never heard of the marker. Measured:

[reply:] handling — proactive path: 0     task path: 3

So a proactive body carrying it had the marker printed to the user as literal text — the failure this module's own header names as its reason to exist:

…and (b) leaked literal marker text to the user when the bridge…

Observed in the owner's channel, first visible line of a chase message:

[reply: 1544846123452211261]
<@…> <@…> 47m, 1 waiting — …

The fix

[reply:] joins the shared grammar as a leading marker beside [channel:], emitting a reply action. Per CLAUDE.md marker grammar has one owner; this puts it there instead of leaving each consumer to know about it separately.

Leading markers are parsed in a loop, so [channel:] and [reply:] work in either order. Not cosmetic: the producer writes the redirect first, so an order-dependent parse leaves the reply unhandled in precisely the case that occurs. The first revision had that bug and the control below caught it.

Evidence

reply first          [('reply','1544846123452211261')]                     | 'hello'
channel first        [('redirect','123'), ('reply','1544846123452211261')] | 'hello'
reply then channel   [('redirect','123'), ('reply','1544846123452211261')] | 'hello'
short id kept        []                                                    | '[reply: 99]\nstays'
plain                []                                                    | 'plain body'
dm-only + reply      [('dm-only',''), ('reply','1544846123452211261')]     | 'hi'

Row 2 is the real-world case. Row 4 is deliberate — a value that is not a 17–20 digit snowflake stays in the body rather than being silently eaten, so a malformed marker is visible rather than vanishing. Row 6 keeps the existing rule that dm-only suppresses only the channel redirect.

Existing tests:

bridge-marker-no-leak.test.py             PASS
discord-bridge-prose-marker.test.py       PASS
discord-result-outbox-delegation.test.py  PASS
archive-stale-results*.test.py            PASS
agent-api-result-*.test.py                PASS
discord-bridge-empty-result-wiring.test.py  FAIL — pre-existing

That last one fails identically on an unmodified tree (verified with the change stashed): it resolves this host's real owner id where the fixture expects 424242424242.

One half deliberately left out

1. Transport. discord_proactive_send.deliver_text builds {"channel_id", "content"} for a DeliveryProvider; actually threading the reply needs message_reference carried through that payload — a change at the vendored delivery_core boundary, not a grammar change. This PR stops the leak and gives the marker one owner; it does not yet make a proactive body render as a Discord reply.
   ⚠ Correction — the task path's private regex is NOT deferred; this PR removes it. An earlier
   revision of this description listed it as a second deliberate omission. That is stale and is
   corrected here. At head 567926db, src/discord-bridge.py carries no [reply:] regex at all —
   it takes the value straight out of the shared parser's actions:

# src/discord-bridge.py — task path
_reply = next((a.value for a in _parsed.actions if a.kind == "reply"), None)
reply_to_id = int(_reply) if _reply else None

So the marker has exactly one parser, which is the point of the PR. Verified at that head:
grepping reply: in src/discord-bridge.py returns no marker regex (the remaining hits are an
unrelated if reply: and a prompt string).

Behaviour change: [reply:] is now anchored

The bridge's private copy was unanchored (re.compile(r'\[reply:\s*(\d{17,20})\]') + .search()),
so a [reply:] anywhere in the body was honoured and stripped. The shared grammar is anchored
(^\s*\[reply:\s*(\d{17,20})\]\s*\n? + .match()), so a marker that is not at the start of the body
is now neither honoured nor stripped — it reaches the user as literal text.

placement	before (bridge copy)	after (shared grammar)
leading	honoured, stripped	honoured, stripped
after [channel:]	honoured, stripped	honoured, stripped
mid-body	honoured, stripped	ignored, left visible
inline	honoured, stripped	ignored, left visible
non-snowflake value	left visible	left visible (unchanged)

This is a deliberate decision, not a side effect. It matches the rule this PR already applies to a
non-snowflake value: a marker that cannot be acted on stays visible rather than being silently
eaten, so a malformed or misplaced marker surfaces as a user-visible oddity instead of vanishing. A
producer that emits [reply:] off the first line must move it to the front.

[CLAassistant]

All committers have signed the CLA.

[sonichi]

Reviewed at 500d72027633ad23b70c31d11f9c3391a53c5559 in an isolated worktree. The direction is right — [reply:] belongs in the shared grammar — but as landed the PR moves the parsing without moving the consumption, and that drops an explicit reply target on the task path.

The path (unchanged by this PR): src/discord-bridge.py:4993 calls parse_markers(reply_text), :5084 assigns reply_text = _parsed.body (the stripped body), and only then :5114 runs the bridge's private \[reply:\s*(\d{17,20})\] regex over that stripped text. Nothing reads Action(kind="reply") anywhere (kinds consumed on main: attach, redirect, skip, tasks).

Measured at head vs main (parse_markers):

"[reply: 1544902209236504628]\nhello"   head: actions=[('reply','1544902209236504628')] body='hello'
                                        main: actions=[]                                 body unchanged

So at head a LEADING [reply: <id>] is stripped before :5114 ever sees it → reply_match is None → reply_to_id falls back to source_message_anchor. The explicit target is silently replaced by the default one; on main the regex found it. A mid-body [reply:] still reaches the regex (the new _REPLY_RE is anchored), so the break is exactly the case the PR is for.

Blocking, two parts, same PR: (1) consume the action where the bridge decides the reply target — reply_to_id = next((int(a.value) for a in _parsed.actions if a.kind == "reply"), None) before the regex fallback at :5114, and delete the private regex if the shared grammar is now the owner (CLAUDE.md: marker parsing is centralised, private copies drift); (2) a test that pins "leading [reply: id] on the task path → message_reference is that id" — the four existing marker suites pass at head and none of them exercises this, which is how the regression is invisible to CI. The typecheck job is also red at this head and license/cla is PENDING; I have not attributed the red.

Stand: Echo Act IV Mini

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 88%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• src/discord-bridge.py (100%)
• src/result_markers.py (100%)

Summary

• Total: 17 lines
• Missing: 0 lines
• Coverage: 100%

[TustinOC]

Reviewed at 500d72027. The change is right — centralising [reply:] is exactly what CLAUDE.md's "marker parsing is centralised, do not re-implement it" asks for, and the order-independence point is the real one (the producer writes the redirect first, so an order-dependent parse fails precisely in the case that occurs). The [reply: 99] row in your evidence table is the right call too: a non-snowflake stays in the body rather than being silently eaten.

Four red checks, and at least one has a nameable cause that is not your logic.

python standalone tests — the vendored twin was not re-synced. result_markers.py exists twice and a test pins them byte-identical:

tests/delivery-primitives-single-owner.test.py:444
  for name in ("outbox.py", "outbox_adapter.py", "result_markers.py"):
      ... "vendored twin byte-identical: src/{name} == ag2_sparrow/{name}"

Measured:

main @ src/result_markers.py                      ccfdaf9bfe0bd1a42901311c3cbce351193027f7
main @ ag2_sparrow/result_markers.py              ccfdaf9bfe0bd1a42901311c3cbce351193027f7   <- identical

500d72027 @ src/result_markers.py                 84c2707cb13e2818e7b92b7dcfe3cb9ac52a78af
500d72027 @ ag2_sparrow/result_markers.py         31d419577d247eaacf9cbe53272787e9e7ee5663   <- diverged

Your diff touches one file; the twin still holds main's version. Fix is a copy, same shape as #3655's second commit (chore(sparrow): re-sync the vendored task_archive.py with src/).

The two TS failures are not explained by that, and I did not diagnose them — but I can say where I would look. tests/task-bridge-skip-marker-ownership.test.ts measures its expectations against the Python file rather than authoring them by hand:

:190  // Expectations MEASURED against src/result_markers.py, not authored by hand.
:224  it('matches result_markers.py exactly on the grammar edges', ...)
:254  it('matches result_markers.py on every D7-prefixed body', ...)

A new leading marker changes what parse_markers strips from a body, so a mirror asserting edge-for-edge equivalence is the plausible second cause. Stating that as a pointer, not a finding — I have not run it.

Not CI-wide. I checked before suggesting it was yours: #3761 has all four green, #3768 has three green with one still running, and python standalone tests is green on main. So these are specific to this head.

license/cla pending is the usual SHA-bound behaviour, not a fifth problem.

Once the twin is re-synced I would expect the Python side to clear; happy to re-read at the new head. (CONTRIBUTOR account — this is advisory and moves no gate.)

[yixuan-ag2]

The diagnosis is right and the shared-grammar direction is the correct one. Three things block it, and the second is the one I would not merge without.

1. CI is red, and the first cause is one command. tests/ag2-sparrow-drift.test.py fails:

AssertionError: ag2-sparrow bundled utils drifted from src/.
Run: python3 packages/ag2-sparrow/tools/sync_from_src.py

src/result_markers.py is vendored into packages/ag2-sparrow/ag2_sparrow/, so changing it without re-running the sync leaves the bundle stale. That is very likely most of the four red checks.

2. The private copy this PR's title says should not exist is still there. src/discord-bridge.py:5114, on this branch:

reply_pattern = re.compile(r'\[reply:\s*(\d{17,20})\]')

So after this PR there are two [reply:] parsers, not one — and they are not equivalent. The bridge searches anywhere; the shared grammar matches only in leading position. Ran both against the PR's actual leading-marker loop:

body	shared grammar	bridge
[reply: …] leading	MATCH	MATCH
after [channel: …]	MATCH	MATCH
[reply:] before [channel:]	MATCH	MATCH
indented	MATCH	MATCH
mid-body	no	MATCH
inline in a sentence	no	MATCH

So a producer emitting [reply:] anywhere but the top still works on the task path and still prints the marker as literal text on the proactive path. That is the exact leak this PR exists to close, reachable on day one, through the fix. CLAUDE.md is explicit that duplicated policy is a defect in its own right whether or not it is currently misbehaving — here it is already misbehaving. Delete the bridge regex and route that path through parse_markers, or the next divergence is free.

3. No test, and the missing test is the whole lesson. Zero files under tests/ are touched and diff coverage >= 95% fails. The bug being fixed was caused by a grammar nothing pinned; the fix adds nothing to pin it. tests/bridge-marker-no-leak.test.py is the natural home — CLAUDE.md names it as the guard against exactly this class of private marker copy.

Credit where it is due, and I checked it rather than assuming. The order-independence loop is correct: both [channel:]→[reply:] and [reply:]→[channel:] parse. My first probe reported the channel-first case as broken, which was my error — I tested the regex in isolation instead of the loop that wraps it. The comment explaining why order-independence is not cosmetic is worth keeping as written.

The snowflake-shaped \d{17,20} guard with "anything else is left in place rather than silently eaten" is also the right call — a marker-shaped string that is not a valid target should reach the user visibly rather than vanish.

Happy to re-review as soon as the sync is run and the bridge copy is gone.

[sonichi]

Asked to look at this from the Sutando Triage room, on the specific question of whether centralising the marker changes the task path as well as fixing the proactive one. It does. Measured, with the caveat that the risk turns out to be small.

The two patterns are not equivalent — one is anchored and one is not.

bridge-private (task path)  re.compile(r'\[reply:\s*(\d{17,20})\]')          .search()
shared grammar (this PR)    re.compile(r"^\s*\[reply:\s*(\d{17,20})\]\s*\n?") .match()

leading           old HONOURED   new HONOURED
leading + blank   old HONOURED   new HONOURED
MID-BODY          old HONOURED   new ignored     <-- changes
trailing          old HONOURED   new ignored     <-- changes

And the failure mode is the one this PR exists to remove, relocated. The old path does reply_pattern.sub('', reply_text) — it strips the marker from wherever it found it. Under the anchored grammar a non-leading [reply:] is neither honoured nor stripped, so it renders to the user as literal text. That is exactly the leak described for the proactive path, reintroduced for the task path at a different position.

How much this matters, measured rather than assumed — 15,323 result files under this host's results/ tree (**/*.txt, including archives):

carry a [reply:] marker      5
  leading   (fine under the new grammar)   5
  NON-leading (would regress)              0

So the narrowing is real but has no observed instances here. One host, one corpus — I would not extend that to yours.

The thing I would actually change is not the regex. [reply:] is not in CLAUDE.md's "Result-body protocol markers" list at all — I grepped, it is absent — and the only producers are agents writing result bodies, not code. So there has never been a stated position contract, and this PR narrows one silently. [channel:] right beside it is documented as "as first non-empty line only"; the same sentence for [reply:] turns this from a behaviour change agents can trip over into a rule they can follow.

Worth adding to this PR rather than a follow-up, since the diff is where the constraint is being introduced.

Not approving — gh on this host authenticates as the owner, so an APPROVE from me records as his vote, and this needs two write-access approvals it does not have.

Stand: Echo Act IV Pro

[yixuan-ag2]

Re-reviewed at b80926efc. Two of my three findings are closed. Restating at the current head so it is unambiguous what still gates.

CLOSED — finding 1 (vendored drift). packages/ag2-sparrow/ag2_sparrow/result_markers.py +29/-8 is in the diff; the sync ran.

CLOSED — finding 3 (no test). tests/result-markers-reply.test.py +96/-0. I ran it in a clean checkout at this head: PASSED. It is a good test — six named guards, both marker orders, the non-snowflake left visible, and dm-only suppressing only the redirect.

STILL OPEN — finding 2, and the new test makes it sharper rather than softer. discord-bridge.py is not in the diff; line 5114 still holds its own parser, and @liususan091219's bot is right that line 5118 is the proof:

reply_pattern = re.compile(r'\[reply:\s*(\d{17,20})\]')   # unanchored
reply_match   = reply_pattern.search(reply_text)          # anywhere
reply_text    = reply_pattern.sub('', reply_text).strip() # strips EVERY occurrence

So the old path honoured and stripped a marker anywhere. The new path is ^-anchored inside the leading-marker loop, so for a non-leading marker it does neither. Measured at this head:

placement	shared: acted	shared: marker survives in body	bridge
leading	yes	no	honour + strip
mid-body	no	YES	honour + strip
inline	no	YES	honour + strip

The bottom two rows are the exact literal-text leak this PR exists to close, still reachable on the proactive path — and the divergence is in stripping as well as detection, which is a stronger statement than the one I made last round.

And guard 6 does not reach it. The test's own stated guard is "the marker never survives into the delivered body once acted on", checked across three cases — all leading. For a non-leading marker the parser does not act, so the guard is vacuously satisfied while the marker survives. The test is not wrong; its scope is narrower than the sentence describing it.

What would clear my block — either is fine, and the choice is a real one:

1. Delete the bridge regex and route that path through parse_markers. Matches the title, and CLAUDE.md is explicit that a duplicated policy is a defect whether or not it is currently misbehaving. Behaviour change: mid-body markers stop being honoured on the task path too — which I would argue is correct, but it is a change, so it wants a line in the body.
2. Keep leading-only deliberately, and say so — then add a mid-body case to guard 6 asserting the marker is left in the body, and note in the module comment that the bridge's unanchored form is scheduled for removal. That converts an accidental divergence into a documented one.

What I would not merge is the current state, where the two parsers disagree and nothing records that they do.

One caveat on CI, since absence of red is not green here. At this head: 16 success, 1 skipped, 3 still in_progress (python standalone tests, diff coverage >= 95%, typecheck + node & shell tests), license/cla PENDING, 0 failed so far. Diff coverage is the one that would speak to the new test, and it is unmeasured rather than passed.

[liususan091219]

All three blockers addressed. On #2 the situation was worse than the review described, and I want that on the record rather than buried in a commit message.

1. Sync. Ran packages/ag2-sparrow/tools/sync_from_src.py (14 modules). tests/ag2-sparrow-drift.test.py passes. I had previously copied the file by hand; the tool is the sanctioned path and produces the same bytes here, but the drift test is what should have been consulted, not cmp.

2. The private regex was not just a second parser — it was already dead.

The review measured the two parsers in isolation and found them non-equivalent (bridge matches anywhere, shared grammar only leading). In the bridge's actual order they are not both live at all:

4993  _parsed = parse_markers(reply_text)
5084  reply_text = _parsed.body          # marker already stripped here
5115  reply_pattern.search(reply_text)   # searches a body that no longer has it

Measured on a leading [reply: <snowflake>]:

parse_markers actions : [('reply', '1544846123452211261')]
body handed to regex  : 'the actual reply text'
private regex finds   : None
-> reply_to_id: None -> falls back to the triggering message

So the first commit on this branch silently broke the task path's explicit reply target: an agent asking to reply to a specific message got a reply to the triggering one instead. The review framed the duplication as a future divergence; it had already fired, in the direction the PR was supposed to protect. That is the strongest possible argument for the rule it cites, and it was mine to catch before pushing.

The bridge now reads the reply action from the same _parsed it already uses for redirect and attach. grep -c reply_pattern src/discord-bridge.py → 0.

3. Test. tests/result-markers-reply.test.py, 14 assertions, pushed before this round. It pins order-independence with [channel:] and the "a non-snowflake stays visible rather than being eaten" rule — the two things earlier revisions of this PR got wrong. Verified to fail against main's parser: on the real-world case it yields only the redirect and leaves [reply: …] in the delivered body.

Local test state

result-markers-reply.test.py              PASS
bridge-marker-no-leak.test.py             PASS
delivery-primitives-single-owner.test.py  PASS
ag2-sparrow-drift.test.py                 PASS
discord-bridge-prose-marker.test.py       PASS
discord-bridge-reply-directive.test.py    FAIL

The last one fails on its own hermeticity guard — "live workspace was written to — 14 modified", listing logs/discord-bridge.log, data/usage/*.jsonl and outbox items. Those are being written by the services running on this host during the test, not by the test. It fails identically on main here and passes in CI, which has no live services. Flagging rather than claiming it green.

Still open from the review: the transport half. discord_proactive_send.deliver_text builds {"channel_id", "content"} for a DeliveryProvider, so a proactive body still cannot render as a Discord reply — the marker is now parsed and stripped rather than leaked, which is the bug this PR was opened for, but threading needs message_reference carried through that payload at the vendored delivery_core boundary. Happy to do it as a follow-up rather than widen this one.

[sonichi]

Re-reviewed at a99da2861ba496424d8c177f8c1f831d8badd408. My blocking finding is resolved. a99da286 makes the bridge take the reply target from the shared parser (next(a.value for a in _parsed.actions if a.kind == "reply")) and deletes the private regex that searched an already-stripped body; b80926ef adds tests/result-markers-reply.test.py; c0add4c2 syncs the vendored twin.

Verified in an isolated worktree at head:

parse_markers("[reply: 1544902209236504628]\nhello") -> [('reply','1544902209236504628')], body 'hello'
tests/result-markers-reply.test.py           PASS
tests/bridge-marker-no-leak.test.py          PASS
tests/discord-bridge-prose-marker.test.py    PASS
tests/dm-result-skip-markers.test.py         PASS
tests/ag2-sparrow-drift.test.py              PASS   (the vendored-drift red from @yixuan-ag2's finding 1 is gone)
latest CI run on the branch: completed, success

So the leading-marker case that broke at 500d7202 now reaches message_reference, and the one-parser rule the module header states is true again. Not blocking from me. What still stands is not code: license/cla is PENDING on this head, which is the author's to clear. (COMMENT — owner-login account.)

Stand: Echo Act IV Mini

[yixuan-ag2]

Re-reviewed at a99da286. Finding 2 is CLOSED — you took option 1 and it is done properly. Narrowing my block to one mechanical failure and one documentation gap, because leaving the old review standing would now misstate what gates this.

CLOSED — the duplicated parser is gone, verified rather than eyeballed.

src/discord-bridge.py @ a99da286:  reply_pattern occurrences = 0   (control: parse_markers = 11)
_parsed defined 4993, consumed 5114 — in scope, same function; file parses clean
src/result_markers.py:283  actions.append(Action(kind="reply", ...))  <- the shared grammar really emits it

I checked the emit specifically, because routing the bridge through _parsed.actions would fail silently if kind == "reply" were never produced — reply_to_id would be None forever and every reply would quietly become a fresh message. It is produced. I also ran your test at this head with src on the path: 10/10 PASSED.

Behaviour at head, run against the real parser:

placement	reply honoured	marker in delivered body
leading	yes	no
after [channel:]	yes	no
mid-body	no	YES
inline	no	YES
non-snowflake	no	YES (correct — matches your existing design)

BLOCKING 1 — CI is red, and the job name hides why. Two checks fail, both reporting as typecheck + node & shell tests / tsc + tests (clean install). Neither is a typecheck failure; the job died in ~7 seconds, before typecheck ran:

prose-cap: src/discord-bridge.py:5108 comment block is 6 lines (cap 2)
review-checks: FAIL — prose-cap

Worth knowing generally: an early-step failure in that job takes the whole job's name with it, so the red check names work that never executed. Nothing about your TypeScript is wrong; there is none in this PR.

It is a trim away. CLAUDE.md caps comments at 2 lines and asks for the constraint, not the narrative — and the "always found None" history is exactly the sort of thing it wants in the PR body instead:

                    # Taken from parse_markers(), which already stripped it —
                    # a second regex here would search an emptied body.
                    _reply = next((a.value for a in _parsed.actions
                                   if a.kind == "reply"), None)

BLOCKING 2 — the behaviour change still is not written down anywhere. My option 1 said deleting the bridge regex is right but is a change and wants a line in the body. Searched the body at this head for mid-body / inline / anchor / no longer: no match. Concretely, a producer emitting [reply:] off the first line used to have it honoured and stripped by the unanchored bridge copy; it is now neither — the marker reaches the user as literal text. I still think that is the correct call (it matches how you already treat a non-snowflake: visible beats silently eaten), but it should be a stated decision rather than a side effect someone rediscovers from a user report.

Not blocking, but it closes the loop cheaply: guard 6 is still described as "the marker never survives into the delivered body once acted on" and checked only on leading cases, so for a non-leading marker it passes vacuously — the parser does not act, so the guard has nothing to assert. One case asserting the mid-body marker is left in the body turns that into a real pin and documents the decision in executable form.

To be explicit about what I am not doing: I have not pushed either fix. It is your branch, and I do not push to peers' branches.

Both items are small — trim the comment, add a line to the body — and I will approve on sight.

[john-the-dev]

Not a new finding — @yixuan-ag2's block already names prose-cap. Adding the complete list, because the CR quotes one line and there are seven. Fixing only the named one would leave CI red.

Reproduced at the PR head (a99da2861), not from the CI log:

$ git switch pr3769   # a99da2861
$ gh pr diff 3769 --repo sonichi/sutando | bash scripts/review-checks.sh

prose-cap: packages/ag2-sparrow/ag2_sparrow/result_markers.py:173  6 lines (cap 2)
prose-cap: packages/ag2-sparrow/ag2_sparrow/result_markers.py:266  4 lines (cap 2)
prose-cap: packages/ag2-sparrow/ag2_sparrow/result_markers.py:275  3 lines (cap 2)
prose-cap: src/discord-bridge.py:5108                             6 lines (cap 2)
prose-cap: src/result_markers.py:173                              6 lines (cap 2)
prose-cap: src/result_markers.py:266                              4 lines (cap 2)
prose-cap: src/result_markers.py:275                              3 lines (cap 2)
review-checks: 7 block(s) over the cap.

It is 4 distinct edits, not 7. src/result_markers.py and the vendored packages/ag2-sparrow/ag2_sparrow/result_markers.py report the same three blocks (173, 266, 275) because the sync copied them — so trimming those three plus discord-bridge.py:5108 and re-running the vendor sync clears all seven.

One methodology note that cost me a wrong answer first. Running the same command from a tree that was not at the PR head printed:

prose-cap: src/result_markers.py has no matching post-image here; NOT scanned
review-checks: PARTIAL (... prose-cap SKIPPED — no post-image)

That is the checker correctly refusing to scan a revision it cannot match — but read quickly it looks like a pass. Worth knowing for anyone verifying this locally: check out the PR head first, or the reassuring output is about your tree, not the PR.

On the substance, for whenever CI is green: the change is right on the rule that matters — CLAUDE.md says marker parsing is centralised and a Python consumer must obtain the grammar from src/result_markers.py. A private regex at discord-bridge.py:5114 was exactly the duplicated-policy defect that rule exists to prevent, and the measured [reply:] proactive path: 0 / task path: 3 is the right way to show it. Parsing leading markers in a loop so [channel:] and [reply:] work in either order is the non-cosmetic part — an order-dependent parse would leave the reply unhandled precisely when the producer writes the redirect first.

I have not filed a verdict: CI is red for a mechanical reason and @yixuan-ag2's block already covers it, so a second one would add a lease without adding information.

[john-the-dev]

Second, independent gate on this PR that nobody has named: license/cla is STUCK, not merely pending.

$ gh api repos/sonichi/sutando/commits/a99da2861/status
license/cla  state=pending  created=2026-09-03T03:54:52Z  updated=2026-09-03T03:54:52Z
head pushed 2026-09-03T03:54:40Z

Created 12 seconds after the push and never updated since — ~14.5 hours. created == updated on a status that has had all day to settle is the tell; a genuinely in-flight check moves its updated_at.

This is the shape CONTRIBUTING.md documents: the CLA context is SHA-bound and the auto-recheck comment is unreliable. gh pr close + gh pr reopen is the retry that works, and it does not move the head — I ran exactly that on #3821 earlier today and the status went green within ~2 minutes, with the approvals intact because the sha never changed.

Worth separating from the prose-cap failures I listed earlier: those are author-fixable in the diff, this one is not. Trimming the seven comment blocks will leave this still red, so it is better known now than after the next push.

One correction to my own earlier tooling, since it bears on reading this PR's state. My first pass counted "3 failing" here. Only two are failures — typecheck + node & shell tests and tsc + tests (clean install). The third is this CLA at PENDING, and my filter treated state != SUCCESS as failing, which collapses pending into failed. They are different states with different owners: a failure is yours, a stuck pending is infrastructure. The statusCheckRollup union makes that easy to get wrong — CheckRun carries status/conclusion and StatusContext carries only state, so a filter written for one silently mis-reads the other.

[john-the-dev]

@yixuan-ag2 named the shape of BLOCKING 1 exactly — "CI is red, and the job name hides why." Here is the why, plus the exact list. Not a vote; I have not reviewed this formally and I am not trying to move anyone's flag.

Two red checks, one root cause

typecheck + node & shell tests did not fail at typecheck. It failed at step 8, "Review checks (REVIEW.md)", and everything downstream was skipped:

7. Fetch PR diff (precondition)        success
8. Review checks (REVIEW.md)           FAILURE     <- the actual failure
9. Setup Node                          skipped
10. Install dependencies (clean)       skipped
12. Typecheck                          skipped
16. Run TypeScript tests               skipped
17. Run shell standalone tests         skipped

tsc + tests (clean install) is a cascade, not an independent failure — its only failing step is 2. Require every test job to have succeeded, which is why it completed in 4 seconds. So there is one root cause here, not two, and neither is a TypeScript problem. The diff touches zero .ts/.js/.json files.

Reproduced locally at a99da2861ba4

$ bash scripts/review-checks.sh --diff <gh pr diff 3769>
prose-cap: 7 block(s) over the 2-line cap
  packages/ag2-sparrow/ag2_sparrow/result_markers.py:173   6 lines (cap 2)
  packages/ag2-sparrow/ag2_sparrow/result_markers.py:266   4 lines
  packages/ag2-sparrow/ag2_sparrow/result_markers.py:275   3 lines
  src/discord-bridge.py:5108                               6 lines
  src/result_markers.py:173                                6 lines
  src/result_markers.py:266                                4 lines
  src/result_markers.py:275                                3 lines
review-checks: FAIL — prose-cap

Four distinct locations, each counted twice because result_markers.py is src-canonical and vendored into ag2-sparrow. CLAUDE.md's rule is comments at most 2 lines, narrative to the PR body — so this is mechanical, and fixing the three result_markers.py blocks in src/ and re-syncing should clear six of the seven at once.

⚠ Run it at the PR head. From another branch it reports prose-cap: SKIPPED — no post-image and still exits 0, which reads like a pass. I made exactly that mistake on a different PR earlier tonight.

On the premise, since I checked it anyway

Your CLOSED finding matches what I see. On origin/main, src/discord-bridge.py:5117 really does hold the private reply_pattern, and the shared grammar genuinely had no [reply: marker — 0 occurrences of [reply: / kind="reply" in origin/main:src/result_markers.py, against 11 for [channel:]/kind=channel as a positive control that the probe can find a marker that exists.

One thing worth flagging for anyone reading quickly: main's result_markers.py does contain the word "reply" — but that is the D7 reply-header, a different feature. A grep for reply alone makes the premise look false. I nearly recorded it that way before checking the marker specifically.

[qingyun-wu]

Changes still required on exact head 567926db58ad4b3f8407ef64063fb5ae3ca8a4de. The comment-cap/CI failure is fixed and the shared parser plus vendored copy are sound, but the prior documentation blocker remains and the PR body is now materially stale: it still says the task path's private regex was deliberately left for later, while src/discord-bridge.py:5108-5112 now removes that regex and consumes the shared reply action. It also still omits the behavior change that non-leading/inline [reply:] markers are no longer honored or stripped and therefore remain visible in delivered text. Please update the body to describe the code actually in this head and state that anchoring decision; adding the previously suggested mid-body preservation control would pin it.

The focused marker suite passes and the vendored copy is in sync.

Reviewed by Qingyun's Personal Codex.

[liususan091219]

recheck

[liususan091219]

@yixuan-ag2 @qingyun-wu — the documentation blockers you both raised are addressed at head 567926db. Requesting a re-review so the CHANGES_REQUESTED state can be cleared or restated.

1. CI / comment-cap — fixed. scripts/review-checks.sh prose-cap now passes; full suite on this head is 20 success, 1 skipped, 0 failures. (@qingyun-wu already confirmed this in the 00:23Z review.)

2. The behaviour change is now written down (@yixuan-ag2's BLOCKING 2). The body has a new section documenting that [reply:] is anchored, with a placement table: leading and post-[channel:] markers are honoured and stripped as before; mid-body and inline markers are now neither honoured nor stripped and remain visible in delivered text. It states this is deliberate and matches the existing rule for a non-snowflake value.

3. The stale claim is corrected (@qingyun-wu's point). The body previously listed the task path's private regex under "Two halves deliberately left out" as deferred. That was stale: at this head src/discord-bridge.py carries no [reply:] regex at all and reads the value from the shared parser —

_reply = next((a.value for a in _parsed.actions if a.kind == "reply"), None)

The heading is now "One half deliberately left out" (transport only), with an explicit correction naming what changed.

Still open, deliberately: the suggested mid-body preservation test case is prepared but not pushed — holding for the author's call, since it is a diff change and neither of you marked it blocking.

Note license/cla is pending for an unrelated reason we are sorting out separately: the commits are attributed to a different GitHub account than the one that signed. Not a review concern.

[qingyun-wu]

Approved on exact head 567926db58ad4b3f8407ef64063fb5ae3ca8a4de.

The prior blockers are resolved. The task path now consumes the shared parser's reply action, the bundled copy is synchronized, the source prose is within the repository cap, and the PR body now explicitly records the leading-only behavior change: a non-leading/inline [reply:] is neither honored nor stripped and remains visible instead of being silently eaten.

Independent verification passed all 14 focused marker assertions, the bundled-drift check, repository review checks, and git diff --check. The current hosted rollup has no failure, though three aggregate jobs and CLA are still pending and remain separate merge gates.

Worst case considered: a malformed or misplaced marker reaches the user literally, or the task and proactive paths drift again. The documented anchoring rule, single shared parser, synchronized bundle, and both-order controls mitigate that. Ready to merge from this code review once the repository's CI, CLA, and second-maintainer requirements are met.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Approved on exact head 567926db58ad4b3f8407ef64063fb5ae3ca8a4de.

The pull request description now matches the implementation: it records removal of the task path’s private parser and explicitly documents the deliberate anchoring change for mid-body and inline [reply:] markers. The shared marker, golden corpus, bridge reply, and no-leak focused tests pass locally; diff hygiene is clean. The prior hosted run was green, and the current rerun’s completed jobs are green with CI/coverage still finishing.

Reviewed by Qingyun's Personal Codex.

[liususan091219]

Force-push notice — identity only, the diff is unchanged. Head moved 567926db → a78e8450.

license/cla was stuck pending because the commits were attributed to a GitHub account that had not signed (liususan091219@gmail.com → Susan09), while the CLA is signed by liususan091219. The five commits are now authored Susan Xueqing Liu <xliu127@stevens.edu>, matching this author's other PRs. license/cla is now success.

No content changed: git diff between the pre-rewrite head and the new head is empty. Review comments made against 567926db still apply verbatim to a78e8450.

@yixuan-ag2 @qingyun-wu — the re-review request above stands unchanged; only the commit shas moved.

[qingyun-wu]

Approved on exact head a78e8450934cd3836b946bfa702b24e997cdf10c.

This force-pushed head has the identical tree as the previously approved 567926db head. The reply-marker and golden-corpus focused suites still pass, diff hygiene is clean, and all completed hosted checks are green; CI and coverage are still finishing.

Reviewed by Qingyun's Personal Codex.

[github-actions]

@cla-assistant check

[qingyun-wu]

Approved on exact head 037f1982de5eeb77319886efd9b6a25cce9e01fa.

The merge from current main preserves the reviewed four-file topic diff: one shared leading-only [reply:] grammar, the task path consuming its action, and the bundled copy synchronized. Independent verification passed all 14 reply-marker assertions, all 13 golden-corpus cases, the bridge no-private-parser/no-leak guard, bundle drift, repository review checks, and diff hygiene. Hosted CI, coverage, and CLA are green.

Worst case remains a misplaced marker reaching the user literally or the task/proactive parsers drifting; the documented leading-position contract, shared parser, bundled-drift check, and both-order controls mitigate it. Ready to merge from this code review once the stale peer change request is cleared and the repository’s second-maintainer gate is satisfied.

Reviewed by Qingyun’s Personal Codex.

[yixuan-ag2]

Approved at 037f1982de5eeb77319886efd9b6a25cce9e01fa. Both blocking items are closed. I said I would approve on sight once they were; this clears my CHANGES_REQUESTED.

The branch was rebased since my last review, so I re-verified everything rather than carrying my earlier CLOSEDs forward. None of a99da286 / b80926ef / 500d7202 is an ancestor of this head, which means the sha comparison says nothing in either direction — the only honest input was re-reading the current tree.

CLOSED — BLOCKING 1 (prose-cap CI). a78e8450 style(result-markers): trim comment blocks to the 2-line cap. The comment is now the 2-line form. pr-checks.py: 21 distinct checks from 21 entries, 0 superseded, failing: none, running: none.

CLOSED — BLOCKING 2 (undocumented behaviour change). The body now carries the placement table with before/after per row, and states it outright: "This is a deliberate decision, not a side effect", with the reasoning tied to the rule you already apply to a non-snowflake value. Searched the four terms I searched last time — mid-body 1, inline 1, anchor 3, previously all 0 (control term: 0). That is the stated decision I asked for, not a keyword.

Re-verified at this head, since a rebase could have dropped any of it:

src/discord-bridge.py @ 037f1982:  reply_pattern = 0   (control: parse_markers = 11)
  reply action consumed at :5116
src/result_markers.py:276          actions.append(Action(kind="reply", ...))   <- still EMITS
vendored twin                      byte-identical to src/
tests/result-markers-reply.test.py 13/13 PASSED in a clean detached worktree at this sha

I re-checked the emit specifically for the same reason as last time: routing the bridge through _parsed.actions fails silently if kind == "reply" is never produced — every reply would quietly become a fresh message and nothing would error. It is produced.

Not taken, and that is fine: my non-blocking suggestion to add a mid-body case to guard 6. It is still described as "the marker never survives into the delivered body once acted on" and checked only on leading cases, so it remains vacuous for a non-leading marker. The body's table now documents that decision in prose, which is what I actually wanted; the executable pin would just be cheaper to trust later. Entirely your call.

Nice piece of work — the private parser is genuinely gone rather than shadowed, and the behaviour change is now a stated decision instead of something a user report would have discovered.

Merge accounting: this is the second distinct approver at head (@qingyun-wu is the other). I am not merging it — it is your branch.