Repository navigation
feat(gateway): a bare [thread] asks the broker to thread a task answer on its ask - #5326
Conversation
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 91%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
|
liususan091219
left a comment
liususan091219
left a comment
There was a problem hiding this comment.
Head moved to 1e013d8 while I was reviewing. I read the delta from a24fe8f: one commit, so that on a proactive file a rooted [thread: $root] wins over a bare [thread] in either order (remote_gateway_bridge.py _proactive_thread_root), plus test case g in gateway-proactive-thread. The fix is correct, proactive-only, and does not touch the task-result path reviewed below. At the time of writing, CI at 1e013d8 is still running (14 pass, 2 skipped, 18 pending) and license/cla shows no status yet (absent, not green).
Reviewed at a24fe8f. I found no blocker. This is not "Ready to approve" yet: nobody has approved this head, and I could not read the backend half.
What I checked
- The new tests at head:
gateway-task-result-thread-askpasses, andresult-markers-threadandclaude-md-conduct-rulesboth pass. With the parent's (d6f7abb) code and docs put back under the same tests: 9 and 4 failures, the same counts the PR body gives. - Revert control: I removed the 400 fallback in
provider_ag2space.py:47. Only case g fails (4 checks). - Gates at head, all green:
delivery-primitives-single-owner,events-plane-boundary,ag2-sparrow-drift,packages/ag2-sparrow/tools/test_no_drift.py,ag2space-provider,result-markers,result-markers-golden,bridge-marker-no-leak,sparrow-result-guard,agents-md-sync.scripts/review-checks.shpasses. CLAUDE.md is 40957 bytes.agents-md-sync.sh --checksays AGENTS.md is up to date. - CI: 34 pass, 2 skipped.
license/cla: success (1 context). No review threads.
The questions I was asked to check
- Can the 400 fallback post twice? The plain re-post runs in the same
deliver()call, with the same outbox item and the same result id (provider_ag2space.py:41-52). The server dedupes on that id, so even a 400 that had already recorded the message could not produce a second post. If the re-post fails, the existing handlers take over unchanged. A 400 on it is a permanent refusal, the same as a 400 on main. A 5xx is indeterminate and gets retried. A timeout or URLError is indeterminate too. A retry sends the stored payload again, with the field, so it falls back again the same way. - A 400 for some other reason, such as the body: the client sends one extra POST, gets a second 400, and refuses permanently, the same as main. Nothing is hidden. A 400 on an envelope without
threadis still one call (with the condition loosened, the existingag2space-providertest fails withIndexError: pop from empty list). - Merging before the backend deploys:
/v1/resultsignored the unknownmetadatakey when #3611 shipped ahead of backend#882, so the server dropping unknown keys has precedent. But I could not check today's production behaviour, or #2138's claims ("400 records nothing", "ignored on a redirect", 400 and not 422).ag2-space/ag2space-backendreturns 404 to this account whileorgs/ag2-spacereturns 200, so the account lacks a grant to that repo. The name is not wrong. Note: if a strict server ever answered 422, the fallback would not fire and the answer would be refused for good (send_failure_policy.py:12; 422 is not transient). - Marker grammar: the marker goes through
src/result_markers.py(_THREAD_ASK_REat :204, the loop at :313), and the vendored copy is byte-identical.[thread]and[thread: $id]give different actions. A skip marker still ends the parse, and the marker is stripped from the body. Orphan recovery postsparsed.bodywithout the field (remote_gateway_bridge.py:5199). - Other placements:
[dm-only]already removes the redirect, so the answer goes to the asking room, as on main. A proactive file (which is how a voice result goes out) is stripped and posted top level (case h).
Nits (not blocking)
- The client still sends
"thread": "ask"alongside a re-stitched[channel:](remote_gateway_bridge.py:4962). Only the backend ignoring it keeps that answer out of a thread, and that is the part I could not check. Droppingthread_askwhenredirectis set would cost one line. Case f doesn't assert either way, so please pin it. - Test gap: if the fallback fires on any HTTP status instead of only 400, every test still passes (I tried it). Please add a case where the server answers 503 with the field: it should stay indeterminate and keep the field for the retry.
Still owed (named, not blocking): the live post-restart round trip once backend#2138 deploys, which the PR already lists.
What would make this "Ready to approve": one maintainer approval at this head, ideally from someone who can read backend#2138 and confirm the 400 and redirect behaviour.
— Sutando (susans-MacBook-Pro)
keweichen
left a comment
keweichen
left a comment
There was a problem hiding this comment.
Requesting changes at 1e013d8d0292c18cf51aebedcd45ca5ff0f29e23.
The happy path is well covered: I reproduced the task-result, parser, provider, proactive, docs-sync, diff, and review checks; the guard/attribution/attachment/outbox path stays intact, and all reported CI checks are green (two expected skips, CLA green). I also reviewed the one-commit delta from the requested a24fe8f5; it correctly fixes the proactive bare/root ordering case in both permutations.
There are still three code/contract blockers plus the repository's live-path gate:
-
Blocking — the bare-marker regex silently truncates ordinary leading prose.
_THREAD_ASK_REis^\s*\[thread\]\s*\n?, so it does not require[thread]to stand alone on a line. On this head:'[thread]ing is a library primitive' -> actions=[('thread-ask', '')], body='ing is a library primitive' '[thread] prose on one line' -> actions=[('thread-ask', '')], body='prose on one line'That both edits the answer and opts it into the new wire behavior, contrary to the documented “leading line” rule. The current inline-prose test only covers text before the token (test lines 127-128). Please require horizontal whitespace followed by newline/EOF after
], and pin byte-zero adjacency in the source and vendored-parser checks. -
Blocking — the voice task bridge speaks/logs the control marker. It strips only
[dm-only]when reading a result (task-bridge.tslines 1636-1642), then deliberately narrates foreign-origin results throughonResult(lines 1782-1812). An isolated current-head watcher probe produced:Result task-foreign-thread-probe.txt: [thread]\nanswer body {"spoken":"[thread]\nanswer body"}This contradicts the new “stripped everywhere” contract (marker docs). Please strip the same standalone marker before any voice/log callback and add a behavioral foreign-result regression test.
-
Blocking — the canonical wire contract still does not define this field or its fallback guarantee.
docs/catalog.jsonidentifiesdocs/remote-gateway-protocol.mdas canonical for the wire protocol, butPOST /v1/resultsstill permits only{id, body}, while its retry section says one POST per drain and other 4xx park (lines 281-296). The client only falls back on 400 (provider lines 43-57); a strict current-contract relay returning 422 for the unknown field yields one call andProviderPermanentRefused, parking an answer that the plain payload would accept. The load-bearing “400 records nothing and keeps the lease” rule currently exists only as a client comment, and I could not independently inspect backend PR #2138 because that repository returns 404 to this account. Please update the canonical protocol with the optional field and exact no-side-effect/status contract, then make and test the compatibility path against that contract. Do not broaden the retry to another status unless the same no-side-effect guarantee holds. -
Blocking merge/approval gate — the real post-restart witness is still absent. The PR body already calls this out.
REVIEW.mdexplicitly says a bridge/network/delivery-loop PR is not approvable on harness proof alone. Once backend #2138 is deployed, please run the exact-head gateway after restart and show one real[thread]task answer rooted on its ask, the lease closed, the outbox/result retired, and no duplicate or held claim.
What changes this verdict: fix and test items 1-3, provide the required exact-head live witness, and re-run current CI/CLA. I did not find a problem with the guarded happy path itself.
|
New head Qingyun's nit (proactive bare/rooted order) — fixed at @keweichen 1 — regex truncated leading prose. @keweichen 2 — voice spoke/logged the marker. @keweichen 3 — no canonical wire contract. @keweichen 4 — live witness. Still owed, not run: after backend#2138 deploys, an exact-head gateway restart and one real @liususan091219 nits — a Under |
liususan091219
left a comment
liususan091219
left a comment
There was a problem hiding this comment.
Re-reviewed at ae7f633. I read the delta from 1e013d8 (my last review) and checked each of Kewei's items at this head. I found no blocker. This is not "Ready to approve" yet, because nobody has approved this head.
Kewei's blockers, checked at ae7f633
- Whole-line
[thread]: fixed._THREAD_ASK_REis now^\s*\[thread\][ \t]*(?:\r?\n|\Z)insrc/result_markers.py:205, and the vendored copy is the same (sync_from_src.py --check: in sync). Revert control: I put the old\s*\n?back in both copies.result-markers-threadthen fails 8 checks (4 shapes, src and vendored), andgateway-task-result-thread-askcase i fails twice ('[thread]ing is a library primitive'posts as'ing is a library primitive'withthread: ask). With the fix restored, both pass. - Voice spoke the marker: fixed.
stripVoiceControlLinesis in the existing TS marker module (src/skip_marker_ownership.ts:18), andtask-bridge.tsuses it on both speak paths (:618, :1640). There is no second inline parser: the only other dm-only regex insrc/*.tsis the detectorDM_ONLY_RE(task-bridge.ts:190), and[threadappears nowhere else. Revert control: I disabled the[thread]line skip in the helper.task-bridge-thread-ask-strip.test.tsthen fails 2 of 5 (voice spoke the marker: ["[thread]\nnudge body","[thread]\nanswer body"]). With it restored, 5/5 pass. That test drives the real result watcher, not the source text. - Wire contract: fixed.
docs/remote-gateway-protocol.mdPOST /v1/resultsnow defines the optional field, the 400 no-side-effect rule, and the one retry exception (422 and every other status park as before). The field name and value are defined once,RESULT_THREAD_FIELD, RESULT_THREAD_ASK(provider_ag2space.py:20), and the bridge imports them (remote_gateway_bridge.py:316, used at :4650). There is no"thread"literal left on the wire path. These are Python-only constants. The TS side never sends the field, so there is no TS constant to keep in step. The provider test pins the constants and the status cases (400, then 503 indeterminate with the field kept, 422 refused after one call, 400 twice, 400 without the field). - Live post-restart witness: still owed. The PR body names it. Under Susan's review policy it is named, not blocking.
My earlier nits
"thread"sent with a re-stitched[channel:]: fixed (remote_gateway_bridge.py:4964,redirect is None and …). Case f now asserts it. Revert control: with the guard removed,f) a redirected answer never asks for a threadfails.- No 503 test: added (above).
- A strict 422 bypasses the fallback: now the documented contract, not a gap. It is still true that a broker answering 422 would park the answer. That depends on backend#2138 doing what the doc says.
- backend#2138: still unreadable.
ag2-space/ag2space-backendreturns 404 to this account, which means a missing grant, not a wrong name. I could not check the 400 and redirect behaviour the protocol doc now cites.
Runs at ae7f633 (env -i, except where node was needed)
result-markers-thread, gateway-task-result-thread-ask, ag2space-provider, gateway-proactive-thread, result-markers, result-markers-golden, bridge-marker-no-leak, sparrow-result-guard, claude-md-conduct-rules, agents-md-sync, delivery-primitives-single-owner (OK — 0 failure(s)), events-plane-boundary (14/14), ag2-sparrow-drift, tools/test_no_drift.py and release-docs-audit all pass. dm-only-strip-preserves-prose passes 16/16 with node on the path (6 skip under env -i). task-bridge-thread-ask-strip.test.ts passes 5/5. CLAUDE.md is 40957 bytes. agents-md-sync.sh --check says up to date. review-checks.sh --diff and git diff --check pass. CI: 35 pass, 2 skipped. license/cla: success. There are no unresolved threads.
Nits (not blocking)
- Nothing pins the TS
[thread]strip to the Python grammar the waydm-only-strip-preserves-prosepins[dm-only]. I fed both the same inputs, and they differ in two places.[channel: !r:s] [thread]\nbody: Python reads a redirect plus thread-ask, but voice keeps and speaks the whole line. A lone[thread]\r: TS strips it, Python keeps it. Both are edge cases, but a shared input table run through both engines would stop drift. - The PR body's "Known gaps" still says "The voice task-bridge (TS) does not know
[thread:]or[thread]; unchanged here." That contradicts item 5 and the new test. Please update it.
What would make this "Ready to approve": one maintainer approval at this head. Kewei's CHANGES_REQUESTED at 1e013d8 still stands until Kewei re-reviews.
— Sutando (susans-MacBook-Pro)
keweichen
left a comment
keweichen
left a comment
There was a problem hiding this comment.
Requesting changes at ae7f6333cbf6248709e29439299f85f2f2904e00.
I re-ran the current-head marker/gateway/provider/voice suites, the TypeScript typecheck, vendored-drift checks, git diff --check, and review-checks.sh. GitHub Actions are green (34 passed, 2 expected skips) and the SHA-bound license/cla status passes. The previous byte-zero prose case, canonical wire documentation/constants, redirected-result suppression, and the local watcher/reconcile stripping cases are fixed. The re-review still found current-head blockers:
-
Blocking — relay-mode voice delivery still speaks and logs
[thread].startRelayResultWatcherreads the raw result and sends it unchanged to_sendTaskStatus/onResult; neither new helper call covers theCORE_API_URLsplit-host path. An isolated relay-mode probe returned:RELAY_SPOKEN=["[Task result for task-relay-thread]\n[thread]\nanswer body"]That leaves the prior “strip before every voice/log callback” blocker unresolved. Please route this third voice path through the shared helper and pin the actual relay behavior; the current source-count assertion only requires two call sites.
-
Blocking — the new local strip changes skip semantics and silently retires an answer. The watcher transforms
rawResultat line 1640, then classifies/authorizes a skip using that transformed value at lines 1667-1675. For[thread]\n[no-send]\nvisible, the canonical Python parser emitsthread-askand leaves body[no-send]\nvisiblebecause a skip is terminal only when it is first. The TS path instead strips[thread], promotes[no-send]to the front, and reports a skip. A real watcher probe loggedhas skip marker; archiving silentlywithspoken=[]. Classify ownership/suppression from the raw body; strip controls only for the presentation callback, and add this ordering regression for all three skip forms. -
Blocking — the claimed whole-line grammar is still inconsistent. The fixed
_THREAD_ASK_REis strict in isolation, but the preceding channel/root/reply patterns consume same-line whitespace. Consequently[reply: 12345678901234567] [thread]\nbodyemits bothreplyandthread-askand strips the token; equivalent channel/root/dm-only probes fail in both source and vendored parsers. Separately,neutralize_markersnow rewrites prose the parser correctly preserves:[thread]ing is a library primitive -> [ thread]ing is a library primitive [thread] prose on one line -> [ thread] prose on one lineThat reaches owner-visible paths such as
ask-owner.py. Please make the leading-marker loop and its neutralization inverse share the documented standalone-line boundary, with source/vendored regressions. -
Blocking — the 400 fallback crosses the frozen delivery boundary and can leave a live claim held.
AG2SpaceResultProvider.deliverperforms two POSTs inside one provider call, while the frozensparrow-v1provider contract requires one attempt and no private retries. A400 -> 503probe made two HTTP calls while the outbox recorded one failure. More seriously,400 -> malformed 2xx JSONescapes asJSONDecodeError; the item remainsREADYwith anALIVEprocess'sHELDclaim and the next drain returnsNOT_CLAIMED(even with reclaim TTL 0). Reconcile the endpoint exception with the canonical contract, classify malformed/non-mapping responses from both legs into the provider taxonomy, and test that no provider exit can strand the claim. -
Blocking approval/merge gate — the exact-head restarted live round trip is still absent.
REVIEW.mddoes not permit approving a bridge/network/delivery-loop change on harness evidence alone. After the matching backend is deployed, please show one real[thread]result rooted on its asking message, lease closed, outbox/result retired, no duplicate, and no held claim. The PR currently also reports a merge conflict withmain; any conflict-resolution head needs fresh CI, CLA, and live evidence.
What changes this verdict: fix and regression-test items 1-4, provide the required live witness at the resulting head, resolve the current conflict, and re-run current CI/CLA.
…r on its ask A task result whose leading lines carry a bare `[thread]` is posted through the ordinary result path with `"thread": "ask"` on POST /v1/results. The broker (ag2space-backend#2138) roots a new thread on the task's own asking message; an ask already in a thread stays in it, and a broker without the field ignores it. The marker is parsed by the shared parse_markers (vendored copy synced), stripped everywhere, and read from the guarded body. Should the broker answer 400 for the field, the provider re-posts once without it instead of parking the answer. CLAUDE.md gains the marker and the judgment rule. Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… proactive file _proactive_thread_root returned at the first thread-family action, so a bare [thread] above a valid [thread: $root] posted top level. The rooted form is now checked first; the bare form is only logged when no rooted one decides. Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hread field once - Grammar: a bare [thread] counts only as a line of its own; "[thread]ing ..." and "[thread] text" are prose (source and vendored parser tests). - Voice: stripVoiceControlLines in src/skip_marker_ownership.ts removes a leading standalone [thread] (and the standalone [dm-only], unchanged) before any voice/log callback; task-bridge uses it on both speak paths. - Wire contract: RESULT_THREAD_FIELD/RESULT_THREAD_ASK in provider_ag2space.py are the one client owner; docs/remote-gateway-protocol.md defines the field, the broker's 400 no-side-effect contract, old-broker compatibility and the single 400 re-post (422/503 tested to keep today's handling). - A [channel:] redirected answer no longer sends the field. Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e strips it for speech only - The provider makes a single attempt (sparrow-v1 row 5): a 400 for the thread field parks like any other 4xx. Only "ask" is sent and the broker defines it as valid, so a 400 is a bug to investigate. - parse_markers counts a bare [thread] only at a real line start: not after another marker on the same line, and not where a [dm-only] strip glued it. neutralize_markers quotes only the standalone form. - Voice: the relay watcher strips through stripVoiceControlLines too, and skip and ownership decisions read the raw body, so [thread] above a skip marker is an answer, as in parse_markers. Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ae7f633 to
d8de155
Compare
|
New head 1. Relay mode spoke and logged Now it speaks 2. Strip-before-skip retired answers. The local watcher decides Now all three are narrated, matching 3. Whole-line grammar and the neutralizer. Now all pass. A standalone 4. The 400 fallback. Removed. The coordinator chose to keep the frozen sparrow-v1 single-attempt rule rather than amend it. The provider makes one POST, and a 400 for the field parks like any other 4xx, as on main. Now there is one POST, the result is quarantined to 5. Live witness. Still owed, and not run: one exact-head, post-restart Tests at |
liususan091219
left a comment
liususan091219
left a comment
There was a problem hiding this comment.
Re-reviewed at d8de155 (rebased onto main b485282). I found no blocker. This is not "Ready to approve" yet: nobody has approved this head, and Kewei's CHANGES_REQUESTED at ae7f633 still stands until Kewei re-reviews.
What changed since ae7f633. git range-diff shows the first three commits carried over unchanged, apart from the task-bridge.ts import line and one context line from #5310. All the new behaviour is in d8de155. The provider (provider_ag2space.py) is now byte-identical to main apart from the two RESULT_THREAD_* constants.
Kewei's items at ae7f633, checked at d8de155
- Relay mode spoke
[thread]: fixed.startRelayResultWatcherdecides onrawand hands onlystripVoiceControlLines(raw)to status andonResult(src/task-bridge.ts:1472-1484). Revert control: I setconst result = raw;at :1474.task-bridge-relay-thread-ask.test.tsthen fails 1/1. With the line restored, it passes. - Strip-before-skip retired answers: fixed. The local watcher runs
isSkipMarked,mayRetireSkipMarkedanddedupTargetonraw(:1663, :1691-1700). Revert control: I putresultback into the two skip calls.task-bridge-thread-ask-strip.test.tsthen fails 1 of 6, loggingtask-thread-then-{deduped,noSend,replied} has skip marker; archiving silentlyandnot all spoken: ["nudge body","answer body"]. With them restored, 7/7 pass across both files. - Grammar and neutralizer: fixed for the shapes Kewei listed.
[reply: …] [thread],[channel: …] [thread]and[dm-only] [thread]now emit no thread-ask (src/result_markers.py:325).neutralize_markersno longer rewrites[thread]ing …or[thread] prose(:636). Vendored copy: byte-identical (delivery-primitives-single-owner). - The 400 fallback: removed. There is one POST, and a 400 parks like any other 4xx. The malformed-2xx stranding is the same on main (same
self._request(...) or {}line, no JSON guard), so it is reasonable to track it separately as #5329. - Live post-restart witness: still owed, and the PR body names it. Under Susan's review policy this is named, not blocking.
The ordering question: what happens before backend#2138 deploys
With the fallback gone, a [thread] answer reaches today's production broker as {"id","body","thread":"ask"} once.
- If that broker ignores the unknown key, nothing breaks: the answer posts as a plain reply.
- If it answers 400 or 422, the answer is parked in
undelivered/and never re-posted. The person who asked gets no answer, and the broker's lease stays open.
Main never sends the field. There, the same result posts with a visible [thread] line, but the answer arrives. So under a strict broker this is a regression, and only for results an agent marks [thread]. Unmarked results are byte-identical to today (case b). The agent can avoid it by not writing the marker. But the CLAUDE.md bullet this PR adds (CLAUDE.md:350) tells every agent it may use the marker, and gives no "after the backend deploys" condition.
- Evidence it is safe: the PR body ("safe to merge first") and
docs/remote-gateway-protocol.md:139both say a broker that predates the field ignores it. The only precedent I found is #3611 (2026-08-31). It said that before backend#882,/v1/results"silently drops the unknownmetadatakey and returnsok: true". That is consistent, but it is six weeks old and about a different key. - I could not check the broker today:
ag2-space/ag2space-backendreturns 404 to this account (orgs/ag2-spacereturns 200). That means a missing grant, not a wrong name. So I cannot confirm that it still accepts unknown keys, or what backend#2138 does. - The body points two ways. The top line says "safe to merge first". "Known gaps" says the round trip, which can only happen after the deploy, is "required before merge-ready".
- Ask (not blocking): either paste one current request to the production
/v1/resultswith an unknown key accepted (2xx), or state plainly that this merges after backend#2138 deploys.
Earlier TS/Python disagreements
[channel: !r:s] [thread]\nbody: resolved. Both engines now treat the same-line[thread]as text. Python redirects with the body[thread]\nbodyand no thread-ask. Voice keeps the line.- A lone
[thread]\r: still different. Python emits no action and posts[thread]. Voice strips it (THREAD_ASK_LINE_REallows\r?$,src/skip_marker_ownership.ts:14). - New, same kind: an indented
[thread]([thread]\nbody,\t[thread]\nbody, or[thread]under a[channel:]line). Python emits no thread-ask and posts the literal[thread]line, becauseatis the[offset and its predecessor is a blank, not\n(result_markers.py:325). Voice strips it (skip_marker_ownership.ts:14allows leading[ \t]*).neutralize_markersalso quotes it (_THREAD_ASK_OPEN_REallows leading[ \t]*, :636), so the neutralizer treats a shape as a marker that the parser treats as prose. At ae7f633,\s*accepted indentation, so this changed in this round. All of these are edge cases. One shared input table run throughparse_markersandstripVoiceControlLineswould stop this drift. Today only a source-count pin exists (dm-only-strip-preserves-prose:count("stripVoiceControlLines(") >= 3), next to the real-watcher tests.
Runs at d8de155
All of these pass: result-markers-thread, gateway-task-result-thread-ask, ag2space-provider, gateway-proactive-thread, result-markers (48), result-markers-golden (13), result-markers-reply, bridge-marker-no-leak, sparrow-result-guard, sparrow-integration-e2e, outbox-receipt-destination, claude-md-conduct-rules, agents-md-sync, delivery-primitives-single-owner (OK, 0 failures), events-plane-boundary (14/14), ag2-sparrow-drift, tools/test_no_drift.py, release-docs-audit, dm-only-strip-preserves-prose (16/16).
TS: task-bridge-relay-thread-ask 1/1 and task-bridge-thread-ask-strip 6/6. tsc --noEmit is clean.
CLAUDE.md is 40957 bytes. agents-md-sync.sh --check says up to date. review-checks.sh --diff and git diff --check pass.
CI: 35 pass, 2 skipped. license/cla: success (1 context). There are no unresolved threads. GitHub reads the PR as MERGEABLE.
Nits (not blocking)
- PR body "Known gaps" still says "The voice task-bridge (TS) does not know
[thread:]or[thread]; unchanged here." That is now false. Item 5 also says "both speak paths", but there are three (local, reconcile, relay). - A result whose whole body is
[thread]strips to empty, and the local watcher thencontinues on it every poll without archiving it (task-bridge.ts:1664-1665). This is degenerate, but the file stays in place.
What would make this "Ready to approve": one maintainer approval at this head, and Kewei's re-review. The live witness stays owed and named.
— Sutando (susans-MacBook-Pro)
keweichen
left a comment
keweichen
left a comment
There was a problem hiding this comment.
Requesting changes at d8de155c78b6dcba0be545d772410a9bd10f850c.
I re-ran the current-head parser/gateway/provider/voice suites, TypeScript typecheck, vendored-sync checks, git diff --check, and review-checks.sh. GitHub reports 34 successful checks, two expected skips, no incomplete checks, and a passing SHA-bound license/cla status. The prior relay stripping, raw-body skip/ownership ordering, same-line/prose handling, and private 400-retry blockers are fixed; #5329 is a valid follow-up for the unchanged malformed-2xx behavior. Two blockers remain:
-
Blocking — the new line-start guard rejects indented standalone
[thread]markers and makes the Python/gateway and voice paths disagree. The regex intentionally accepts leading horizontal whitespace, butparse_markersthen requires the character immediately before[to be\n. That rejects any spaces or tabs preceding the marker:ae7f6333: '[reply: 12345678901234567]\n [thread]\nbody' -> reply + thread-ask, body='body' d8de155c: same input -> reply only, body='[thread]\nbody'The same d8 false negative occurs at byte zero and after
[channel:],[thread: $root], and[dm-only]. The gateway harness consequently posts the literal[thread]\nbodywithoutthread: ask. MeanwhilestripVoiceControlLinesstrips those forms, andneutralize_markersrewrites them as markers. This violates both the “stripped everywhere” behavior and the parser/neutralizer/voice grammar parity. Please recognize horizontal indentation from the physical line start while retaining the same-line/glued rejection, and add source, vendored, gateway, and TS parity cases after each leading-marker form. -
Blocking approval/merge gate — the exact-head post-restart live witness is still absent. This changes a bridge/network delivery loop, so
REVIEW.mddoes not permit approval on harness evidence alone. The PR and follow-up both acknowledge that this is still owed after the matching backend deploy. Please show a real[thread]task result rooted on its ask, the lease closed, result/outbox retired, and no duplicate or held claim. Without that witness, a strict pre-deploy broker could reject the new field, park the marked answer, leave the lease open, and return no answer to the asker.
What changes this verdict: fix and regression-test the indentation/parity case, provide the required live witness at the resulting exact head, and re-run current CI/CLA. The stale PR-body statement that the voice task bridge does not know [thread] should also be removed, but that is not independently blocking.
…ted [thread] counts - parse_markers treats [no-send]/[REPLIED]/[deduped:] directly after the leading markers ([thread], [thread: $root], [reply:], standalone [dm-only]) as a skip, as the broker does on the body it is sent; the gateway archives it through its skip path with the reason logged. After [channel:] it stays text, so the team guard still reviews redirect-plus-skip. - Voice: bodyIsSkipMarked mirrors that verdict on the original leading block. - A bare [thread] indented from its physical line start counts again; same-line and glued forms stay prose. - One table, tests/fixtures/thread-ask-cases.json, drives the Python parser, the vendored copy, neutralize_markers and the TS strip/skip helpers. Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
New head Skip directly after the leading markers is now a skip everywhere (Qingyun, re-review at One deliberate exception: after a Failing beforehand: 1. Indentation parity (Kewei). A bare One shared table, Nit: the PR body no longer claims the voice task bridge doesn't know 2. Live witness: still owed. It needs to run at the exact head after backend #2138 deploys, and I have not run it. Tests at |
liususan091219
left a comment
liususan091219
left a comment
There was a problem hiding this comment.
Re-reviewed at 69b5d9e (base main b485282). I found no blocker. This is not "Ready to approve" yet: nobody has approved this head, and Kewei's CHANGES_REQUESTED at d8de155 stands until Kewei re-reviews.
Kewei's items at d8de155, checked at 69b5d9e
- Indented standalone
[thread]: fixed.parse_markersnow measures the line start past spaces and tabs (src/result_markers.py:326-327), and the glued and same-line forms still stay prose. I ran the revert control: I setstart = atandresult-markers-threadfailed 8 checks, among themtable src: ' [thread]\nbody' thread-ask=True — [] '[thread]\nbody'and the same after[channel:],[thread: $r],[dm-only]and[reply:]. With the line restored, it passes. The vendored copy is byte-identical. - Live post-restart witness: still owed, and the author says so (comment 6103261883). Under Susan's review policy this is named, not blocking. It is still Kewei's stated merge gate.
- Stale "voice task bridge does not know
[thread]" line: removed from the body.
The shared table (tests/fixtures/thread-ask-cases.json, 26 rows)
All four consumers read it from the file. None has a copy. result-markers-thread runs it through the src parser, the vendored parser and both neutralize_markers. task-bridge-thread-ask-strip.test.ts runs it through stripVoiceControlLines and bodyIsSkipMarked. Each side checks only its own columns: thread_ask and neutralize are checked in Python, voice in TS, and skip in both.
Revert controls:
- TS: I made
bodyIsSkipMarkedtest only the body start (src/skip_marker_ownership.ts:80). Three tests then failed, including the table test, with 6 skip rows listed. The live-Python parity test and the real-watcher test also failed. Restored, 9/9 pass. - Python: see item 1. The table rows are among the 8 failures.
Skip after the leading markers (Qingyun's finding, as the author's comment reports it; I found no Qingyun post on GitHub)
- Team tier. I looked for a new way to suppress delivery or get around the guard, and found none. I compared main's and this head's
parse_markerson 4,599 generated bodies plus 36 adversarial ones. Every body that newly reads as suppression-only (is_suppression_only,src/policy/egress/result.py:232) has a leading[thread],[thread: $r],[reply:]or standalone[dm-only]followed by a skip. Main already lets a team result do the same thing by putting[no-send]first. Either way the body is journaled as a silent close and nothing moves. - Redirects. No body lost suppression. Any body with
[channel:]in front of the skip keeps its redirect action, so the guard still withholds it ([thread]\n[channel: !r:s]\n[no-send]…givesthread-ask, redirect, attach). That holds in either order with[dm-only],[reply:]and[thread:]. - One shift worth knowing: on main, a team result
[thread: $r]\n[no-send]\n[file: /etc/passwd]was withheld for owner review as an unconfined attach. At this head it is a journaled silent close with no upload. That matches main's leading[no-send]\n[file: …]. - Owner tier. This changes behaviour. Main posts
[reply: …]\n[no-send]\nx,[thread: $r]\n[no-send]\nxand[dm-only]\n[REPLIED]\nxwith the literal marker visible. At this head they are archived silently. This is intended and documented (docs/claude-md-moved-detail.md, the[thread:bullet), and it drops the old line saying a skip after[thread:]"is not seen and the body is posted". - Producers that write a leading marker. I checked the four that write one in front of a body. Three are in voice (
task-bridge.ts:386,:791,:796); they only forward results that already passed the skip check. The fourth ismorning-briefing.py:891. In normal use, none writes a skip on the next line. - Parity. On the 4,599 bodies, TS
bodyIsSkipMarkedand Python agree on skip, except lowercase[replied]. TS treats it as a skip (its regex is/i) and Python does not. This is the same on main, and it errs toward silence in voice.
My earlier open items
- (a) Deploy ordering: not answered. There is still no evidence that today's broker accepts an unknown key, and I still cannot read
ag2-space/ag2space-backend(404, withorgs/ag2-spacereturning 200: a missing grant). The body still says "safe to merge first" on line 1, and in "Known gaps" that the round trip is "required before merge-ready". The two only fit together if this merges after backend#2138 deploys, and in practice the witness gate forces that order. Ask (not blocking): say it in one line at the top of the body. - (b) TS/Python edge disagreements: the indented
[thread]disagreement is fixed. What remains are edge cases:- A lone
[thread]\rat the end of a body. Python reads it as text and voice strips it. It is not in the table. - New, from the corpus: two markers on one line followed by a
[thread]line, e.g.[channel: !r:s] [thread: $r]\n[thread]\nbody. Python asks for a thread and voice keeps, and speaks, the[thread]line. - Both are malformed and harmless. The table replaces the call-site-count pin I complained about, so I count (b) as resolved apart from these. Adding the two rows would close them.
- A lone
- (c) PR body: the "does not know
[thread]" line is gone. The "What changed" item 5 still says "both speak paths", and there are three (local, reconcile, relay). The Round 3 section's "[thread]\n<skip>\nvisibleis an answer" is now reversed by Round 4. It is labelled as history, but a reader could take it as current. Nits. - (d) A body that is only
[thread]still strips to empty, and the local watchercontinues on it without archiving (src/task-bridge.ts:1664-1665). This is unchanged and degenerate. Not blocking.
Runs at 69b5d9e (perl alarm 300 each)
All of these pass: result-markers-thread, gateway-task-result-thread-ask, ag2space-provider, gateway-proactive-thread, result-markers, result-markers-golden, result-markers-reply, bridge-marker-no-leak, sparrow-result-guard, sparrow-integration-e2e, outbox-receipt-destination, team-result-guard, team-guard-suppress-journaled-silent-close, claude-md-conduct-rules, agents-md-sync (5), delivery-primitives-single-owner (OK, 0 failures), events-plane-boundary (14/14), ag2-sparrow-drift, dm-only-strip-preserves-prose, release-docs-audit.
TS: task-bridge-thread-ask-strip and task-bridge-relay-thread-ask, 9/9. tsc --noEmit is clean.
CLAUDE.md is 40957 bytes. AGENTS.md is in sync. review-checks.sh passes and git diff --check is clean. SUTANDO_WORKSPACE appears only in tests, pointed at temp dirs.
CI: 35 pass, 2 skipped. license/cla: success. There are no unresolved threads. GitHub reads the PR as MERGEABLE.
What would make this "Ready to approve": one maintainer approval at this head, and Kewei's re-review. The live witness stays owed and named.
— Sutando (susans-MacBook-Pro)
qingyun-wu
left a comment
qingyun-wu
left a comment
There was a problem hiding this comment.
Review at 69b5d9e4. The code is ready to approve; the one thing still owed is the live post-restart witness (REVIEW.md lesson 15), which @keweichen's open changes request already holds. I'll approve as soon as it's posted, so I'm not adding a second block.
Witness, now runnable: backend #2138 merged to backend dev at 2026-10-10T23:50Z (0bb173d2), after every review here. Run the exact head against the dev broker after a restart and show one [thread] task answer rooted on its ask, the lease closed, the result and outbox item retired, and no duplicate or held claim.
Server contract, checked in backend source:
- dev (
services/agent-message-broker/api/v1.py:759-764):threadis validated before any side effect; absent /null/false/"ask"pass, anything else is a 400 beforeadd_result, nothing recorded, lease kept. Matchesdocs/remote-gateway-protocol.md:123-148. - main (
v1.py:712-761):post_resultsreads onlyid,status,metadata,body,no_send,consolidated_into, so an unknownthreadis ignored, not a 400. "Safe to merge first" holds. - The gateway can only send
"ask"or nothing: the value is defined once (provider_ag2space.py:21), added only in_deliver_result_payloadwhenthread_ask=True, and only the ordinary result branch sets it (redirect is None and any(thread-ask)). The earlier 400 re-post is gone.
One policy owner: marker grammar is in src/result_markers.py, the vendored copy is byte-identical (ag2-sparrow-drift and test_no_drift.py pass), and task-bridge.ts uses the shared helpers. skip_marker_ownership.ts afterLeadingMarkers is a ~30-line TS copy of the leading-marker loop; TS can't import the Python module, and the shared fixture (26 rows) plus the parity test against live parse_markers mitigate it, so I count it as acceptable, but it is a second grammar to keep in sync.
Kewei's earlier points: prose truncation, voice speaking the marker on all three paths, the wire contract docs, the relay path, deciding skip on the raw body, same-line/neutralize grammar, the 400 retry and indented [thread] are all fixed at this head. Only the witness is open.
Nits (non-blocking):
- "A skip right after the leading markers is a skip" (
result_markers.py:339-345) changes behaviour for every Python consumer (Discord, Telegram and Slack too):[reply: id]\n[no-send]\nxused to post and now archives silently. Arguably right, but say it plainly in the PR body. - Two malformed cases still differ between Python and voice:
'[thread]\r'at the end of a body (Python leaves it, voice strips it) and[channel: !r:s] [thread: $r]\n[thread]\nbody(Python threads, voice speaks the line). Harmless; adding both to the shared table would close them. - PR body: item 5 says "both speak paths" (there are three), and the Round 3 history about
[thread]\n<skip>is now reversed. - A body that is only
[thread]strips to empty, and the local voice watchercontinues without archiving (task-bridge.ts~1664). Pre-existing and degenerate.
Ran at this head (env -i): review-checks.sh --diff (pass); Python suites result-markers-thread, gateway-task-result-thread-ask, ag2space-provider, gateway-proactive-thread, result-markers, result-markers-golden, result-markers-reply, bridge-marker-no-leak, team-result-guard, team-guard-suppress-journaled-silent-close, dm-only-strip-preserves-prose, claude-md-conduct-rules, agents-md-sync, ag2-sparrow-drift, delivery-primitives-single-owner, test_no_drift.py (all pass); the new thread-ask TS tests pass (one unrelated canary failed only because of a stale local dependency; CI is green, 35 pass).
Not verified: any live run (the blocker); which backend environment #2138 is deployed to.
john-the-dev
left a comment
john-the-dev
left a comment
There was a problem hiding this comment.
Review at 4a5014d93. This is 69b5d9e4d plus main, and the PR diff against its merge-base is byte-identical. Commenting rather than approving.
Kewei 1, indented [thread]: fixed (result_markers.py:326-327).
'[reply: …]\n [thread]\nbody'now givesreply, thread-askwith bodybody. The same holds at byte zero, with a tab, and after[channel:],[thread: $r]and[dm-only].- Same-line and glued forms stay prose, and the vendored copy is byte-identical.
- Mutating
start = atmakesresult-markers-threadfail 7 checks (src) andgateway-task-result-thread-askfail case k (vendored). Removing the indentation allowance from TSTHREAD_ASK_LINE_REfails the shared-table test. All green again once restored. - The owner's Codex verifier independently found the same.
Kewei 2, live witness: still absent. Body line 183 and comment 6103261883 both say it is owed and has not been run. This PR changes the result delivery loop, so REVIEW.md lesson 15 does not allow approval on harness evidence alone. Please post one real round trip after a restart, at the exact head, against a broker running backend#2138:
- a
[thread]answer that appears in the thread rooted on its ask - the lease closed
- result and outbox retired
- no duplicate or held claim
Nits:
- I reproduced two TS/Python divergences:
'[thread]\r'and'[channel: …] [thread: $r]\n[thread]'. Both are malformed input and harmless, but they belong in the shared table. - Say plainly in the body that
[reply:]/[thread]plus a skip is now silent on every Python bridge.
Ran: 12 Python suites and 2 TS suites (9/9), tsc --noEmit and review-checks.sh, all passing. CI at review time: 32 pass, with "diff coverage >= 95% (python)" and "shell tests (lanes)" still pending.
qingyun-wu
left a comment
qingyun-wu
left a comment
There was a problem hiding this comment.
Approving at 4a5014d on Qingyun's call. Since my review at 69b5d9e the only change is a merge of main (parents 69b5d9e + 0985ea4, which is main); the PR's own files are unchanged. Per my review: the code is ready, the server contract matches backend #2138, and merging first is safe because main's broker ignores the field. The live post-restart witness is still owed under @keweichen's open request; this approval does not replace it.
liususan091219
left a comment
liususan091219
left a comment
There was a problem hiding this comment.
Approved on Susan's instruction ("approve 5326" in her DM) at 4a5014d, a merge of main into 69b5d9e, where my review had no blockers. CI 34 pass, 2 skipped; license/cla success. The live post-restart witness is still owed after ag2space-backend#2138 reaches prod, so this should merge after that deploy.
— Sutando (susans-MacBook-Pro)
|
@cla-assistant check |
|
Live post-restart witness at 4a5014d, prod broker, owner Go (2026-10-10, times PDT) Setup: a detached worktree at the PR head, its The round trip. The owner sent "merge 5326 when ready" as a new top-level message in the Pro-dev DM. The worker wrote a result whose first line was a bare Sanitized envelope (ids masked): Result file as written (archived as Gateway log (witness gateway, UTC as logged; 19:09 PDT): Those five lines are every log line for this task id. The 90 s that followed held no retry, no "not confirmed" and no second POST. What was POSTed. The outbox item After 90 s of passes (19:11:11):
Read-back from the gateway's Exactly one event carries the answer body. Its body has no The witness gateway also carried three unrelated tasks during the window (task-f0100799…, task-09d50bc6…, task-f46d96ec…). Each went from queued to Restore. Polling gaps, as measured. The owner's Go asked for under a minute each way; neither direction made it.
Both are the stale poller lock: the previous holder's lock has to age out before the next gateway reaps it. No task was lost: the broker held tasks until a gateway polled, and two waiting tasks were queued at 19:12:52. The return leg needed launchctl kickstarts because the first bootstrap's process exited 0 on the stale lock. Nothing was merged. |
john-the-dev
left a comment
john-the-dev
left a comment
There was a problem hiding this comment.
Approving at 180f2efe. My earlier review (5481282676, at 4a5014d93) asked for a live witness after a restart. That witness is now posted: #5326 (comment).
What the witness shows. A real gateway was restarted at 4a5014d93, running against the prod broker with backend#2142 deployed.
- A bare
[thread]answer to a top-level owner ask was posted with"thread": "ask". The stored payload confirms it, and the[thread]line was stripped from the body. - The answer landed once, in the thread on that ask. Broker read-back shows
thread_rootequal to the ask's event. - The outbox item was DELIVERED and retired, and the task was archived once.
- For 90 s afterwards there was no retry, no duplicate and no held claim.
Why it applies to 180f2efe. This head is 4a5014d93 plus a main merge of #5325 only.
- The PR's own diff has the same
git patch-id --stable(975064549ede) at both commits. - Rui's Codex verifier compared the two trees. They differ only in
skills/agent-room-ops/doc_sync.pyand its test, so the gateway, provider, marker and delivery paths are byte-identical. - At this head, the thread-marker, gateway, provider and proactive-thread suites pass, along with the 26 shared TS fixture rows.
- CI: 34 checks succeeded and 2 were skipped.
What this does not cover. I did not run the restart myself, and nothing was restarted at the merge commit. The witness also discloses 87–95 s gaps while the restart and restore polled stale singleton locks.
Dismissed on the owner's explicit word (Chi, 2026-10-10 5:1x PM and 'merge 5326' at 7:2x PM PDT). Every item in this review is fixed and verified at the head, reproduced first. Kewei was asked to re-review at 00:01Z/5:01 PM, 7:13 PM and after the live witness, with no review since. Qingyun (code owner) and Susan approved at head, and the live post-restart witness passed (comment 6104518754).
Follow-up to #5305 (merged) and #5323 (closed). Depends on ag2-space/ag2space-backend#2138 for effect; safe to merge first: a broker without that change ignores the new field, so until it deploys a
[thread]answer is posted exactly as today (marker stripped, plain reply).What changed, and why?
#5323 tried to thread a task answer from the gateway (hand the answer to the proactive leg, close the lease
no_send). Its reviews showed why that cannot be done on the client: the answer left the outbox, a refused thread post lost it, an unknown outcome duplicated it, the sentinel was not generation-bound, unreadable provenance was guessed, and the root was not proven to belong to the task. backend#2138 moves placement to the server:POST /v1/results {"id", "body", "thread": "ask"}roots a thread on the task's own asking message, keeps an in-thread ask in its thread, falls back to a plain reply where threading is impossible, and answers 400 (nothing recorded, lease kept) for any other value.This PR is the gateway side, and only that:
src/result_markers.py, vendored copy regenerated bysync_from_src.py). A leading line that is exactly[thread](trailing blanks allowed;[thread]ing …or[thread] textis prose and untouched), in any order with[channel:]/[thread: $root]/[reply:], emits athread-askaction and is stripped. It is distinct from feat(markers): [thread: $root] posts a proactive result inside a thread #5305's[thread: $root](different regex, different action), carries no id, and is repeated-safe. An inline[thread]in prose is left alone; skip markers stay terminal;neutralize_markersknows the new word.remote_gateway_bridge.py; name and value areRESULT_THREAD_FIELD/RESULT_THREAD_ASKinprovider_ag2space.py, the one client owner, anddocs/remote-gateway-protocol.mdPOST /v1/resultsis the canonical contract, citing backend#2138). The ordinary, non-redirected task-result branch of_post_ready_resultspassesthread_askto_deliver_result_payload, which adds"thread": "ask"to the same doc. Nothing else moves: the marker is read from the guarded body (the sameparsedthe skip/redirect/attach decisions use), attribution runs before the publish, and the outbox/lease/idempotency path is unchanged. No proactive hand-off, no lease close before delivery. Skip, dedup, owner-mention and orphan-recovery paths never send the field (orphan recovery strips the marker and posts its labelled body top level, as today).delivery_core/provider_ag2space.py). Only"ask"is sent and backend#2138 defines it as valid, so a 400 for the field is a bug to investigate: the provider makes its single attempt (sparrow-v1 row 5) and a 400 parks like any other 4xx, as on main. (An earlier head re-posted without the field; removed in round 3, see below.)[thread]there is stripped, logged and posted top level; a rooted[thread: $root]on the same file wins in either order. Valid only on a task result.src/skip_marker_ownership.tsstripVoiceControlLines, used by both speak paths intask-bridge.ts): a leading standalone[thread]is removed before any voice/log callback, next to the existing standalone[dm-only]strip (same expression, moved into the helper).docs/claude-md-moved-detail.md(40957 bytes; AGENTS.md regenerated). The detail doc has the full[thread]semantics.tests/claude-md-conduct-rules.test.pypins the new bullet.[channel:]and attachments keep working exactly as today: the redirect is re-stitched as before and a redirected answer does not send the field; uploads run before the POST as before.Review first
remote_gateway_bridge.py:thread_askin_post_ready_resultsand thedoc["thread"]line in_deliver_result_payload.provider_ag2space.py: theRESULT_THREAD_FIELD/RESULT_THREAD_ASKconstants (the one client owner of the field).src/result_markers.py:_THREAD_ASK_REand the leading-marker loop.What behavior does this prove?
A task result
[thread]\nall greenis posted as{"id": …, "body": "all green", "thread": "ask"}through the ordinary outbox path; without the marker the payload is byte-identical to today; a guarded result that is withheld never sends the field; a 400 for the field parks like any other 4xx.Feature/documentation consistency
Updated: CLAUDE.md marker list (+ AGENTS.md),
docs/claude-md-moved-detail.md"Result-marker semantics" (canonical).docs/catalog.jsonlast_verifiedfor that doc is already 2026-10-10.docs/README.mdunchanged: navigation did not change.Before / after
All runs under
env -i HOME=$HOME PATH=/usr/bin:/bin:/opt/homebrew/bin. Tests are production-wrapper level:tests/gateway-task-result-thread-ask.test.pyloadssrc/remote-gateway-bridge.pyin-process (its realPROACTIVE_CLAIM_GATEis installed and asserted), with an isolated workspace and a fake_req, and drives the real_post_ready_results→ delivery core →AG2SpaceResultProvider.python3 tests/gateway-task-result-thread-ask.test.pyat the parentd6f7abbd4(test copied into a detached worktree), rc=1:At head
a24fe8f5c, rc=0:python3 tests/result-markers-thread.test.pyatd6f7abbd4, rc=1:At
a24fe8f5c, rc=0:python3 tests/claude-md-conduct-rules.test.py(head test, main's CLAUDE.md):AssertionError: '-[thread]— task-result lead line: …' not found×2,Ran 5 tests … FAILED (failures=2). At head:Ran 5 tests … OK.Mutation checks (each restored afterwards)
parsed = parse_markers(body)→parse_markers(raw)in_post_ready_results:doc["thread"] = "ask"removed: a, d, e, g fail (6 failures).Tests run at the first head (
a24fe8f5c, sameenv -i)Gate files run explicitly:
delivery-primitives-single-owner(OK — 0 failure(s)),events-plane-boundary(14/14 passed),agents-md-sync(5 passed;agents-md-sync.sh --checkup to date),claude-md-conduct-rules(OK),pending-questions-review-r35(size cap, OK; CLAUDE.md 40957 bytes).80 files in total, 79 rc=0: every
tests/gateway-*,tests/remote-gateway-*, every result-marker suite (result-markers,-golden,-reply,-thread,bridge-marker-no-leak,result-file-marker-guard,pathless-file-marker-controls,discord-bridge-prose-marker,dm-result-skip-markers,telegram-bridge-send-reply-markers,proactive-suppression-marker-*,health-check-*-skip-markers,startup-media-marker),ag2space-provider,outbox-receipt-destination,sparrow-result-guard,sparrow-integration-e2e,ag2-sparrow-drift,packages/ag2-sparrow/tools/test_no_drift.py,release-docs-audit. The one non-zero istests/remote-gateway-bridge.test.py:FAIL env-fallback: no env token and no file yields empty token (no crash), which fails identically atd6f7abbd4on this host (a host token is visible to the test), so it is not from this PR.bash scripts/review-checks.sh --diff <(git diff origin/main...HEAD): PASS (hardcoded-paths + root-artifacts + prose-cap clean).git diff --check origin/main...HEAD: clean.CI at head
a24fe8f5c: 34 checks pass, 2 skipped, includingdiff coverage >= 95% (python),refuse AGENTS.md stale vs CLAUDE.mdandlicense/cla.Round 2 (head
ae7f6333c): Qingyun's nit and Kewei's reviewTests written first; each failed at
1e013d8d0(env -i):At
ae7f6333call pass. New provider cases pin the contract: thread+400 re-posts once without the field; thread+503 stays indeterminate with the field kept (one call); thread+422 is refused after one call; 400 twice is refused after two calls; 400 without the field is one call.Full run at
ae7f6333cunderenv -i: 90 Python files, 89 rc=0 (the same host-onlyremote-gateway-bridgeenv-fallback case that fails on main);delivery-primitives-single-ownerOK — 0 failure(s),events-plane-boundary14/14 passed; 27 TS files that import task-bridge or the marker module: 333/333 pass.review-checks.sh --diffPASS,git diff --checkclean,sync_from_src.py --checkin sync, AGENTS.md up to date, CLAUDE.md 40957 bytes. CI atae7f6333c: 35 pass, 2 skipped, including diff coverage andlicense/cla.Round 3 (head
d8de155c7, rebased onto mainb48528280): Kewei's review atae7f6333c[thread]throughstripVoiceControlLinesbeforeonResult/status; new real-relay-watcher testtests/task-bridge-relay-thread-ask.test.ts.[thread]\n<skip>\nvisibleis an answer for all three skip forms (real-watcher test),[no-send]\n[thread]stays a skip.[thread]counts only at a real line start (not after another marker on its line, not where a[dm-only]strip glued it);neutralize_markersquotes only the standalone form. 14 src+vendored regressions.Before/after outputs: the round-3 PR comment. At
d8de155c7underenv -i: 96 Python files, 95 rc=0 (same host-only env-fallback case as main); both gates green; 29 TS files 355/355.Round 4 (head
69b5d9e4d): Qingyun's skip-parity finding and Kewei's review atd8de155c7parse_markers(one owner), voice (bodyIsSkipMarkedmirrors it) and the broker alike:[thread]/[thread: $root]/[reply:]/standalone[dm-only]followed by[no-send]/[REPLIED]/[deduped:]. The gateway archives it through its skip path (lease closedno_send, logged(marker …, lease closed, not sent);[deduped:]goes through the dedup plan). Not after[channel:]: there it stays text so the team guard still withholds redirect-plus-skip for owner review (team-guard-suppress-journaled-silent-closee).[thread]indented from its physical line start counts again (byte zero and after every leading form); same-line and glued forms stay prose.tests/fixtures/thread-ask-cases.json, run againstparse_markers, the vendored copy,neutralize_markers(Python) andstripVoiceControlLines+bodyIsSkipMarked(TS).Edge cases checked
Withheld team result (no field, no text on the wire); refused attribution (no POST at all, quarantined); attribution plus the field together;
[file:]upload kept;[channel:]re-stitch unchanged; 400 parks; proactive bare[thread]top level; inline[thread]prose untouched; skip before[thread]stays terminal.Risks / rollback
No migration or config. Before the backend deploys, the only visible change is that a bare
[thread]line is stripped instead of printed. Rollback is a revert.Known gaps / follow-up
[thread]→ gateway restart from this head → answer appears in a thread on the ask, lease closed). This PR changes the result delivery loop, so that round trip is required before merge-ready; it cannot be meaningful until the broker honours the field.[thread]before speech/logs and mirrorsparse_markers' skip verdict (src/skip_marker_ownership.ts); a rooted[thread: $root]is kept, as forwarded voice results need it.🤖 Generated with Claude Code