Repository navigation
Fix apostrophe breaking web client script - #20
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 of 4 tasks
sonichi
added a commit
that referenced
this pull request
Apr 20, 2026
-23) (#488) 9 open CodeQL path-injection alerts across 3 sites in src/agent-api.py. All three had strong existing sanitization (regex whitelist + is_relative_to confinement), but CodeQL doesn't recognize is_relative_to as a taint sanitizer. Apply the Path.name pattern matching PR #487's dashboard.py fix: 1. _safe_path(base_dir, filename) — the task/result lookup helper (alerts #16-19). Rebuild filename via Path(name + ".txt").name. 2. /media/{rel} handler (alerts #7, #20-21). Multi-segment; split on "/" and rebuild each component via Path(p).name, then joinpath. 3. Inline task-file build for "answer-{qid}-{ts}.txt" (alerts #22-23). Apply Path(...).name to the full basename. Path.name is functionally a no-op after the regex whitelist (no separators possible), but it's a CodeQL-recognized path-injection sanitizer that breaks the taint flow CodeQL tracks through f-string path construction. Same pattern Mini validated on #487. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
liususan091219
pushed a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
…ichi#20) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
liususan091219
pushed a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
…chi#7, sonichi#16-23) (sonichi#488) 9 open CodeQL path-injection alerts across 3 sites in src/agent-api.py. All three had strong existing sanitization (regex whitelist + is_relative_to confinement), but CodeQL doesn't recognize is_relative_to as a taint sanitizer. Apply the Path.name pattern matching PR sonichi#487's dashboard.py fix: 1. _safe_path(base_dir, filename) — the task/result lookup helper (alerts sonichi#16-19). Rebuild filename via Path(name + ".txt").name. 2. /media/{rel} handler (alerts sonichi#7, sonichi#20-21). Multi-segment; split on "/" and rebuild each component via Path(p).name, then joinpath. 3. Inline task-file build for "answer-{qid}-{ts}.txt" (alerts sonichi#22-23). Apply Path(...).name to the full basename. Path.name is functionally a no-op after the regex whitelist (no separators possible), but it's a CodeQL-recognized path-injection sanitizer that breaks the taint flow CodeQL tracks through f-string path construction. Same pattern Mini validated on sonichi#487. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
liususan091219
added a commit
that referenced
this pull request
Jun 6, 2026
…udio() fn Per Susan 2026-06-06: separate the two concerns VOICE_CONTROLLER conflated — SUMMON/mode-control stays name-gated (can't be summoned by just anyone), but audio OUTPUT (answering conversation) is any-human. Extract the four inlined gate conditions (allowAckAudible / force-audible / active-mode any-human / meeting-mode summon-by-name) into a single top-level shouldEmitAudio(s, nowMs). Pure refactor — identical behavior, tsc clean. The summon name-gates (dismiss, switch_mode, meeting-enter) are untouched. This is the seam for bodhi #20: the same function is handed to VoiceSession as config.shouldEmitAudio once the pin bumps, letting the handleAudioOutput monkey-patch be deleted (the real line saving lands then). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
liususan091219
added a commit
that referenced
this pull request
Jun 8, 2026
…udio() fn Per Susan 2026-06-06: separate the two concerns VOICE_CONTROLLER conflated — SUMMON/mode-control stays name-gated (can't be summoned by just anyone), but audio OUTPUT (answering conversation) is any-human. Extract the four inlined gate conditions (allowAckAudible / force-audible / active-mode any-human / meeting-mode summon-by-name) into a single top-level shouldEmitAudio(s, nowMs). Pure refactor — identical behavior, tsc clean. The summon name-gates (dismiss, switch_mode, meeting-enter) are untouched. This is the seam for bodhi #20: the same function is handed to VoiceSession as config.shouldEmitAudio once the pin bumps, letting the handleAudioOutput monkey-patch be deleted (the real line saving lands then). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
qingyun-wu
added a commit
that referenced
this pull request
Jul 1, 2026
Presence wire-up (client half): the relay client already POSTs /v1/heartbeat
every interval; now it reads this node's core-status.json and includes
{status, step} so the AG2 broker's presence sweep can derive the agent's
activity badge (working / available / needs-attention) + the human "what it's
doing" text — the same core-status signal the Discord bridge streams locally.
- _read_core_status() reads <workspace>/state/core-status.json, best-effort
(any error → omitted, heartbeat still fires as plain liveness).
- idle status sends no step, so the sweep reads 'available' rather than a stale
"what it was last doing".
- status/step included only when present → never clobbers the broker's
last-known status; advertises a "core-status" capability.
- Backward compatible: brokers without /v1/heartbeat 404 + disable, as before.
Also pins REMOTE_TASK_TIER=team in the bridge test: it was non-hermetic and
read the host's ambient REMOTE_TASK_TIER (e.g. "owner" on the owner's node),
failing the tier-clamp + newline-forge assertions depending on where it ran.
(The sanitization itself was fine — the forge was always blocked.)
Pairs with ag2-space/ag2space-backend broker PR #20 (adds the endpoint that
records this into HUB.set_core_status). Bridge suite now fully green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
qingyun-wu
added a commit
that referenced
this pull request
Jul 1, 2026
…atus.json Addresses the blocking review finding on #1884: _read_core_status ran in the main loop BEFORE the /v1/tasks poll but only caught OSError/ValueError. A valid-JSON non-object core-status.json (e.g. a list) raised AttributeError on .get() — uncaught — backing the loop off and stalling task delivery. A presence side channel must never become a delivery blocker. - Guard isinstance(cs, dict); coerce status/step via _core_str → bounded non-empty str-or-None (non-string fields are dropped, not forwarded — the broker calls .lower() on them); catch-all → (None, None). - _read_core_status now MUST NOT raise; the heartbeat still fires liveness-only on any malformed file. - Regressions: valid-JSON non-object, non-string fields, malformed JSON, oversized step (bounded), and heartbeat-survives-malformed-file. Mirrors the broker-side normalization (ag2space-backend #20). All reviewer focused checks pass. Reviewed-by: Qingyun's Codex. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
qingyun-wu
added a commit
that referenced
this pull request
Jul 1, 2026
…#1884) * feat(relay-client): forward core-status in heartbeat for AG2 presence Presence wire-up (client half): the relay client already POSTs /v1/heartbeat every interval; now it reads this node's core-status.json and includes {status, step} so the AG2 broker's presence sweep can derive the agent's activity badge (working / available / needs-attention) + the human "what it's doing" text — the same core-status signal the Discord bridge streams locally. - _read_core_status() reads <workspace>/state/core-status.json, best-effort (any error → omitted, heartbeat still fires as plain liveness). - idle status sends no step, so the sweep reads 'available' rather than a stale "what it was last doing". - status/step included only when present → never clobbers the broker's last-known status; advertises a "core-status" capability. - Backward compatible: brokers without /v1/heartbeat 404 + disable, as before. Also pins REMOTE_TASK_TIER=team in the bridge test: it was non-hermetic and read the host's ambient REMOTE_TASK_TIER (e.g. "owner" on the owner's node), failing the tier-clamp + newline-forge assertions depending on where it ran. (The sanitization itself was fine — the forge was always blocked.) Pairs with ag2-space/ag2space-backend broker PR #20 (adds the endpoint that records this into HUB.set_core_status). Bridge suite now fully green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(relay-client): harden _read_core_status against malformed core-status.json Addresses the blocking review finding on #1884: _read_core_status ran in the main loop BEFORE the /v1/tasks poll but only caught OSError/ValueError. A valid-JSON non-object core-status.json (e.g. a list) raised AttributeError on .get() — uncaught — backing the loop off and stalling task delivery. A presence side channel must never become a delivery blocker. - Guard isinstance(cs, dict); coerce status/step via _core_str → bounded non-empty str-or-None (non-string fields are dropped, not forwarded — the broker calls .lower() on them); catch-all → (None, None). - _read_core_status now MUST NOT raise; the heartbeat still fires liveness-only on any malformed file. - Regressions: valid-JSON non-object, non-string fields, malformed JSON, oversized step (bounded), and heartbeat-survives-malformed-file. Mirrors the broker-side normalization (ag2space-backend #20). All reviewer focused checks pass. Reviewed-by: Qingyun's Codex. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sonichi
added a commit
that referenced
this pull request
Aug 3, 2026
…shared login
`is_mine` was `author == owner_login`. Several agents commit through the SAME
GitHub account, so that boolean was True for every one of them and for the human
whose account it is -- three principals collapsed into one field whose name says
"mine". A consumer reading it as "the owner\x27s PR" and a consumer reading it as
"my PR" are both wrong, and nothing in the record could correct either.
That is not hypothetical: it produced a digest that handed the owner 13 PRs as
"your own 13" when he had authored none of them, and told him he was the author
of a PR another agent wrote.
The `Stand:` commit trailer is the only signal that separates the agents, so
derive identity from it:
stands distinct Stand trailers across the PR\x27s commits, sorted
principal the stand, or "joint" (>1), or "unattributed" (none)
is_mine now trailer-derived via --stand; null when --stand is omitted,
because unknown beats a confident guess
`state_hash` keys on `principal` instead of `is_mine`: a change of author now
refires the digest. This rehashes once on first run after merge.
Trailers are fetched in their own narrow GraphQL pass, NOT by adding `commits`
to `gh pr list --json`: that also pulls each commit\x27s `authors` connection and
at --limit 1000 GitHub rejects the entire query ("up to 1,000,000 possible nodes
which exceeds the maximum limit of 500,000"), returning ZERO PRs rather than a
partial answer.
Falsified against the old code -- three PRs, one shared login, three different
principals, and the old field cannot tell them apart:
PR | OLD is_mine | NEW principal | NEW is_mine (as Pro)
#20 | True | Echo Act IV Pro | True
#21 | True | Echo Act IV Mini | False
#23 | True | unattributed | False
Verified on live data: the tool reproduces by machine the partition that had to
be derived by hand -- 7 Pro / 1 Mini / 4 joint across the 12 open shared-login
PRs.
Stand: Echo Act IV Pro
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sonichi
added a commit
that referenced
this pull request
Aug 3, 2026
…shared login
`is_mine` was `author == owner_login`. Several agents commit through the SAME
GitHub account, so that boolean was True for every one of them and for the human
whose account it is -- three principals collapsed into one field whose name says
"mine". A consumer reading it as "the owner\x27s PR" and a consumer reading it as
"my PR" are both wrong, and nothing in the record could correct either.
That is not hypothetical: it produced a digest that handed the owner 13 PRs as
"your own 13" when he had authored none of them, and told him he was the author
of a PR another agent wrote.
The `Stand:` commit trailer is the only signal that separates the agents, so
derive identity from it:
stands distinct Stand trailers across the PR\x27s commits, sorted
principal the stand, or "joint" (>1), or "unattributed" (none)
is_mine now trailer-derived via --stand; null when --stand is omitted,
because unknown beats a confident guess
`state_hash` keys on `principal` instead of `is_mine`: a change of author now
refires the digest. This rehashes once on first run after merge.
Trailers are fetched in their own narrow GraphQL pass, NOT by adding `commits`
to `gh pr list --json`: that also pulls each commit\x27s `authors` connection and
at --limit 1000 GitHub rejects the entire query ("up to 1,000,000 possible nodes
which exceeds the maximum limit of 500,000"), returning ZERO PRs rather than a
partial answer.
Falsified against the old code -- three PRs, one shared login, three different
principals, and the old field cannot tell them apart:
PR | OLD is_mine | NEW principal | NEW is_mine (as Pro)
#20 | True | Echo Act IV Pro | True
#21 | True | Echo Act IV Mini | False
#23 | True | unattributed | False
Verified on live data: the tool reproduces by machine the partition that had to
be derived by hand -- 7 Pro / 1 Mini / 4 joint across the 12 open shared-login
PRs.
Stand: Echo Act IV Pro
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
john-the-dev
added a commit
that referenced
this pull request
Aug 3, 2026
…shared login (#2553) * fix(pr-flag): identify the authoring AGENT by Stand trailer, not the shared login `is_mine` was `author == owner_login`. Several agents commit through the SAME GitHub account, so that boolean was True for every one of them and for the human whose account it is -- three principals collapsed into one field whose name says "mine". A consumer reading it as "the owner\x27s PR" and a consumer reading it as "my PR" are both wrong, and nothing in the record could correct either. That is not hypothetical: it produced a digest that handed the owner 13 PRs as "your own 13" when he had authored none of them, and told him he was the author of a PR another agent wrote. The `Stand:` commit trailer is the only signal that separates the agents, so derive identity from it: stands distinct Stand trailers across the PR\x27s commits, sorted principal the stand, or "joint" (>1), or "unattributed" (none) is_mine now trailer-derived via --stand; null when --stand is omitted, because unknown beats a confident guess `state_hash` keys on `principal` instead of `is_mine`: a change of author now refires the digest. This rehashes once on first run after merge. Trailers are fetched in their own narrow GraphQL pass, NOT by adding `commits` to `gh pr list --json`: that also pulls each commit\x27s `authors` connection and at --limit 1000 GitHub rejects the entire query ("up to 1,000,000 possible nodes which exceeds the maximum limit of 500,000"), returning ZERO PRs rather than a partial answer. Falsified against the old code -- three PRs, one shared login, three different principals, and the old field cannot tell them apart: PR | OLD is_mine | NEW principal | NEW is_mine (as Pro) #20 | True | Echo Act IV Pro | True #21 | True | Echo Act IV Mini | False #23 | True | unattributed | False Verified on live data: the tool reproduces by machine the partition that had to be derived by hand -- 7 Pro / 1 Mini / 4 joint across the 12 open shared-login PRs. Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(pr-flag): a failed trailer fetch is UNKNOWN, not a confident identity qingyun-wu's review is right, and the defect is the same shape this PR exists to remove. `_attach_commits` turned both failure paths -- a non-zero `gh api graphql` and an unexpected response -- into `pr["commits"] = []`, which downstream is indistinguishable from a successful fetch that found no trailer. So a transient error, rate limit or permissions hiccup produced: principal = "unattributed" is_mine = False for a PR this agent actually authored. That is a confident wrong identity, which is exactly what `is_mine = author == owner_login` was doing before this PR. My own body says "unknown beats a confident guess" while the fetch path did the opposite. A `STANDS_UNAVAILABLE` sentinel now marks the could-not-look case, and it is kept distinct from looked-and-found-none: could not look -> principal "unknown" is_mine None looked, none -> principal "unattributed" is_mine False looked, found -> principal "<stand>" is_mine per --stand `is_mine` is None for the unknown case even when --stand is supplied, so an outage can never relabel this agent's own PRs as someone else's. Behavioural falsification against the previous head, same input (a PR by this agent whose fetch failed): OLD principal=unattributed is_mine=False <- confident, and wrong NEW principal=unknown is_mine=None <- honest Three regressions added, including one asserting the dedup hash MOVES between "unknown" and a real stand -- otherwise a fetch outage would silently inherit the previous run's identity verdict instead of refiring. ruff clean; full pr-flag suite passes. Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(pr-flag): a 100-commit page was treated as the whole history qingyun-wu and john-the-dev both blocked on the same thing at head 32a5ce7, and they are right: `_attach_commits()` queried `commits(first:100)` with no `pageInfo`/cursor and treated that first page as complete. The STANDS_UNAVAILABLE sentinel added earlier covers a FAILED fetch but not a SUCCESSFUL, truncated one. That is the same defect this PR exists to remove, one layer down: a confident wrong identity. Demonstrated against the parent using the suite's own fixture -- TRUNCATED at 100 (what the parent fetches) principal='unattributed' is_mine=False COMPLETE 101 (the truth) principal='Echo Act IV Pro' is_mine=True so a Stand trailer on commit 101 silently disowns the agent's own PR, exactly as john's activated repro showed. The query now paginates with `pageInfo{hasNextPage endCursor}` and an `$after` cursor, walked to exhaustion. Anything that prevents a COMPLETE walk -- a failed page, `hasNextPage` with no cursor, or the defensive 100-page ceiling -- fails CLOSED to STANDS_UNAVAILABLE rather than publishing a partial set. A truncated history is not a small error here: one missing commit can drop the only trailer. `_gh_stands_page()` is a seam so the walk is testable without a network, which is what lets the regression exercise `_attach_commits()` itself rather than only `raw_state()` fixtures (john's explicit ask). Its `# pragma: no cover` moved down to the subprocess boundary; `_attach_commits` is now covered. Three regressions, inserted ABOVE the suite's final print so a failure cannot report after a green verdict: trailer-on-commit-101 survives pagination (and the cursor is actually threaded), a mid-walk page failure fails closed, and hasNextPage-without-a-cursor fails closed. ruff clean; py_compile clean; src-map parity unchanged; full suite ALL PASS. Stand: Echo Act IV Pro Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Rui Wang <52230987+john-the-dev@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unescaped apostrophe in reconnect error message broke the entire inline JS.