mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-11-07 06:19:43 -10:00
main
100
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Shaojin WenandQwen-Coder
|
5b44cfad71 |
fix(web-shell): include channel sessions in release dialog (#16537)
* fix(web-shell): include channel sessions in release dialog The release dialog inherited the sidebar's default source filter, which excludes sourceType: "channel". A workspace filled by idle channel sessions therefore opened an empty recovery list. Query the workspace catalog without a source filter for this dialog and keep the default, channel, and qwen-live rows. The sidebar and every other useScopedSessions caller keep the default filter. Add hook/dialog regressions and verify banner -> release -> create with a real sourceType: "channel" session filling a one-session workspace. * fix(web-shell): keep live sessions visible in release dialog Expanding the recovery list from the sidebar's default source filter to persisted history can still bury the sessions holding slots: closed channel rows fill the first page before they are reached. Add a liveOnly session-list option on the daemon route and SDK so callers can read the workspace bridge instead of persisted history. The release dialog requests the shared page size with that option and keeps listing default, channel, and qwen-live rows, so closed history can no longer crowd out what can be released. Co-authored-by: Qwen-Coder <[email protected]> * fix(sdk): serve liveOnly session lists over native REST The ACP session/list mapping drops liveOnly, so a DaemonClient configured with an AcpHttpTransport silently returned persisted history for the release dialog's live-only request. Route liveOnly through the native REST transport for both the legacy and workspace-qualified list methods, and cover the real ACP transport with a regression test. Co-authored-by: Qwen-Coder <[email protected]> * test(web-shell): pin the release dialog's source whitelist Live-only listings can include side_task, scheduled_task, and standalone sessions that the server does not hide. Add those rows to the dialog test so widening the whitelist fails, and state the 100-row live-page ceiling next to the page-size request. Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin Wen
|
0b54b9be99 |
fix(managed-agent): restore two runtime-broker classes to their size ratchet (#16542)
ClassSizeRatchetTest fails on main itself: RuntimeBrokerService is 2947 lines against a 2943 ceiling and RuntimeBindingProvisioning is 1327 against 1326. Both overruns come from #16535 replacing inline fully qualified names with imports, which adds import lines while the usage lines keep their count — 4 imports in RuntimeBrokerService and 1 in RuntimeBindingProvisioning — without updating the ceilings that #16501 had just recorded after the extraction. The SDK Java lane runs the merge ref, so every PR that merges main inherits the failure. The ratchet only lets ceilings go down, so reclaim the lines instead of raising the pins: join expressions that were already split across two lines. RuntimeBindingProvisioning's single Objects.equals operand, and the four BrokerValues.requireId assignments in release() and releaseMount(). Every joined line is at most 88 characters; the diff is formatting only and keeps #16535's import style. ClassSizeRatchetTest red before / green after; RuntimeBrokerServiceTest 199 green; runtime-broker checkstyle and spotbugs clean. No pin was edited. |
||
|
Shaojin WenandQwen-Coder
|
cf29f90062 |
fix(ci): route the review/resolve permission gate to the ECS pool (#16482)
* fix(ci): route the review/resolve permission gate to the ECS pool The authorize job gates every @qwen-code /review and /resolve command, but its ECS routing required github.event.pull_request, which issue_comment and workflow_dispatch payloads do not carry. Every command therefore fell through to ubuntu-latest, and the gate sat in the hosted queue for hours (PR #15286: 3h10m for a 4-second job) while the ECS pool was idle. Route issue_comment and workflow_dispatch to the persistent ECS pool, matching qwen-autofix.yml's route/review-scan pattern for PAT-bearing API-only steps. authorize runs no agent, checks out nothing, and processes no PR content, so this does not widen the fork attack surface. PR-payload events keep the same-repo fork guard; the hosted fallback and ECS kill switch are unchanged. resolve-pr and publish-resolution stay on hosted by design (ephemerality); this removes the gate wait only. * fix(ci): harden the authorize ECS-routing pins and risk docs Address review feedback on the gate routing change: - Pin the checkout-free invariant the ECS routing depends on: a uses: step on authorize would execute contributor-authored files on the persistent pool with CI_BOT_PAT in the step env while the expression pin stayed green (R1-1). Scoped to authorize; mutation-witnessed (adding actions/checkout reds it). - Record the trigger-set widening in the design-doc Risks (both languages): issue_comment makes the gate fireable by any GitHub user on a fork PR, and the should_review authorization fact is now produced on the shared host and consumed downstream without re-deriving (R1-2). - Correct the precedent wording: route loads only the read-only github.token; review-scan is the job that loads CI_DEV_BOT_PAT on the pool. * fix(ci): harden the shared-pool authorize gate against gh config reroute Address the round-2 CHANGES_REQUESTED review (R2-1 Critical, R2-2): R2-1: routing the gate to the persistent ecs-qwen pool moved its CI_BOT_PAT gh api call onto a host with a shared, writable ~/.config/gh. A co-resident survivor could plant http_unix_socket there, receive the PAT, and forge the permission response downstream consumes as should_review. Prepend the same pins as qwen-autofix.yml's route/review-scan — export GH_HOST=github.com plus a private, guarded GH_CONFIG_DIR (a failed mktemp exits 1; the gate fails closed). No `unset GH_TOKEN`: it is this step's only credential. Witnessed with real gh 2.103.0: the as-written step leaks the PAT to a planted socket and approves a stranger; the hardened step denies with the socket untouched. R2-2: extend the checkout-free guard to the run: half — pin the step inventory to the single permission step and forbid `${{` spliced into any run body (event values must arrive via env:). Mutation-witnessed: removing the preamble and adding a run-only second step each red the test. Also correct the design-doc parity claim (both languages): the exposure matches review-scan only WITH these gh pins; document the hardening. * fix(ci): extend the ECS gate routing to triage and harden the gate pins Two batches, verified together before push: 1. Extend the command-gate routing to qwen-triage.yml's authorize (the /triage, /tmux, /verify sibling blind spot): the same issue_comment / workflow_dispatch disjuncts, and — because that job has three steps that spend CI_BOT_PAT on gh — the same self-contained GH_HOST + guarded GH_CONFIG_DIR preamble on each, per the repo's per-step convention. Raise the recorded workflow size baseline to match the added hardening. 2. Address round-3 review (R2-2 fix-induced, R3-1): the gate pins tested substring presence only. Now evaluate the comment-stripped live lines and pin the guarded mktemp form + fail-closed shape, require GH_HOST / GH_CONFIG_DIR exported before the first gh api, and forbid executing repository code via relative paths or interpreters (the pool cwd is the reused $GITHUB_WORKSPACE). Applied to both the review and triage gate tests. Mutation-witnessed: an unguarded one-liner, a relocated preamble, and an appended `node ./…` line each red the tests. Tests: qwen-pr-review-workflow.test.js 271, qwen-triage-workflow.test.mjs 131, ci-runner-routing.test.mjs 103 — all pass; prettier + ESLint clean. * fix(ci): supply RUNNER_TEMP to the triage gate replay tests; harden the gate pins R4-1 (Critical): the new ${RUNNER_TEMP} gh-config preamble is the first RUNNER_TEMP reference in the triage permission step, and three replay harnesses in scripts/tests/qwen-triage-workflow.test.js execute that body verbatim without setting RUNNER_TEMP — off-CI (npm run test:scripts) the fail-closed guard exits 1 before writing should_run. Set RUNNER_TEMP to each harness's existing mkdtempSync dir (the :1773 precedent); the workflow's fail-closed guard is unchanged. Verified: the file is red under `env -u RUNNER_TEMP` before and 203/203 green after; dropping the line reds it again. (The review named two harnesses; running the full file surfaced a third.) Also clear the round-4 deferred notes: - R4-2: the /verify ack comment no longer promises a receipt "even when the sandbox lane is backed up" — the gate now also queues with the pool. - R4-3/R4-4: the triage routing pin is now an exact expression equality, the step inventory is pinned, and PAT-bearing steps are identified by env GH_TOKEN rather than gh subcommand spelling. - R3-1: tighten the no-repository-code deny-list (quote boundaries + interpreter as a word) so `bash -c './x.sh'` no longer slips past; mutation-witnessed in both the review and triage gate tests. Tests: triage vitest 203 (off-CI), triage .mjs 131, review vitest 271, ci-runner-routing 103 — all pass; prettier + ESLint clean; size gate green. * fix(ci): correct the gate-routing narrative; close the round-5 verify gaps R5-1 / F1 (narrative): the motivating incident is a /resolve, whose end-to-end latency this PR does not change — resolve-pr stays on ubuntu-latest, so only the gate wait is removed. State this in both design-doc languages and the pinning test comment; the saving lands on /review, /triage, /tmux, /verify (pool-routed downstream). Correct the "cancelled without ever running" sentence to the run's timeline (resolve-pr started when the gate finished and was cancelled ~29m36s later). S1: the no-repository-code deny-list missed absolute-path interpreters (/usr/bin/node x.js) and versioned Python (python3.11 x.py); widen the regex (optional path prefix + python[0-9.]*) in both gate tests. Mutation-witnessed: both previously-surviving mutants now red, clean tree green. S2: each hardened gh step minted a $RUNNER_TEMP/*gh-config.* dir that nothing removed; add trap 'rm -rf "$GH_CONFIG_DIR"' EXIT to all four steps (RUNNER_TEMP survives between jobs on the pool). S3: record the exact final workflow sizes in .size-baseline. Tests: triage vitest 203 (off-CI, RUNNER_TEMP unset), review vitest 271, triage .mjs 131, ci-runner-routing 103 — all pass; prettier + ESLint clean; size gate green. * fix(ci): pin the no-repository-code invariant on every triage gate step The triage gate meta-test applied the relative-path/interpreter denylist only to PAT-bearing steps, so a repository-code execution line added to the credential-free budget step survived the suite (witnessed by mutation). The no-repository-code invariant is a job-wide safety premise of the ECS routing: even a credential-free step that runs a planted file leaves a survivor on the persistent host. Pin it on every step's comment-stripped live lines, and align both design-doc versions with the widened coverage and the witnessed mutation set. Co-authored-by: Qwen-Coder <[email protected]> * docs(ci): correct the routing-matrix coverage claim for the triage gate The validation plan said the triage gate is pinned by the triage meta-test AND the ci-runner-routing matrix, but the matrix never reads either authorize job (it pins the sibling pool jobs). State the actual coverage in both language versions. Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin WenandQwen-Coder
|
47d41dbf71 |
fix(channels): keep the cursor write alive when the temp file mode cannot be tightened (#16257)
* fix(channels): keep the cursor write alive when the temp file mode cannot be tightened Mounts without Unix permissions (SMB/CIFS, exFAT, some FUSE) accept the write but refuse chmod, so the unguarded temp-file chmod made every save throw before the rename and the cursor was never persisted. Guard it like the directory chmod and pin the behaviour with an EPERM test. Also seed the scope directory at 0o755 in the permissions test so it proves the tightening, and pin that an empty scoped cursor starts fresh instead of falling back to the legacy global file. * test(channels): assert the injected chmod fault fired in the cursor guard test Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin Wen
|
4c8ebeab4d |
feat(core): view one pull request by URL for the PR-state watch (#16530)
* feat(core): view one pull request by URL for the PR-state watch Add fetchGitHubPullRequestView(url, cwd, env?), the single-PR view the daemon's PR-state watch will use (step two of PR A in docs/design/2026-11-04-daemon-pr-state-transitions.md, section 1). It runs `gh pr view <url> --json url,title,headRefOid,mergeable,state, statusCheckRollup` and returns the canonical URL, title, head SHA, open/merged/closed, mergeable (absent while GitHub reports UNKNOWN), the CI state and the failing check names (`<workflow> / <job>`, the check name or the status context; once each, at most 20). Only an http(s) /owner/repo/pull/<n> URL is accepted, and it is rebuilt without credentials, query, fragment or tab path before it reaches gh, so an issue URL, /PULL/7 or /pull/0 never becomes a branch lookup. The working directory must exist, so a missing one is not reported as gh being unavailable. Failures carry a reason (not_a_pr_url, cwd_missing, gh_failed, timed_out, unparsable). There is no host check; the caller restricts URLs to the allowed GitHub hosts. aggregateChecks is exported with unchanged behaviour; its failing-entry predicate is shared with the failing-name extraction. Every rollup entry still counts: gh's JSON exports no event, app or workflow id, and a dedupe by names and times would let a skipped or other-trigger run hide a current failure. The design doc's Files-affected row for core names the real signature and fields. * fix(core): report gh's diagnostic, not Node's argv, when gh fails execFile's callback error has no stderr property (only the promisified form attaches it), so ghErrorMessage fell back to Node's "Command failed: <argv>" message: a failed PR list, view or create reported the gh command line (for create, the PR title and body) ahead of gh's diagnostic, which the 4,096-character raw cap and the route's 512-character display cap could then cut off. The three callback wrappers now attach the callback's stderr, as the PR issues wrapper already does. When gh exits or is signalled with nothing on stderr, the message names the exit code or the signal instead of falling back to the argv, and a failure Node raises itself (a string code such as a spawn error or an overflowing buffer) keeps Node's message even if gh wrote something. `gh api graphql` failures of the PR issues fetch, which share the helper, get the same treatment. The test helper now fails the way execFile does: an exit 1 unless told otherwise, with stderr only as the callback's third argument, so the stderr tests exercise the wrappers and the stderr-over-exit-code order rather than the mock. Tests cover a silent exit, a signal, a buffer overflow with stderr, and a silent create that must not echo the title and body; the view's failing-names test gains a failing check run with an empty name and a status context with an empty context, which fail the head but are not named. |
||
|
Shaojin Wen
|
c1aa266e82 |
perf(core): compare a grown local journal prefix in chunks (#16507)
- Re-read a file that grew during the read through one 64 KiB chunk instead of a second buffer of the whole size, so an unbounded read of a transcript being appended to holds one copy of it. - Name the same-length rewrite within one timestamp tick as a cost decision in the comment. - Inject the read races through one helper, and cover a same-length rewrite with moved timestamps, a bound below the file size, a shrink that stays above the bound, a short comparison read and a comparison over several chunks. Drop two cases that duplicated others. |
||
|
Shaojin Wen
|
0807f5d391 |
refactor(channels): share the bound-session prompt contract via channel-base (#16523)
* refactor(channels): share the bound-session prompt contract via channel-base Move the GitHub channel's operator instruction template contract (key validation against a caller-supplied event vocabulary, template validation, the role/event/lane resolution chain, the %var% renderer) and the bound prompt envelope (platform header, untrusted-data and not-published notices, labelled trusted-instruction block, sanitised data lines, Role line, visible 6,000-code-point body cap) into @qwen-code/channel-base, so the daemon's PR-state watch can reuse them instead of keeping a copy (step one of PR A in docs/design/2026-11-04-daemon-pr-state-transitions.md). The adapter keeps its vocabulary, the deliverToBoundSessions precondition, the comment-lane class, metadata parsing and provenance line, and its output is unchanged except for two rendering fixes: %constructor%-style names now stay literal like any other unknown name (they rendered Object.prototype members), and every substituted value is sanitised, as the templates design specified (only %title% and %sender% were; those two are still also capped at 200 code points). Tests pin the legacy, role-aware comment and role-aware watch prompts byte for byte (strings captured from the previous adapter), %url% on a GitHub Enterprise origin with a port, and the exact validation messages. The design docs, the template editor design and the user guide are updated to match. * fix(channels): keep the trusted instruction label inside the tag peel window The bound prompt labelled its trusted block `[Operator instruction — trusted, from <source>]`. With the channel's source that label's content is 65 characters, one over the 64 that sanitizePromptText peels from a start-of-line tag, so an event body that began with a copy of the label kept its brackets and read as a second trusted block. The label is now a fixed `[Operator instruction — trusted]` with the source on a `Source:` line under it, so it stays peelable whatever source a producer names. The platform the header names becomes a closed list (BOUND_EVENT_PLATFORMS) for the same reason: the header's content is the name plus 36 characters. A test runs over that list and feeds a copy of the header and of the label through the body and a data line; another puts a copy at the start of a substituted value. Also from review: pin the untyped-winner guard of the resolution chain, empty values rendering as empty, and misspelt `watch:` types being rejected, and align the templates design's validation plan with the cap that applies only to %title% and %sender%. The templates design, the daemon design and the user guide show the new label; the templates design's prompt example gains the not-published notice it was missing, and the daemon design's validation plan now expects only the prompt without a template to stay byte-identical. |
||
|
Shaojin Wen
|
ced03aef5d |
feat(serve): reclaim idle channel sessions (#16532)
Channel workers keep an SSE event pump attached to every cached route, so the idle reaper treated those sessions as actively watched and never reclaimed them. A workspace could fill every maxSessions slot with sessions nobody was using and then reject new creates with 503. Let the reaper look past a channel-owned subscriber while preserving the local-work, active-prompt, and child conditional-close guards. Surface session_limit_exceeded as a banner with a release action. The release dialog falls back to the workspace daemon client when it opens from the empty new-session screen, where no session client exists. Design: docs/design/channel-session-idle-reclaim.md |
||
|
Shaojin WenandQwen-Coder
|
61aef512e7 |
feat(web-shell): offer a details side task from pending AskUser questions (#16458)
* feat(web-shell): offer a details side task from pending AskUser questions While an AskUserQuestion is pending, the Web Shell hides the composer and the btw shortcut ignores the pending-approval state, so a user who does not understand the options can only answer blindly or ignore the question. Add an "Ask for details" action to the pending panel. It opens a side task whose prefilled prompt snapshots the question and options and asks for a verified explanation — background, per-option meaning, differences, impact and risks, and a recommendation — delivered as a self-contained single-file HTML in controlled ASD-STE100-style sentences under .qwen/tmp/, previewable in the right panel. The side task inherits the parent context but never answers the question or touches the main session; the question stays pending until the user answers or ignores it. The action is main-view-only and gated on the session_side_task capability, matching /btw side. No daemon, protocol, or settings changes. * fix(web-shell): one details side task per question, reachable on narrow panels Post-publication audit of the AskUser details action found three issues: - Repeat clicks provisioned duplicate side tasks for the same pending question, each burning a model turn. Give the details tab a stable id derived from the parent session and the permission request id and upsert it, reusing the artifact panel's open-or-focus pattern; /btw side keeps its unique per-invocation draft id. - The question snapshot dropped multiSelect, so the explanation prompt did not say options can combine. Append a [multiSelect] marker (strict true). - On a 320px two-question panel the actions row overflowed to the left and pushed the details button out of the panel (left: -103px in a real-browser probe; scrollWidth misses left overflow on a flex-end row). Let the row wrap and pin a narrow-viewport regression test asserting every action button stays inside the panel and stays clickable. Each fix carries a red/green witness: the repeat-click App test, the multiSelect unit test, and a flex-wrap mutation of the narrow visuals test. * fix(web-shell): keep the details tab id within the daemon clientId budget The dedup fix derived the details tab id from the parent session id AND the request id. SideTaskPanel derives the pane's daemon clientId as side-task:<parentSessionId>:<tabId>, which with two uuids reached 138 chars — over the daemon's MAX_CLIENT_ID_LENGTH (128). The child session load then 400s, the pane never connects, and the prefilled prompt is never sent. Caught by re-running the real-daemon scenario on the post-fix head; mocked unit tests could not see it. Derive the stable id from the permission request id alone (a uuid; tabs are already bucketed per parent session), bringing the clientId to 101 chars. Add a SideTaskPanel regression test asserting the derived clientId stays within 128 for uuid-length inputs; the old id shape fails it with 138. * fix(web-shell): retain the details question-to-child association across tab close Review found that deduplicating only against open tabs loses the existing child when the details tab closes: clicking "Ask for details" again for the same pending request provisioned a second child, and reopening the child from the side-task catalog first did not deduplicate either (its tab id is side-task:<sessionId>, not the details-request id). Persist the pending-request-to-child association per parent session in localStorage, written when the details child is created. On click, focus whichever open tab shows the known child (details tab or catalog tab); with no tab open, reopen one attached to the existing child session — no draft, no initial prompt, so nothing is provisioned or sent again. One entry per parent supersedes the resolved question's association, keeping the store bounded; the association also survives reloads. Regression tests cover both reported paths (close + re-click keeps exactly one createSideTaskSession call and reopens with the child's sessionId and no initialPrompt; catalog reopen + click focuses the single existing tab). Red witness: dropping the association lookup fails the close + re-click test with two creation calls. * fix(web-shell): close the details association lifecycle gaps Re-review of the association persistence found three lifecycle gaps, all fixed here with regression tests and mutation witnesses: - A child exists before its explanation prompt is admitted. Reopening a closed details tab now retains the prompt on the tab; the pane's existing empty-session guard delivers it exactly once (only while the child has no user prompt) and never re-sends it. - Archiving a details child left its persisted association pointing at an unloadable session (the daemon's load route 409s session_archived). archiveSideTasks now invalidates associations pointing at archived children, so the next click provisions a fresh child. - Closing the draft tab before creation resolved skipped the association write, orphaning the child and provisioning a duplicate on the next click. The parent identity is captured when creation starts, and the write no longer depends on the tab still existing. Also pin two invariants the deep-verification report found unasserted: the derived clientId budget with uuid-length inputs through the real tab-id derivation (App test), and the formatter's array guard against non-array objects (formatter test). * fix(web-shell): supply the unsent details prompt to a catalog-reopened tab Co-authored-by: Qwen-Coder <[email protected]> * fix(web-shell): treat media-only user prompts as interaction in the side-task guard Co-authored-by: Qwen-Coder <[email protected]> * fix(web-shell): validate details child archive state at the read site and address review suggestions Co-authored-by: Qwen-Coder <[email protected]> * fix(web-shell): replace stale details tab on fresh draft and re-check pending after the liveness query Co-authored-by: Qwen-Coder <[email protected]> * fix(web-shell): replace a details tab bound to a different child than the validated binding Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin WenandQwen-Coder
|
5b4dcbad19 |
fix(channels): watch every open bound PR and keep polling during public turns (#16502)
* fix(channels): watch every open bound PR and keep polling during public turns The PR state watch selected bound PRs purely by binding recency under a hard cap of 30, so older open bindings were never polled once newer bindings (including already-merged ones) filled the budget, and merged bindings outside the cap never received the terminal observation that would retire them. Selection now watches open PRs first and drains terminal-pending bindings with the leftover budget; the cap becomes the configurable prWatchLimit (default 30, range 1-500, fractional values round up), exposed in the channel descriptor and management API. Public-path tasks (review requests and assignments on unbound PRs) ran their full agent turn inside the poll loop, freezing watch detection, bound delivery, and every other notification for the turn's duration (observed: 64 minutes on a production daemon). They now execute in a background FIFO runner (concurrency 1); claims stay durable and synchronous, the cursor gate and crash recovery keep their semantics, and bound/watch deliveries stay inline. The first-contact body feed gains a thread-level gate so an async body turn cannot overtake and steer-cancel a still-retrying comment turn. * fix(channels): close review gaps in watch coverage and the async task runner Review round on the bound-delivery reliability fix. Three correctness findings, each reproduced against the base: - The first-contact body deferral gate could lose the body permanently: nothing reserves a deferred feed, and the end-of-poll cursor commit can mark the thread read once the blocking task settles. Remove the gate; the runner FIFO already serializes the body behind the comment turn. The absence gate now runs synchronously at dispatch so backpressure deferrals keep the false verdict the lane's body suppression needs. - disconnect retired the runner generation but the winding-down runner's restart hook drained the queued tasks anyway. Guard the restart by generation and resolve idle waiters at disconnect. - Recovery could re-resolve a backpressure-parked task whose public turn was running (the running transition merges patches) and deliver it bound a second time. Skip in-flight ids before any recovery branch mutates a task; crashed tasks without an in-flight entry still recover. Also: gone-session public fallbacks requeue through the serial runner instead of awaiting the turn in the poll lane (head-of-line block and steer-cancel on shared sessions); a same-instance reconnect keeps the orphaned runner's executing id in flight so recovery cannot double-run it; held threads skip redundant comment re-fetches via a poll-start snapshot; escaped runner failures are journaled once per task; the watch budget reserves a terminal slice (20%, at least one) so the drain progresses even when open PRs fill the limit; stale-open bindings tier as terminal once the watch observed closed; the limit constants are exported for the descriptor, the field is registered and translated in the Web Shell editor, and validation enforces the bound-delivery prerequisite; the rate arithmetic is documented at every surface. * fix(channels): drain terminal bindings outside the open watch budget The in-budget terminal reservation excluded up to ceil(limit*0.2) open bindings while a merged/closed backlog drained, permanently swallowing edge events on those bindings — the defect this branch fixes. Open PRs now take the full prWatchLimit; terminal-pending bindings drain in an additional slice outside it (20%, at least one, plus leftover budget), bounded at limit + ceil(limit*0.2) pulls.get per poll during the drain. Pins the verification fixture (30 open + 5 newer terminal at limit 30: oldest open observed, its head change delivered) and keeps the removed in-budget variant as a mutation witness. Rate arithmetic updated on the descriptor, i18n (EN/ZH), user guide, and bilingual design docs. Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin WenandQwen-Coder
|
a12d6946c2 |
fix(managed-agent): mask digit-led passwords and refuse an empty Harness user info in the upgrade command (#16352)
* fix(managed-agent): mask digit-led passwords and refuse an empty Harness user info
Follow-ups to the review items deferred on #16110 and #16175:
- A refusal hides everything up to a URL's last @, also when the URL
parses: a password that starts with digits and holds a / ? or # reads
as a port to both the URL parser and java.net.URI, so
http://svc:2024#Secret@harness:4170 printed whole. The --server-url
query refusal masks too.
- Without the server's verdict, start refuses an @ before the Harness
host (http://@host, http://:@host), which java.net.URI reads as user
information and the connector refuses, instead of restarting the
Harness for it.
- The health row and start show a status of 0, false or "" instead of
dropping it, and print the URLs they dial through the control-byte
neutralizer.
- A Session id with a lone surrogate fails its row by name instead of
"URI malformed".
- The Flyway reader test covers a path two modules both ship; the user
guide names verify --tenant's --actor condition, and the design docs
state the masking, status and id rules as applied.
* fix(managed-agent): neutralize the DEL and C1 bytes a quoted value keeps
JSON.stringify escapes C0 controls but not DEL or C1 bytes, so a refused
URL or a Session id the verify row echoes could carry U+009B, an 8-bit
CSI introducer, to the terminal, and the id was not bounded. The four
refusals that quote a URL and the id row now go through printable()
after JSON.stringify: C0 controls keep their escapes, DEL and C1 bytes
show as U+FFFD, and the quoted text stops at 200 code points.
* fix(managed-agent): print no password a --server-url carries, and stop predicting java.net.URI
Review round 2 on #16352.
- A --server-url with an @ left after the URL parser reads no user
information is refused: it is the rest of a password that starts with
digits, which the parser reads as a host and a port, so the phase would
dial the wrong host and print the password. check judges --server-url
the same way, since its plan repeats it.
- start's three outcomes and verify's health row print every URL with its
user information masked.
- With the server's verdict false, the refusal names a reason only where
the value shows one the server gives too: a user name or password, a
query or a fragment, or an @ in an authority after //.
- Without a verdict, start takes only a plain http(s) URL, a rule of its
own and narrower than the server's, instead of modelling how
java.net.URI reads user information: http:/@host, a backslash, a tab,
an underscore or a space no longer reach the Harness restart, and an @
in a path still does.
- The design record and the deployment guide say so.
* fix(managed-agent): correct upgrade URL guards and plan output
* fix(managed-agent): keep refusing an @ in --server-url, and spell plan words
|
||
|
Shaojin Wen
|
f2e9a3fc66 |
feat(channels): add GitHub repository event filter with allowlist and blocklist (#16493)
* feat(channels): add GitHub repository event filter with allowlist and blocklist The GitHub channel processed notifications from every repository the authenticated account can see. The only repository-level control was the group access allowlist, which cannot express "everything except these repositories" and hides a plain event filter behind access-policy machinery. Add repoFilterMode (off / allowlist / blocklist) and repoFilter to the GitHub plugin. The filter runs at notification intake, before the reason filter, journals capped repo_filter skip rows plus one collapsed count row, and never holds the poll cursor. One shared validator rejects an active mode with an empty or malformed list at save time and at connect time; while the mode is off a stored list is ignored, so it can be parked without blocking unrelated saves. Notifications about bound PRs are filtered too; only their PR state watch events bypass the filter. The Web Shell editor gains a Repository Filter select and a Filtered Repositories list shown only while a mode is active, with the visibility and validation gated on the same predicate. Pasted repository or PR URLs are shortened to owner/repo on save; non-repository GitHub URLs are rejected instead of silently shortened. A parity table binds the editor mirror to the plugin's own validator, and the visuals suite captures the new editor in both themes. * fix(channels): close review findings on the repository event filter Address the verify-round findings and the triage review notes on #16493: - F1 (Medium): the reserved-path guard missed sibling GitHub areas, so pasted URLs like github.com/apps/renovate or /features/copilot were still silently shortened to nonexistent repositories that both validators accepted — fatal under an allowlist. Extend the guard with the measured missing roots plus the other known two-segment areas, export the set, and replace the sampled fixture with an enumerated test over every member plus explicit regression pins for the reported shapes. Removing a listed root now fails the pins; the denylist's best-effort nature is documented instead of overclaimed. - F2 (Low): pin the documented ordering — a notification excluded by both gates is journaled repo_filter, not reason_filter. The verify round's M7 mutation (swapping the two blocks) survived the old suite and now fails this test. - Triage boundary note: pin that a bound PR's state watch events bypass the repository filter, so a future refactor of the watch call site cannot silently silence bound sessions. - Docs: soften the URL-rejection claim to "known non-repository areas" in the user guide and both design docs, and state the residual denylist risk. * fix(channels): close the ten inline review findings on the repository filter - R1-1: close the arbitrary-host class structurally. The paste-shortener now only shortens URLs on the channel's own GitHub host (github.com plus the GHES host derived from baseUrl, api. prefix stripped); api.github.com, gist.github.com, gitlab.com and any other host pass through for validation to reject. Add the missing advisories/copilot areas, a snapshot assertion over the whole reserved set (removing any member now fails, which the enumeration alone could not witness), and foreign-host fixtures. - R1-2: seed paginate before connect() in the rejection helper so a successful connect no longer prints an unasserted poll-error backoff line. - R1-3: dialog case opening on a stored active mode — the list shows the stored text and a re-save keeps both keys. - R1-4: pin the notification side of case-insensitive matching (mixed-case subject URL vs lowercase entry) and the normalizer's trim/empty handling. - R1-5: pin the complement of the watch bypass — a notification about a bound PR in a blocklisted repository is filtered, not delivered. - R1-6: capitalized reserved-root variant pins the guard's own lowercasing. - R1-7: vocabulary test binding the repoFilterMode descriptor options and default to the exported REPO_FILTER_MODES, mirroring prWatchEvents. - R1-9: two-poll journal budget reset test (42 rows, two counted rows). - R1-10: extend the Off-retirement case with the discriminating switch back to Allowlist, where a stale error would remount. - R1-11: typing a valid entry (or correcting a malformed one) now retires the shown instruction through refreshErrors, with two no-save dialog cases; Submit remains the only error population site. Mutation witnesses on this head: dropping chatId lowercasing, the host gate, the list-typing retirement branch, and the refreshErrors guard each fail exactly their new test; all restored byte-identical. * fix(channels): exclude the channel's own API host and paths from URL shortening Round-3 review findings on #16493, all closed in this commit: - R1-1 (Critical, fix-induced): the host gate re-admitted the channel's own API host — with baseUrl set, a pasted REST URL shortened to `repos/octo` (api.github.com) or `api/v3` (GHES /api/v3), phantom repositories both validators accepted. Only the WEB host now qualifies: an api.-prefixed baseUrl host contributes only its stripped form, and paths under the baseUrl's own API prefix are excluded. Genuine GHES web URLs still shorten. Pinned with baseUrl actually passed through validation, for both REST shapes; re-admitting the raw API host reddens the test. - R2-1: the baseUrl argument now has a call-site witness on both paths — a GHES draft validates clean and saves ['octo/repo'] through buildChannelUpsertRequest, mirrored in the parity table against the daemon authority. Dropping either argument reddens its half. - Deferred notes (all three closed, none deferred onward): trailing sentence punctuation is stripped before matching (a pasted `…/octo/repo.` no longer becomes the phantom `octo/repo.`); `enterprises` joins the reserved set (snapshot updated); and a dialog case witnesses the parked-list round trip — hidden while Off, restored with its text on re-enable. Mutation witnesses: re-admitting the raw API host, dropping the validator baseUrl argument, and dropping the save-path baseUrl argument each fail exactly their new test; all restored byte-identical. |
||
|
Shaojin WenandQwen-Coder
|
f387c3a75a |
fix(ci): recreate git metadata before the publish-triage checkout (#16494)
* docs(ci): state the real publish-triage safety argument The publish-triage header comment claimed a 'clean runner', but the job routes to the persistent ECS pool like publish-tmux and runs without a container. The guarantee that actually holds is trusted-code-only: a sparse checkout pinned to github.sha, never agent-provided code, and a publisher that imports only node builtins plus one relative helper, so a reused workspace offers no bare-specifier node_modules to hijack. Reword the comment to say that, per the non-blocking note in the #16468 re-review. Comment-only change; no job, timeout, or behavior delta. Co-authored-by: Qwen-Coder <[email protected]> * fix(ci): recreate git metadata before the publish-triage checkout The patrol closeout on the comment rewrite checked the claims against the file and found two gaps. First, publish-triage was the only job reaching actions/checkout on the shared pool without recreating the workspace .git: an earlier agent run owned that workspace and could have planted hooks/config that checkout's git commands would execute in the one job holding the write PAT. Add the same Recreate runner Git metadata step the triage/tmux/verify jobs run before their checkouts, and add publish-triage to the qwen-runner-checkout-workflow matrix that behaviorally executes it (removal witness: deleting the step turns the new entry red). Second, narrow the header: drop the 'it is safe there' certification in favor of naming the controls, and replace 'sole writer' — the triage job's reaction and lifecycle host steps also write with the PAT — with 'the only path from the agent's drafts to GitHub'. Design pair updated in both languages. Co-authored-by: Qwen-Coder <[email protected]> * docs(ci): keep the new step comment off the PAT-holder claim The publish-triage step comment and the design pair said the checkout runs in "the job that holds the write PAT", which is the same overstatement this PR removes: the triage job's reaction, status comment, and finalize step also write with that PAT. Name the publisher as what it is instead, so the header's new parenthetical and the design prose agree. Co-authored-by: Qwen-Coder <[email protected]> * fix(ci): pin the publish-triage safety claims and derive the checkout guard Addresses the remaining review findings in this PR. R1-2: publish-tmux's banner still claimed a clean GitHub-hosted runner while its runs-on selects the ECS pool; reword it to the controls that actually hold (checks out nothing, runs no repository code), routing untouched. R1-4: the publisher's bare-specifier-free import closure is load-bearing for the pool argument but nothing pinned it; add a meta-test that scans the closure and matches it against the sparse-checkout list (witness: adding import 'yaml' reddens exactly that test). R2-2: the runner-checkout guard's hand-typed job list is what missed publish-triage; derive the qwen-triage entries from pool-routed jobs that check out, so the next such job reddens with no test edit (witnesses: a new pool checkout job without the reset, and deleting publish-triage's reset, both redden). R2-3: pin publish-triage's ownership-restore fallback chain with assertUnconditional (witness: deleting the sudo fallback reddens). R2-1: the design pair's Files-affected list still said the runner-checkout test needs no change; correct both languages and narrow the pinned-by parenthetical to what each suite actually pins. Co-authored-by: Qwen-Coder <[email protected]> * test(ci): make the new pins layout-independent, floored, and complete Round 3 found three holes in the pins added by the previous commit, all in the guards themselves. R3-1: specifiersOf used a single-line pattern, so Prettier-wrapped imports were invisible in both directions — a wrapped bare specifier kept the closure test green while the pool job would die ERR_MODULE_NOT_FOUND, and a wrapped relative one dropped a needed entry from the expected sparse list. Span newlines, hoist the scanner to module scope, and fixture-pin it (wrapped relative, wrapped bare, and the publisher's own multi-line node:fs import); those three fixtures were verified red against the old pattern and green after, and the sparse comparison stays exact equality. R3-2: the derived job set had no floor, so rewriting one pool job's runs-on into a vars lookup silently dropped its row; add an arrayContaining floor that names the lost job while still admitting new pool jobs with no test edit. R3-3: the ownership test's name promised the full fallback chain but assertUnconditional matches chmod unanchored, so deleting the sudo chmod arm stayed green; pin that arm in this test rather than the shared helper, which has five other call sites. Witnesses: wrapped bare import reddens the closure test, a vars-lookup runs-on reddens the floor naming publish-triage, and deleting the sudo chmod arm reddens the ownership pin — each with byte-level restore. Co-authored-by: Qwen-Coder <[email protected]> * test(ci): prove the sparse checkout by loading it, not by scanning syntax The APPROVED review deferred three findings against the closure guard, all instances of one root cause: the guard modelled JS module syntax with a regex. Proven on the shipped code — a 1164-character wrapped bare import left it green (the 400-char bound), and an out-of-tree helper turned the byte-offset slice into the garbage entry /.github/scripts/elper.mjs. Replace the scanner with the condition the pool actually creates: materialize exactly the sparse-checkout list into a temp tree, assert no node_modules exists above it, and import the publisher there. Sufficiency is the load succeeding; necessity is each .mjs entry omitted making it fail; the owner policy is tied by resolving the publisher's own new URL literal and requiring that exact path in the list, instead of an unanchored substring plus a hard-coded entry. This removes the scanner and its layout fixtures rather than patching them again — wrapped imports of any length, prose that looks like an import, and out-of-tree helpers are all covered by the load itself. Witnesses: the long wrapped bare import that was previously green now fails, as do a short bare import, an out-of-tree helper, a helper dropped from the list, a stale extra entry, and a moved owner-policy path; each with byte-level restore. Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin Wen
|
930962ab51 |
fix(managed-agent): let a late snapshot publication wait for the claim instead of losing deadlocks (#16489)
* fix(managed-agent): let a late snapshot publication wait for the claim instead of losing deadlocks Since #16143 resource_key is a secondary unique index, so a second publication's manifest upsert queues for the first's uncommitted row with a next-key lock. Every page the first Build inserts into that lock's gap rolls the queued publication back, and on longer journals #16446's three attempts run out: on a 31-page journal a late second publication failed for 15 of 40 snapshot ids. The claim now locks the manifest row by key first. At READ COMMITTED that takes a record lock when the row exists, which does not block the inserts into the gap, and only a missing row is upserted. Two publications that both find no row can still meet in the upsert; #16446's retry handles the one rolled back, whose locking read then waits for the winner. The MySQL cases pin both paths and assert the key order they depend on. * test(managed-agent): pin the snapshot claim's reads, upserts and rollback interleaving - State in the claim's Javadoc that the wait on the manifest row lasts at most innodb_lock_wait_timeout and that a lock wait timeout (1205) is not retried, only deadlock victims (1213) are. - Count the claim's locking reads and upserts in the test JdbcTemplate and assert them in the new concurrent cases, so a reworded claim statement fails those cases instead of turning them into plain races. - Cover a publication queued on a claim that rolls back: it finds no row and publishes the snapshot itself. - Check that publications run at READ COMMITTED, which the claim's comment relies on. - Read the monitor fixture through one helper. |
||
|
Shaojin Wen
|
80fd7c3353 |
fix(core): re-read a grown local journal prefix even when its timestamps did not move (#16483)
readLocalJournalBytes accepted a read when ctime and mtime were unchanged, without checking the size. Linux takes file timestamps from a coarse clock, so a growth that also rewrote the prefix within one tick of the previous write was accepted, and "refuses a frozen prefix rewrite even when the file also grows" failed in about half of its runs. Equal timestamps now count only while the size is unchanged. A file that grew is read again up to the size read the first time, bounded or not, and accepted only when those bytes match. An unbounded read during a pure append therefore returns the generation its first stat saw instead of failing, which writer acquisition and the session list relied on. The tests pin fstatSync's timestamps so the one-tick cases hold on every run. |
||
|
Shaojin Wen
|
e98e4db75e |
perf(managed-agent): cache parsed statements on the MySQL pools by default (#16414)
Connector/J parses a statement's SQL on every prepareStatement unless its per-connection parse cache is on, and that parse was the largest share of the control plane's Java CPU under load. The server and embedded MySQL pools now add cachePrepStmts=true, prepStmtCacheSize=512 and prepStmtCacheSqlLimit=8192 as connection properties. A connection property overrides the URL in Connector/J, so each setting is added only when neither the URL nor the Druid connection properties name it, ignoring case, percent-encoding and surrounding spaces. Nothing is added when either names useServerPrepStmts or useCursorFetch, since the cache would then hold statements open on the server. The server pool binds spring.datasource.druid inside its factory, before the defaults are added: @ConfigurationProperties bound it only after the factory returned, and the string form of connection-properties replaces every connection property. |
||
|
Shaojin Wen
|
24effd60a4 |
fix(managed-agent): lock storage rows through their alias key (#16376)
* fix(managed-agent): lock storage rows through their alias key A storage row has a primary record and three secondary indexes, and a locking read through one of them locks that record and then the primary record. Storage admission read the row for its fence through (tenant_key, storage_key), locking the primary record first, and then asked for the alias record in requirePhysicalOpen. Runtime binding creation, execution membership and journal commits lock the alias record first, so they deadlocked with it (#16320). - owner() and clearFence go through alias_key. - lockCovered lists the physical storage's aliases on a fresh read and locks them through alias_key, in alias_key order, before its physical_key scan, which is forced onto the physical index. Its scan used to take each alias's primary record first; it would otherwise deadlock with alias-first storage admission, and two recoveries of aliases sharing a storage take the aliases in one order. - StorageRowLockOrderMySqlIT pauses one transaction after a chosen statement until the other queues, for each pair, and pins the scan's plan. * test(managed-agent): pin the alias pre-lock to a range read of the alias index The pre-lock that orders lockCovered's alias locks carried no index hint and no plan assertion. It now forces uk_ma_storage_business_key and reads physical_key as well: a covering read of a list as long as the estimated row count becomes a scan of the whole alias index, which locks every gap. The plan case asserts a range read on that index, including a storage whose aliases are the whole table; the covering form fails it. The alias-list read on the separate connection now fails with its own message, so a log tells it apart from the later authority read. * fix(managed-agent): keep lockCovered's side reads off a single-connection caller lockCovered reads the alias list and the other aliases' authorities on a connection of its own, set to autocommit and read-only, so they see what is committed. On the offline recovery entry the DataSource is a SingleConnectionDataSource, whose every connection is the caller's own: the read committed the caller's work and the next FOR UPDATE failed in a read-only transaction. Both reads now go through committedRead, which reads on the caller's connection there and on an independent one otherwise. StorageRowLockOrderMySqlIT records what each second transaction queues on and requires the alias index, covers an alias committed after the recovery's snapshot, and runs lockCovered on a single connection inside a transaction that then rolls back. * test(managed-agent): pin clearFence's alias index and the offline write StorageRowLockOrderMySqlIT now drives a migration abort on MySQL and checks that the UPDATE clearing the fence plans through uk_ma_storage_business_key, the index owner() already locked the row through. The single-connection case asserts the caller's write matched its row before relying on the rollback discarding it. The alias-list comment in lockCovered says what the single-connection entry reads. |
||
|
Shaojin Wen
|
acea5b1335 |
docs(design): correct the daemon PR state transitions design (#16487)
Fold the unposted findings of #16246's final docs-only review, and what the audits of those fixes turned up, into both language versions: - v0.26.8 (2026-11-05) ships the watch:* keys of #16258, so they follow the same deprecation window as prWatchEvents. - The view identity check normalises both URLs as pullRequestKey does (any path after the number and the port ignored) and compares host and number only. - Latency is stated as nominal (one interval, or ceil(n/30) for a view set of n), with the cases that make it longer. - The shared template contract moves key and template validation, chain resolution (event vocabulary and comment lane as parameters) and rendering from variable values into channel-base; %sender% is GitHub, %repo%/%url% keep the winner's bound URL spelling. - Every watch-file change reads and writes within one hold of the per-PR lock; a daemon claims the watch only when its refresh timer runs and its uid owns the watch directory and files, so writes stay temp-and-rename; a non-owner writes nothing. - The alias guarantees, the untrusted-primary exposure and the validation plan are updated to match; the Chinese twin carries the technical templates verbatim. |
||
|
Shaojin Wen
|
301b61ddcc |
fix(channels): deliver plain conversation comments to PR-bound sessions (#16416)
* fix(channels): deliver plain conversation comments to PR-bound sessions With deliverToBoundSessions on, a plain comment on a bound PR still had to pass the repository's mention requirement before it reached the session. On threads GitHub lists as a mention, an assignment or a review request, the comment lane dropped it whatever the setting. On subscribed and similar threads the bound path's preflight rejected it, and the workaround, turning requireMention off, also made the channel answer plain comments on unbound PRs and issues there in public. Threads the bot account opened or commented on were not affected. The mention requirement keeps ambient chatter out of public replies; it is no authorization boundary. The private path now waives it through a preflight option that admits a message whose only group rejection is the mention requirement. The waiver covers a comment, not the PR's own first-contact description, from a sender the repository's allowlist names, in a repository the group gate admits, unless the sender is in operators: a bound session usually acts on GitHub as an operator's account, and its own comments would come back to it as turns. Sender, repository, pairing and message-route checks still run, and repositories that admit any sender keep the requirement. On the mention-type threads the lane admits such a comment only when the private path would deliver it: the routes match and a target resolves for it as a comment event, so templates keyed by the thread's reason do not apply to it. Its task is marked private-only: it never takes the channel path or posts a public error comment, and it is retired when no bound session is left. Comments only the waiver admits are dispatched after the rest of the lane and the first-contact body, so their outcome, a deferral or a gone session evicted from the binding map, cannot change how those are handled. Two failure paths that predate this change no longer post a public error comment for a bound event when the task store fails: the gone-session deferral under a partial scan and the retirement at the deferral cap. * test(channels): pin the default route, audit refusal and window hold of waived comments - Admit an unprefixed plain comment through `defaultMessageRoute`. - Keep a waived comment's key out of the cursor when an unreadable publication audit refuses its dispatch, and deliver it once the audit reads again. - Feed the first-contact body on the re-read when a failed waived delivery holds the notification window. |
||
|
Shaojin Wen
|
3ec010e490 |
fix(web-shell): place the session search right of the activity toggle (#16481)
Follow-up to #16467: the search magnifier is now the rightmost control in the top row, immediately right of View activity, instead of sitting to its left. The open field still expands to the left of the bell, so the bell keeps its position while searching. |
||
|
Shaojin WenandQwen-Coder
|
dd4515ca27 |
feat(ci): isolate triage agent credentials and run all triage on ECS (#16468)
* feat(ci): isolate triage agent credentials and run all triage on ECS The triage agent carried a write PAT, so comment reruns and external fork-PR triage had to stay on ephemeral hosted runners while hosted capacity is tight and the ECS pool has headroom. Wire the token-isolation follow-up the workflow already documented: the agent reads GitHub with the job's read-only GITHUB_TOKEN and expresses every write as a draft; a separate publish job validates and executes drafts with the bot PAT, using one publisher script shared with local /triage runs. Contain the agent job, launch the CLI directly behind the hardened loopback model proxy, and move the deterministic maintainer resolver into the publisher. With no write credential in the agent environment, route every triage trigger to ECS behind the existing kill switch, and retire author_can_write. Markers, bot identity, and the finalize workflow's approve-on-green protocol are unchanged. Co-authored-by: Qwen-Coder <[email protected]> * fix(ci): bump triage CI budget and address triage review findings The new publish-triage job crossed the .ci-budget ratchet (7->8 jobs, 280->290 declared timeout-minutes), aborting Lint & Static before any lint or helper test ran. Bump the budget entry, mirror the artifact-name fallback expression on the upload side so an early-death run's drafts stay addressable, and enforce SKILL.md's process-label rule at the publisher chokepoint (welcome-pr, maintainer, help wanted, good first issue) with fail-closed tests, aligning the issue-workflow instruction that previously contradicted the rule. Co-authored-by: Qwen-Coder <[email protected]> * fix(ci): harden the triage draft pipeline after an adversarial audit Six findings from a multi-round pre-push audit, each with a mutation witness: kill lingering agent-user processes before the drafts strip/upload so a detached child cannot re-plant symlinks in the race window (upload-artifact follows them and RUNNER_TEMP is host-shared; unkillable survivors drop the draft set deny-by-default); lstat the draft containers themselves in the publisher preflight, since a symlinked bodies/ redirects the realpath containment base; reject cross-kind actions (review/close-pr on issues, close-issue on PRs); install util-linux for runuser as the sibling lanes do; make the mktemp'd proxy-wrapper directory traversable by the agent user so the gh/git wrappers do not silently fall out of its PATH; pin the artifact-name symmetry, the kill-before-strip ordering, and the toolchain install line. Design pair updated in both languages. Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin Wen
|
ad83860092 |
feat(web-shell): move session search beside the activity toggle (#16467)
* feat(web-shell): move session search beside the activity toggle The sidebar search lived in the Workspaces header, where it appeared only on hover and moved its input into a separate row. Put the toggle in the top brand row next to View activity, expand the field inline when the column is wide enough, and fall back to a full-width row under the brand row for narrow drawers and columns. Hide the control while the activity view replaces the session list, and keep the collapsed strip's search in its session popover. * test(web-shell): scope the drawer brand assertion to the logo The always-visible search toggle adds a second SVG to the sidebar brand row, so the drawer smoke test parent-level svg locator matched both the logo and the search icon and failed in strict mode. Target the logo own aria-hidden container instead. |
||
|
Shaojin Wen
|
478c2f87ab |
feat(ci): make flaky-patrol rerun and no_action decisions visible (#16459)
* feat(ci): make flaky-patrol rerun and no_action decisions visible The CI Failure Patrol posted only a hidden HTML marker when it re-ran a flaky failure or could not classify one, so maintainers saw a red PR with no explanation and clicked Update Branch — advancing the head, dismissing approvals, and re-running the whole CI for a failure that was already being handled on the same commit. Post a short bilingual notice alongside the marker for both decisions: the rerun notice states the transient cause and that no Update Branch is needed; the no_action notice asks for manual review before rerunning or updating the branch. The deterministic-failure comment is byte-identical, and the post-success reset marker stays hidden. Pin the visible notices, the marker round-trip through dedup and the per-head action count, and reason escaping for all three actions. * fix(ci): strip bidi overrides from flaky-patrol notice reasons An adversarial audit of the visible patrol notices found that bidi control characters (U+200E/U+200F, U+202A-U+202E, U+2066-U+2069) passed through the reason escaping untouched. The reason is classifier output derived from untrusted CI logs, and a right-to-left override can visually rewrite the very sentence maintainers trust when deciding whether to update a branch. Drop those characters in the shared escaping helper so all three visible decisions (and the deflake issue signature) are covered, and pin the stripping for each action. Removing the strip line fails the new test. * fix(ci): cover the Arabic letter mark in the patrol bidi strip An escalated audit round compared the strip class against the full Unicode Bidi_Control set and found U+061C (Arabic letter mark) missing; it can reorder adjacent Arabic-script text in a visible notice. Add it to the class and extend the per-action pin payload, so the class now matches the complete Bidi_Control set. Removing just U+061C fails the pin test. * fix(ci): collapse newlines in patrol reasons and pin the audit gaps The verification round found that a newline plus a line-start fence in a classifier reason opens a block-level code fence in the visible notice: the folded Chinese block and the tracking marker render as visible code. Collapse newlines and their surrounding whitespace to a single space in the shared escaping helper, keeping every reason single-line. Close the coverage gaps the same round measured: feed all twelve Bidi_Control code points individually, pin the English-before-fold and Chinese-inside-fold placement of the deterministic comment, and assert the fold's closing tag in all three notices. Witnesses: removing the collapse, narrowing the bidi class, dropping U+200F alone, swapping EN/ZH, or dropping the closing tag each fails the suite; a marked rendering probe shows the fence swallowing the fold before the fix and a clean render after; benign comment bodies remain byte-identical to base. |
||
|
|
ff775787f5 |
feat(channels): journal private bound-session delivery and scan health (#16237)
* feat(channels): journal private bound-session delivery and scan health The event timeline could not tell the private delivery lane's story. A prompt injected into a PR-bound session ended on the same `running` word as an event still in flight, no task row named the session it reached, and the three private watch retires plus the dead-letter left only stderr behind — so a silent consumption looked like a quiet repository. A bound event rejected by the channel's own gates was worse than invisible: its skip row was followed by running rows, reading as a delivery. The private delivery method returned a boolean meaning "settled", which conflated an injected prompt, a private retire and the public fallback. It now returns a verdict, and the task runner derives the journal's terminal word from it, so an event that was never delivered can no longer be recorded as delivered. Routing, fallback, retry-budget and cursor decisions are unchanged. The binding scan is a new hard dependency: while it has never succeeded the cold-start gate stalls every notification, yet the poll reports ok. Degraded rows now surface that beside the stderr lines the adapter already throttles, and on the edge of the existing watch-reconcile flag, so a permanent outage cannot flood the journal. Attaching this to the poll detail was rejected: both cold-start gates return before the detail is set, which is exactly when it would matter. No new event type and no protocol change — an outcome is already an open string. The dead-letter's message body and its failure-path payload dump stay out of the journal. Also labels `failed_ambiguous`, which was already written but rendered as a raw token. * fix(channels): make the bound-delivery journal claims true Review round 1 found sixteen places where the journal, its labels or its documentation promised something the code did not do. All sixteen are addressed here; none is deferred. Three were ordering or attribution defects in the new rows. A watch retire was journaled before the removal it describes, so a throwing removal left a terminal claim for an event the next poll re-runs and retires again under a different reason. Rows written while a delivery is cleared lost the session they were about, which is the one fact the private lane's attribution adds. And the pre-claim watch retire sliced the loaded state before saving with no rollback, so a failed save lost the event with no trace at all — the comment claimed the opposite. Two were counts that measured something other than their label: the partial scan published the routing map's size, which counts bound PRs, as a session count; and the streak advances per binding scan, not per poll, because a gone-session retry refreshes again inside the same poll. The session count now comes from the scan's own rows, and the streak is labelled as scans in both languages and in the stderr lines that share the counter. Re-cadencing the counter itself would change the throttle timing the previous PR shipped, so the label moved instead. Two were missing surfaces: the pending-queue-overflow retire wrote only stderr and the dead-letter file, and an unreadable watch state was reported as a pending backlog whose stated remedy cannot clear it. The overflow now journals a body-free skip, and the watch guard classifies a read failure separately while its gate stays fail-closed. The recovery signal the prose promised did not exist: a poll/ok row is written after every poll that does not throw, including all of an outage, so it cannot mark the end of one. The watch guard now journals its clearing edge, and the prose says why ok rows cannot serve. The rest were documentation and test-coverage corrections: the protocol reference promised detail.streak on every degraded row when only the binding-scan rows carry it; the design file map certified the protocol reference as unchanged while this branch edits it; the override's stated consequence was backwards; and five guards had no witness or only half of one. Each now has a test, and the suite grew from 438 to 444 in the GitHub package. Verified by 37 mutations, all killed. Two initially survived and were investigated rather than accepted: a loose sessionId count that three unattributed-row-free rows still satisfied, and a filter that matched no test after a rename. Both were tightened until the mutation died. * docs(channels): align the journal ledger with the round-2 verification The validation section still cited the round-1 mutation count after the review fixes added fifteen more, and the Chinese half had lost the count altogether during the rewrite. Both now state 37 applied and 37 killed. Also records why two mutations initially survived and what was done about them, since a sweep that cannot distinguish "no test ran" from "the guard held" is not evidence: a loose sessionId count that three correctly attributed rows still satisfied, and a filter left matching nothing by a test rename. The Web Shell line now mentions the absence coverage too. * fix(channels): keep the lost bound session named on every journal row The second review round of the bound-session journal found fifteen gaps. Three were consequential, and two of those were introduced by this PR's own previous round: The gone-session retry never updated the local delivery, so with two followers both gone the terminal row named the first one — a session evicted a step earlier — while the row above it named the last. An operator reading "which session lost this event" would open the wrong workspace. Remembering the target only for the transition that clears the delivery is not enough: once the clear is persisted, every later poll re-enters the absence gate with no delivery to read, and the dead-letter at the deferral horizon reads a record that has none either. A driven run of that sequence left 239 of 242 rows unattributed, with the terminal row naming nobody — the one fact this lane's attribution exists to supply. The lost target is now persisted as attribution only; the delivery is deliberately not restored, since that would re-enter bound delivery and change routing. The overflow retire was journaled before its queue shift was durable, with no rollback, so a failed save left a skip row for an event a restart would later deliver. It now saves before it journals and restores the shift when the save throws, matching the sibling pre-claim path. Also: the partial-scan session count now includes the sessions retention keeps routing (a draining workspace lists none while still delivering to three), and both pre-claim retires carry the frozen target so the timeline and the dead-letter agree about the same event. Four coverage gaps whose mutants left the whole suite green are closed: the guard keeping the partial-scan row off the cold-start path, the session on the no-candidate retire, a second healthy poll so the recovery row's edge trigger is distinguishable from an unconditional write, and both a shape test and a Chinese render test for the interpolating labels. The docs said more or less than the code: detail.sessions is carried by one row, not by every partial scan; the normative sessionId definition read a retire chip as a delivery; the emit-point inventory missed the overflow site; and bindings_partial was described as recovering implicitly when a successful multi-page scan is its producer. Auditing that last correction found the stderr census was still incomplete — the lane has five recurring health and capacity sites, not three — so it now names all five, ranks them by consequence and states its own scope. * docs(channels): define what the partial-scan session count actually counts The count became listed-plus-retained when retention was included, but three surfaces still described the old value. The protocol reference called it "the sessions the map still routes to", which is wider than the code: the map also skips channel-owned sessions and unparseable binding URLs. Both design halves still said it was "the scan's own row count", which is narrower than the code and was the exact wording the review asked to correct. Define it as a session-row count — the rows this scan listed plus the sessions retention keeps routing, bounded by the retention cap — and say explicitly that it is not the number of entries surviving the map's own filters. Also record that "listed" in the row label and in the stderr line is shorthand for listed-or-retained and that both print the same number, so the wording cannot quietly drift apart from the value again. Documentation only: the adapter and its tests are byte-identical to the previous commit, so every runtime result reported for this branch still applies. * fix(channels): close the review gaps in the private-lane journal Address the eight open suggestions and the five deferred findings from the round-3 review, each with a mutation-verified guard (10 new mutations, 10 killed; one cap-count fixture initially survived because the first empty partial already capped lastBindings, and was rebuilt so retention is above the cap exactly at the emitting scan). - R1-15: the overflow dead-letter append and the sidecar save now fail independently. A failed save rolls the shift back, names the watch-state path instead of blaming the dead-letter file, drops the payload dump (the body is already durable), and remembers the head so the next detection skips the re-append. The save-throws test now drives a second detection and asserts exactly one dead-letter line. - R2-6: the pre-claim skip comment and both design languages now say only the overflow retire writes a dead-letter record; watch_no_candidate and no_role_matched leave the skip row as the event's only trace. - R2-3: bindings_cold_empty no longer claims implicit recovery — an empty partial is a successful scan, the streak resets only on a complete or non-empty snapshot, and past the escape the silence is the throttle maxing out, not recovery. - R3-1: a 1100-session fixture pins detail.sessions to the 1024 retention cap at the emitting scan; swapping retainedCapped.length for retained.length reds it while the two sibling count tests stay green. - R3-5/R3-6/R3-15: the design's outcome cell lists failed_ambiguous and degraded, the watch-guard bullet admits the read-failure error field, and the sessionId carve-out covers every row written on a clear plus the pre-claim skip; the chip sentence says reached-or-lost. Both languages. - R3-7: the three private-retire watch tests assert the public path stayed empty; leaking the envelope through handleInbound reds them. - Deferred: the gone-horizon attributes the in-memory task before retiring and the dead-letter record names the lost session; the watch-guard row re-journals when the blocking cause changes; the subscriptions-off recovery edge has a test; the recovery row renders Recovered/已恢复 instead of Degraded (wire outcome unchanged); a gone-retry fixture pins the scan-scoped streak at 10 inside poll 4. Verified: channels/github 595 passed, web-shell channels 343 passed, eslint and prettier clean; design mutation ledger updated to 47/47. * fix(channels): rework the overflow retire ordering and close round-4 review Critical R4-1: the overflow branch saved the sidecar mid-pass, committing the advanced baseline before the pass's later events were queued; a crash then never re-derived them. Remove the mid-pass save entirely — the head is sliced in memory only, and the baseline and full queue commit together in the single end-of-pass save, with the skip rows journalled after it. The dead-letter record is now the retire's commit point. A lazily loaded, file-backed key set (restart-durable, replacing the process-local map) skips the re-append (R4-16), and both drains recognize a dead-lettered head and complete its retire instead of delivering it, so no event is both delivered and filed as retired undelivered (R1-15 follow-up). Also addressed: - R4-12/13/14: terminal, transition lost-session and pre-claim rows carry sessionWorkspaceCwd so Open-session reaches a session in another workspace. - R4-11: the no_role_matched comment states the real ordering — the dedupe cursor is in memory until the base saves it, so the skip row is at-least-once. - R4-15: already_delivered_other_role is documented as a defensive, unreachable guard and dropped from the published vocabulary. - R4-2/3, R3-5: §1 outcome cell adds suppressed/posting, reason covers task and poll rows, sessionId carve-out adds cancelled and distinguishes live running rows. Both languages. - R4-4/5/6: the pre-claim census acknowledges the role-aware drain's pre-existing fourth retire; the retire-carries-session rationale notes the preflight exception; the dead-letter write-failure note names the non-recoverable public-suppression exception. Both languages. - R2-3/R4-7: cold-empty reset is keyed on the routing map and trust flag, and the escape's lane-open claim is qualified for the refresh-failure producer. - R4-8/9: the recurring-site census adds the credential/rate-limit pass abort and corrects the no-existing-flag claim for the stale-map refresh failure. - R4-10: watch-guard error wording covers reconciliation failure too. - R4-17/18/19: openTarget joins the interpolation guard with a ZH render assertion, the EN streak label says consecutive, and ZH degraded/retired/ recovered labels get render assertions. Verified: channels/github 601 passed, web-shell channels 331 passed, eslint and prettier clean; 4 new mutations killed (mid-pass save, file-dedupe, drain guard, workspace lookup); design ledger updated to 51/51. --------- Co-authored-by: verify-round <verify-round@local> Co-authored-by: verify-pr <verify@local> |
||
|
Shaojin WenandQwen-Coder
|
3663209c3d |
chore(ci): shorten stale PR window to 30 + 14 days (#16435)
* chore(ci): shorten stale PR window to 30 + 14 days The open PR backlog keeps growing, and the 60 + 30 day window only reaches PRs inactive for two months, leaving the 30-60 day band untouched. Lower the stale threshold to 30 days and the close window to 14 days, and raise operations-per-run to 300 so the first sweep can label the backlog in one or two runs. The stale/close messages and the fleet-shepherd design docs are updated to match. Co-authored-by: Qwen-Coder <[email protected]> * docs: keep the stale-window reference in the design current Avoid pinning the note to a merge-date guess: name the policy change instead, and reflow the paragraph to the file width. Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin Wenandverify-round
|
e9a20d6ed7 |
fix(core): keep session list titles when a transcript is rebuilt (#16299)
* fix(core): keep session list titles when a transcript is rebuilt The sidebar label came only from the transcript: a custom_title record or the first user prompt. A transcript recreated without its head (missing file recreated on the next append, crash truncation, external cleanup) lost the prompt, so displayName became empty and the Web Shell rendered the session id prefix. Persist the list label to <sessionId>.title.json next to the transcript: - the first user prompt (source "prompt"); - custom titles from ChatRecordingService and SessionService.renameSession (source "manual"/"auto"). Reads keep the transcript authoritative and fall back to the sidecar. Archive, unarchive, delete, and storage relocation move or remove the sidecar with the session. Web Shell rows now render "Untitled session" / "未命名会话" instead of an id prefix. Design: docs/design/session-title-sidecar.md * fix(core): keep the list-label sidecar best-effort Write it after the transcript append and swallow its errors, so a partial Config or an unwritable home can never drop a user message. Update the deletion-cleanup ownership assertion for the extra title-sidecar check. * fix(core): align the title sidecar with the list reader - Seed the sidecar when a transcript is resumed (custom title first, then the recorded first-prompt display text) so sessions created before the sidecar existed gain one without another prompt. - Write the sidecar when forkSession commits a titled fork. - Store the prompt's displayText when present and share the reader's 200-code-point-plus-ellipsis truncation instead of a UTF-16 slice. - Pin the intended behaviour with tests for a text-less first prompt, resume seeding, and a titled fork; update both design documents. * fix(core): close the review findings on the title sidecar - Seed a resumed session with its actual first prompt: track that text outside the 20-entry title window and fall back to the record's text part when no display projection exists. - Never replace a persisted label from a later prompt. This also lets an image-only first prompt leave the slot open for the next text prompt. - Truncate and validate the sidecar name by code point so an astral prompt cannot store a lone surrogate or a discarded ellipsis. - Reconcile the title sidecar in both archive/unarchive conflict-resolution branches, where the hand-rolled sidecar set previously left it orphaned. - Update the workflow-mode e2e locator for the untitled row label, and cover the primary and fanout rows, archive moves, and storage relocation. - Correct both design documents: the list reads the sidecar when either the title or the prompt is missing, and a wholesale chats-directory clear also removes the sidecar. * fix(core): keep sidecar-backed session names consistent Review round follow-ups for the session title sidecar: - wrap the archive/unarchive conflict reconcile so a sidecar IO error no longer reports an already-completed transcript move as failed; - also fall back to a manual/auto sidecar in findSessionsByTitle, the picker prefix scan, and the unarchive collision guard, so a name the sidebar shows can be found with --resume and cannot collide unnoticed; - converge the live write and the resume seed on one label policy that mirrors the list reader (explicitly empty projection, Mod prompts); - stop a projector-restored session from being labelled with its first post-resume prompt, which is not the session's first prompt; - copy the source sidecar into a fork made without an explicit title; - return no name instead of another project's prompt text when the transcript is refused for project membership; - keep the id in the fan-out tooltip and the read-only row's accessible name, where it is not the visible text. Tests cover each fix, including the archived half of the lifecycle and a writer/reader pair on one filesystem; design docs updated in both languages. --------- Co-authored-by: verify-round <verify-round@local> |
||
|
Shaojin WenandQwen-Coder
|
36c8b34222 |
perf(core): load the MCP client's Google auth providers on demand (#16242)
* perf(core): load the MCP client's Google auth providers on demand mcp-client imported GoogleCredentialProvider and ServiceAccountImpersonationProvider statically, so every process that loads the MCP client, including each resident Managed Runtime worker, also loaded google-auth-library whether or not any server uses Google authentication. createTransport is already async; import each provider only for a server configured with it. Add transport tests for the service-account impersonation provider, which had none in this file. On Linux x86_64 an idle worker loads 0.24 MiB less JavaScript and holds 0.9 MiB less private memory. * fix(core): ask for a restart when a Google auth provider chunk is missing Now that the MCP client imports its Google auth providers on demand, the bundle puts them in separate chunks. A background update that replaces dist under a long-lived process made the next connection fail with "Cannot find module", wrapped as a connection error for the server. Move the restart guidance that contentGenerator already gives for its lazy provider chunks into utils/errors (getModuleNotFoundError, wrapLazyModuleLoadError) and apply it to both provider imports. The original error stays as the cause. Add google-auth-library to the ACP static closure guard, so a static import of either provider fails check-serve-fast-path-bundle again. * test(core): isolate the lazy Google auth chunk tests in their own file Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin Wen
|
a32646e640 |
docs(design): propose PR state transitions from the daemon PR refresh (#16246)
* docs(design): propose PR state transitions from the daemon PR refresh The daemon's PR refresh already sweeps every bound PR with the host's gh credential; the GitHub channel polls the same PRs again to detect CI failures, merge conflicts, head changes and merged/closed transitions for bound sessions, with its own credential, schedule, CI definition and state machinery, and only where such a channel is configured. Propose observing head, CI and mergeability in the refresh, keeping the snapshot and pending transitions in the PR sidecar, delivering each transition into the bound session through the existing prompt admission and resume paths, and removing the channel watch in a second PR. * docs(design): keep per-head CI history and daemon-wide recipients for PR state events Two inconsistencies with the channel watch the proposal replaces: - A single ciFiredSha lost earlier heads, so a force-push back to a head whose CI failure had already fired would fire it again. The sidecar now keeps a bounded ciFired ring (16 heads per PR), the daemon's equivalent of the channel's durable fired ring, and the acceptance criteria pin the revisit case. - Recipient selection was per workspace, so two registered workspaces for one repository would each deliver to their own bound session. The sweep that detects a transition now selects one recipient across all trusted workspaces, dispatches through that session's runtime, and writes the snapshot and fired record to every sidecar bound to the PR so sibling sweeps observe no transition. * docs(design): give prWatchEvents a deprecation window now that v0.26.7 ships it The proposal justified rejecting prWatchEvents at connect() with "no tagged release contains the channel watch". That premise is stale: #16117 merged on 2026-11-03 and v0.26.7 (2026-11-04) ships and release-notes it. PR B now keeps accepting the field for one minor release (one warning naming sessionPrWatch, no watch pass), and the following minor release rejects it; acceptance criterion 3 and the files table follow. The refresh-interval note no longer claims a one-minute floor: sub-minute values fall back to the default and 0 disables the sweep. * docs(design): reconcile sibling PR snapshots before detection Propagating a snapshot to every sidecar bound to a PR is a sequence of separate file writes, so a crash between the recipient's write and the sibling writes left a sibling one observation behind; its next sweep re-detected the same transition under a fresh key and the recipient was admitted twice. Detection now takes the newest sibling snapshot (by a new observedAt field) as its baseline, so a lagging sibling catches up silently and the original pending key is the only one for the episode. The durability section lists this fourth crash window, and the validation plan and acceptance criterion 5 pin it. * docs(design): version slim-listing state observations for baseline recovery The newest-sibling baseline only worked for observations made by gh pr view: merged and closed states come from the slim listing and left observedAt unchanged, so after a crash between the recipient's write and the sibling writes the two sidecars carried the same version and recovery depended on enumeration order. Every observation that changes the watch block now stamps observedAt, the block carries the state seen at that version, and the baseline is the whole block of one observation. The validation plan covers the terminal-state crash case. * docs(design): PR-scoped pending, provenance-bound admission and sweep budgets Folds in the findings of the docs-only review on |
||
|
Shaojin Wen
|
ad487ffedb |
feat(autofix-pr): give helper refusals a code and a next step (#16238)
* feat(autofix-pr): give helper refusals a code and a next step
A helper that refuses now exits 1 with one JSON line last on stderr —
{"refused":"<code>","next":"<step>","message":"…"} — instead of throwing
to the CLI's critical-error handler, which printed the error's stack. Each
yargs handler catches the error and prints the line, as review's
extract-step does. Anything that is not a coded refusal is `failed`.
The step depends on the helper as well as the code: `watched_elsewhere` is
`take_over` from `prepare` and `end_task` from a round's helper,
`worktree_dirty` is `discard` from `sync` and `needs_human` from `push`,
and a `reply` that failed is `retry`, as its second run skips what the
first one posted.
The skill acts on `next` through a new "Refusals" section instead of
matching the wording of messages, and the autofix_pr tool returns the coded
line first, verbatim, then what the helper printed before it. Design
section 6.16 lists the codes.
A clone with no directory for the watch's lock (its watch files removed)
is refused as `not_prepared` instead of with ENOENT.
* fix(autofix-pr): code the refusals whose step the caller needs
- A reply plan that is not JSON or not shaped as a plan is `plan_invalid`
(`fix_plan`), not `failed` (`retry`): retrying it unchanged fails again.
- A switch back that `sync --discard` asked for and git refused is
`failed`: as `worktree_dirty` it answered `discard`, which asks again.
- The skill's Resume step keys on the code `pr_closed`, since `prepare`
gives it `report`; `retry` holds wherever a step says a refusal ends it;
refusals with no coded line are the calling step's to handle.
- The tool takes the last coded line of stderr, not the last line, and a
refusal's message keeps at most 8,000 characters, its head and tail.
- Each helper's code is pinned through the command in an empty clone.
* fix(autofix-pr): stop a watch git cannot switch back, and fit the coded line
- `sync --discard` that git cannot switch back to the watch's branch
answers `synced: false`, as a worktree still dirty after discarding
does, so the round stops instead of reporting the same refusal at every
fire.
- A reply plan that cannot be read is `plan_invalid` as well.
- A refusal's message drops terminal controls and is cut to 8,000
characters as JSON, not as text, so the line fits the stderr the tool
keeps.
- The tool reads a coded line only from a helper that exited 1, and only
when nothing but Node's warnings follows it.
- The skill's `discard` step tells Round step 1, where it clears what an
earlier round left, from the push of step 6, where the round's own fix
left the worktree unclean.
* fix(autofix-pr): answer the first review of the refusal codes
- A switch back git refuses because the branch is checked out in another
worktree names that worktree and how to free it: `cleanup` cannot
delete the branch either while it is held.
- A message drops escape sequences whole and the daemon's log-unsafe
class (C1 controls, bidi overrides, invisible formatters), keeping tabs
and newlines.
- A defect, not a refusal, prints its stack frames before the coded line.
- The coded line survives the frames `--trace-warnings` adds after it.
- `fix-start` refuses a fix in flight with the message that says how it
ends.
- The tool's description states the result's prefix, and `partial` is
described by the deadline that raises it.
- Tests: the whole step matrix as a specification, `cleanup` and `status`
through the command, the order of the watch checks, prepare's `failed`
arm, the line within the stderr tail the tool keeps, and the skill's
steps read from the helpers' source with what each does.
* test(autofix-pr): pin a C1 control a refusal's message drops
* fix(autofix-pr): say how to free a branch a stale or locked worktree holds
When `sync --discard` cannot switch back because another worktree has the
watch's branch, the reason now fits what holds it:
- a worktree whose directory or `.git` was deleted by hand: drop its
record with `git worktree prune`, since `git worktree remove` refuses
one whose `.git` is gone;
- a locked worktree: unlock it first, since neither `remove` nor `prune`
touches a locked one.
The test runs each case's git commands as the reason gives them and
checks that the next round gets through.
The every-helper refusal test now joins its fake `gh` to PATH with
`path.delimiter`, and its `prepare` row refuses before calling gh. The
fake records its calls and the test asserts there are none, because on
Windows `execFile` would never run the script.
* fix(autofix-pr): quote the holder path in the recovery commands
The `git worktree remove` and `git worktree unlock` commands that a
switch-back refusal prints interpolated the holder's path bare. A path
with a space split into two arguments (git exits 129), and a quote broke
the shell, so the branch stayed held. Both commands now quote the path
with `shellQuotePath`, the form review's printed repair commands use.
The holder test now runs every case at a plain path and at one with
spaces and both quotes. It runs the printed commands through `sh -c`
instead of splitting them on spaces.
* fix(autofix-pr): answer the second review of the refusal codes
- The tool runs its helpers with NODE_NO_WARNINGS=1. Node flushes a
warning raised while a helper refuses after the coded line, and a
warning's text can run over several lines, which hid the line from
`codedRefusal`, so the code and its `next` were lost. A new test runs a
helper that raises such a warning: run directly, the helper prints the
warning after the line; run through the tool, its line is read.
- The switch-back reason's git commands carry `-C` with the watch's
worktree, so they run as printed from any directory. A branch held by
the clone's own checkout gets only "switch that checkout", because git
never removes the main worktree.
- `worktreeHolding` reads `git worktree list --porcelain -z` (git 2.36+)
and falls back to the line form on older gits, so a path with a
newline in it is no longer cut short.
- New tests: the holder matrix also runs at a path with a newline and
runs its commands from outside the clone; the main-checkout holder;
and the no-holder arm (an index.lock stops the switch), whose `cleanup
--force` advice nothing reached before.
* fix(autofix-pr): keep a watch whose branch another checkout has
When `sync --discard` cannot switch back because another checkout has
the watch's branch, it answers `synced: false`, and the skill stops the
watch with `cleanup`. `cleanup` removed the worktree first and then ran
`git branch -D`, which git refuses while that checkout has the branch.
The watch was left with its state file and branch and no worktree, and
every later `cleanup` and `prepare` failed at the same delete.
`cleanup` now checks `git worktree list` before it removes anything,
`--force` or not. When another checkout has the branch, it keeps all
three and ends the watch, as it does for unsaved work. It answers
`removed: false` with `heldBy`, and a reason that says how to free the
branch and to run `cleanup` again. The holder lookup is shared with
`sync` (`branchHolder`) and anchors its commands at the clone root.
The skill's Stop step and both design docs say what `heldBy` means.
|
||
|
Shaojin Wen
|
a7d0193991 |
fix(core): keep a writer grant an in-flight renewal refreshed (#16287)
* fix(core): keep a writer grant an in-flight renewal refreshed reacquireWriter() waits for an in-flight writer renewal before it acquires, then always dropped the grant and posted the acquire. When that renewal had answered with a live lease, the fresh grant was thrown away; until the acquire answered, authorizeLifecycle and any caller reading the grant without waiting in ensureWriter refused locally with "the HTTP Managed Session writer is not active." A Store holding the Session's lifecycle can hold that acquire, so a Hosted Harness refused lifecycle phases during Spring recovery. HostedShellLifecycleRecoveryIT [spring-kill] failed this way in 17 of 239 CI runs since #15813 made renewActivation reacquire a lapsed writer. Keep the grant when the awaited renewal moved the lease past the lapsed one and the new lease is live. A refused renewal, or a commit that only copied the lapsed lease, still reacquires. * test(core): pin the lease check and renewal timer a kept grant relies on Two cases join the reacquireWriter tests: - A refused renewal is held while, after a wall-clock step back, a journal read copies the lapsed lease into a new grant object. The writer must still be re-acquired. Only the check that the lease moved forward guarantees this; a check on grant identity would keep the copy. - The grant is kept, then the activation renewal meets the lifecycle claim fence. The timer that the kept grant's renewal re-armed must still send the next ordinary renewal before that lease ends. The reverse case now reaches the lapse before the wait, so the live check has to read the clock after it. The comment in reacquireWriter names the journal read among the paths that copy a lapsed lease. |
||
|
Shaojin Wen
|
b3ad01e875 |
perf(managed-agent): index the activation lease lookup (#16336)
* perf(managed-agent): index the activation lease lookup Each activation renewal reads the Session's current activation row, three times a minute for every resident Session. Renewals update that row in place, so it is as old as the activation, and the lookup walked back over every journal transaction committed since: 22 rows after one Turn, 717 after 40. Add a generated is_activation_record flag for the two activation operations and an index that leads with the Session, its activation epoch and the flag, and look the row up by the flag. A simple CASE keeps H2 from compiling the expression into an IN set bound to the session that created the table. The CSI application image builds its expected journal row column by column, so it sets the flag the way the database does. A MySQL integration test pins the lookup's plan and result with 1,000 and 10,000 later transactions. * test(managed-agent): pin the CSI image's mirror of the activation flag The CSI application image rebuilds the journal row a cut adds column by column, so it computes is_activation_record in Java as the generated column does. Nothing held the two together. - WorkspaceCsiApplicationImage.ACTIVATION_OPERATIONS names the operations the CASE marks, and V1 points back to it. - ActivationRecordFlagTest parses the journal table's CASE and requires exactly those names and ELSE 0, then checks on H2 that the generated value matches the mirror for every name the CASE mentions and other operations the Session Store commits. - The CUT-image test asserts the added row carries "false" for the cut's releaseActivation. |
||
|
Shaojin Wen
|
76c692815f |
perf(core): read each Managed message body once per process (#16337)
* perf(core): read each Managed message body once per process A Hosted Turn rebuilds the Session's history from its message bodies several times; every rebuild read every body through the Session Store again, so Turn t made about 8·t resource reads, each an HTTP call and about five SQL statements on the control plane, serially. Keep the bodies a projection has read in one byte-bounded LRU shared by the process (64 MiB). Bodies are immutable and keyed by their store and full durable ref, so a hit is exactly what the store returned; only reads fill it, so a body never read still fails the projection when it is missing. The bound keeps resident Sessions from each holding their whole history. * perf(core): keep a Session's own message bodies when it outgrows the cache A projection reads every body of its Session in commit order, so with one LRU over all stores a Session whose bodies exceeded the bound evicted each body just before its next projection reached it and never hit, and flushed the other Sessions' bodies on every pass. The cache is now kept per resource store, with the stores ordered by when they were last read. A store never evicts its own bodies to keep a new one: an oversized Session keeps the bodies it kept first and reads the rest from the store. Room for a new body comes from the other stores, least recently read first, so contending Sessions re-read only what the others displaced. Adds cases for that, for the shared default cache, for two concurrent misses of one ref, and for counting a chunked body by its assembled bytes. * perf(core): bound the message body cache by the memory it holds - Charge every entry its body plus a fixed overhead (768 bytes, measured at 520 to 740 on Node 22), so many small bodies cannot hold several times the bound. - Keep a body that is a view of a larger buffer in an exact-size, unpooled copy, so a small body from the HTTP store does not pin an 8 KiB pool slab. - Release a store's entries once the store object is collected. - Once keeping another of a store's own bodies would take them past the bound, the store only uses free room: taking room back gained it nothing and made the other Sessions re-read their whole history. - Pin each key field, the production wiring, concurrent misses, assembled-byte accounting, the boundaries and the policy in tests, and document what a cache hit no longer checks. * test(core): pin that a cached message body outlives its store copy A cache hit does not read through the store, so a body already read is still served after the store loses it, while a projection with a fresh cache fails. The behaviour is deliberate and documented on the cache; the case keeps a future change from restoring the store read silently. * docs(core): say what a cached message body means for readers with their own store The resource store's read contract does not change: readers that open their own store read it on every call and fail on a missing referenced body, while a projection whose cache kept that body keeps serving it until the entry is evicted or its store is collected. Losing such a body is damage; nothing removes it while the Session can be read. |
||
|
Shaojin Wen
|
5d45b42940 |
feat(verify-pr): give the sandboxed round the PR's review threads (#16300)
* feat(verify-pr): give the sandboxed round the PR's review threads The sandboxed /verify round had no token and no review context. Its only inputs were the metadata snapshot and, on a follow-up round, the previous report. On #13652, Critical R1-26 was confirmed and then marked fixed three days before a round that published merge-ready without mentioning it. Eight hours later the review bot, at the head that round had certified, still could not rule out its residual. The verify job's trusted resolve step now writes review-threads.json beside pr.json. It holds every review thread, resolved ones included, since resolving is the author's claim, together with each reviewer's standing CHANGES_REQUESTED review. It comes from one paginated GraphQL query reshaped with jq. The snapshot is best-effort: on failure no file is written and the step warns. verify-pr-prepare.mjs sends the same query and writes the same shape for the local path. It removes a previous copy first, so a failed fetch cannot leave a stale file. A test runs the workflow's block verbatim and prepare's transform over the same pages and requires equal output. The skill adds report item 4, Existing review findings. Each term the verdict turns on is computed from snapshot fields: - a thread is blocking when its first comment is tagged Critical, Blocker, P0 or P1 by an owner, member or collaborator; - a blocking thread is open when it is unresolved, or when that comment belongs to a standing change request, resolved or not. That is the rule GitHub's review decision applies. An open blocking thread left unmeasured rules out merge-ready, and with nothing failed the verdict is inconclusive. Findings at fail 0 would publish as "passed with non-blocking findings". Other blocking threads are re-measured as budget allows, and the verdict line counts those left unmeasured. Change request bodies are listed but do not hold the verdict, because their severities are prose. The duty is also a line in the scope budget. A missing file reads as unknown, never as "nothing open". The file adds 2396 bytes, so the size baseline records the exact 378242. * test(verify-pr): pin the review snapshot's jq guard, copy order and odd answers - The CI/local parity test runs the step's real jq program. Like the file's four other jq cases, it now skips when jq is missing, instead of failing one assertion later with a message that names neither jq nor the missing tool. - The local recipe must clear the cached review-threads.json before it copies prepare's file. The test pinned the three lines but not their order. With rm after cp, every local round would read the review state as unknown. - prepare's two other failure arms are now driven through the CLI: an answer that is not JSON, and pages without a pull request. Each must leave no file and say why. An empty state there would read as "nothing open". * test(verify-pr): drive the review snapshot through the shared gh harness The review-snapshot case had its own copy of the executed-step harness: a temp dir, a stub gh on PATH and GitHub's bash flags. makeGhHarness exists so that fidelity contract lives in one place. The factory now records every call's argv, returns its temp dir, and has an empty-stdout arm for paginated reads, GH_STUB_EMPTY_LIST. The snapshot case runs through it. Its failure arms are the factory's GH_STUB_FAIL_LIST and GH_STUB_EMPTY_LIST. Deleting the shared stub's fail arm, its empty-stdout arm or its argv capture turns the case red, and so does recording only the first argument. |
||
|
Shaojin Wen
|
5239d69cf1 |
fix(channels): keep GitHub inbound recovery armed after a failed read (#16402)
A corrupt or unreadable inbound task store at connect and at the first poll's recovery latches the in-memory recoverable-task count at 0. The recovery flag was cleared in a finally, so a later poll never re-ran recovery once the store became readable: the persisted task never ran, and its cursor-blocking state kept the notification window held. Clear the flag only after a successful recovery pass, matching the watch-reconcile guard, so a failed read keeps the retry the log already promises. |
||
|
Shaojin Wen
|
cc78e35d5f |
perf(serve): keep the ACP spawn channel out of Managed Runtime workers (#16235)
The worker sizes media responses with DAEMON_ACP_NDJSON_LIMITS, which managed-tool-media imported from acp-bridge's spawnChannel. That module pulls in the ACP SDK schemas, the acp-bridge status module and, through it, core's package root, so every resident worker loaded about 1.5 MiB of modules for one constant. Define the constant in the dependency-free ndJsonStream module, re-export it from spawnChannel, and import it from there. On Linux x86_64 an idle worker drops from 91.0 to 79.6 MiB of private memory and from 215 to 175 loaded modules. |
||
|
Shaojin Wen
|
ef6914c1d9 |
fix(channels): keep GitHub replies hit by a secondary rate limit (#16334)
* fix(channels): keep GitHub replies hit by a secondary rate limit A final reply that GitHub rejected with a secondary rate limit was classified as an ambiguous delivery failure: it was not retried, not kept as a pending delivery, and no error comment was posted, so the answer was lost while the notification was marked read. GitHub sends secondary limits with quota left (and not always with retry-after), so only the exhausted-budget shape counted as a definite no-write. Treat a 403/429 that carries GitHub's rate-limit headers and names a secondary limit as a definite no-write too. The reply is retried in process and, if the limit persists, kept as a pending delivery for the next channel start. Responses without GitHub's rate-limit headers (for example a 429 from a proxy) and permission 403s stay single-shot. * fix(channels): wait out GitHub secondary limits before retrying A secondary rate limit without retry-after fell through to the generic 1 s / 2 s backoff, so the final reply's three attempts landed within about three seconds while GitHub was still throttling the account; GitHub asks for at least a minute in that case, longer on each repeat, and warns that continued requests can get an integration banned. Wait 60 s and then 120 s for such a limit; retry-after and the budget reset still take precedence. Note in the classifier why a retry-after header alone is not proof of a rejected write there. * fix(channels): stop the pending GitHub retry pass on a rate limit Now that a secondary limit counts as a definite no-write, the retry pass on channel start spent three attempts on every pending reply while the same account-wide limit answered each of them; stop the pass at the first rate-limited record and keep the rest for the next start. Pin the retry count and waits per secondary-limit shape, the older abuse-detection wording, and that a 502 carrying an exhausted-budget header stays single-shot. Reword the classifier comment: the header shows a rate limiter answered, and GitHub's wording tells which one. * fix(channels): end GitHub rate-limit waits on stop, keep cleaning pending replies - disconnect() used to cut a rate-limit wait short and let the remaining attempts go out back to back into the same limit. A stopped wait now gives up with GitHub's last answer, so a final reply stays a definite no-write and is kept as a pending delivery. - The pending pass stops sending at the first record GitHub still rate-limits (either limit) but keeps dropping later records the audit already shows as posted, which needs no request. - The PR watch pass treats a 403 carrying GitHub's secondary-limit wording as shared, like the publication and notification paths. - A retry-after that is not a positive number of seconds (an HTTP date, "0") no longer skips the secondary-limit minimum. * fix(channels): treat retry-after on a GitHub-answered 403/429 as a rate limit The final reply's definite no-write required an exhausted budget or GitHub's secondary-limit wording, so a rate-limit rejection that carried GitHub's x-ratelimit-remaining and retry-after but worded its body differently was audited ambiguous and the reply was dropped. retry-after is GitHub's own documented signal for that limit; with the x-ratelimit-remaining header still required, a proxy's bare 429 stays single-shot. A 403/429 whose retry-after is unusable now waits the secondary-limit minute as well. Also pin the shape where GitHub rejects the reply once and accepts the retry. * fix(channels): stop GitHub retries by the error's shape, not the wait branch - The stop guard keyed on which wait was chosen, so an exhausted budget without a reset header took the generic 1 s / 2 s branch and, after disconnect(), still fired its remaining attempts into the spent budget. Rate-limit answers are now recognised by their shape (403/429 with an exhausted budget, retry-after or the secondary wording) for the wait, the stop guard and the definite no-write check alike; such an answer without a usable retry-after or reset waits the one-minute minimum. - x-ratelimit-remaining must be a count: an empty value no longer reads as an exhausted budget and turns a permission 403 into a stored retry. - The pending pass starts no further record once a stop is requested, even while another write in flight defers its abort. - Tests pin the stop guard for every wait, reconnecting, a kept record between the stop and a posted one, and an infinite retry-after. * fix(channels): let an answer that lands after a stop decide the pending reply - The start-up pass returned on an aborted signal before looking at the error. A stop defers the abort until the in-flight write ends, so a 502 or network failure from that write (GitHub may have posted the comment) left the record and its reply_pending task for the next start, which posted the reply again. The pass now classifies first; only an attempt that was never sent leaves its record, and githubApi marks that abort as not sent. - A rate limit without GitHub's header (a 429 with only retry-after) is still dropped as ambiguous, but it now stops the pass like a definite one instead of spending every later record against the same limit. - x-ratelimit-remaining as a number must be a non-negative integer, and the watch pass judges a 403 with the same rate-limit rule as the reply. - A call that starts after the stop gives up before logging or sleeping. * fix(channels): treat a past GitHub budget reset as clock skew GitHub answering with an exhausted budget means its window has not reset, so a reset time that already looks past says the local clock is ahead; the wait fell to a 1 s retry and spent all three attempts in about two seconds. It now waits the one-minute rate-limit minimum instead. Tests give the secondary limit GitHub's usual reset header and pin the start-up pass keeping its records when a stop, deferred by another write, lets the wait run out. * test(channels): pin the ambiguous audit, the watch 429 abort and the quiet stop Lock three behaviours the suite let drift: a header-less rate limit on the final reply is audited failed_ambiguous (the key stays consumed), a bare 429 aborts the whole PR watch pass, and a reply that starts after the stop logs no retry it will not make. * docs(channels): name the header a GitHub no-write answer needs The no-write rule needs a numeric x-ratelimit-remaining on the 403 or 429, so a rate-limit answer without it (a 429 with only retry-after, say) is ambiguous and its reply is not kept; the guide now says so. The publication test helper's comment also said the opposite of what stopping the poll loop does to retry waits. * docs(channels): bring the GitHub publication contract up to the retry rules The contract still said a definite no-write goes straight to the pending file. It now names what counts as one, the two in-process retries and their waits, what an ambiguous answer to a retry does, the stop that keeps the reply pending, and how the start-up pass stops at the first reply GitHub still rate-limits. The first bullet now speaks of one final response rather than one delivery attempt. * docs(channels): add the Chinese version of the GitHub publication contract docs/design/README.md asks a change to a single-language design to add the missing version. The publication contract gains its zh-CN twin, reciprocal links under both titles, and a Verification line for the retry tests. |
||
|
Shaojin Wen
|
6611b8aeb2 |
fix(cli): land a Web Shell update on the current latest release (#16374)
* fix(cli): land a Web Shell update on the current latest release The daemon pins its update target when it prepares. Once the state is `ready` the check returns the cached status even for `?refresh=true`, so polling never re-downloads — and Web Shell prepares automatically on mount. For a long-lived daemon the prepared release is therefore whatever was latest shortly after it started. Clicking Update restarted into that stale target, and every release published in between needed another prepare-and-restart round, which reads as advancing one version per click. The CLI resolves and installs at each process start, so it always reached the current latest in one step. Re-resolve the release once at activation, after the restart has been acknowledged, and stage the newer one when it is higher and one-click covers the installation. Both the standalone and the managed npm paths share one resolver, so eligibility cannot drift between the check and the click. Readiness stays pinned and background polling still never re-downloads; only the explicit click re-resolves. A failed re-resolve or staging applies the prepared release instead, so a registry failure cannot strand an explicit update request on the running version. The activation guard still preserves an equal or newer active installation, and a failed activation of the newer release cleans both payloads. Staging runs after the acknowledgement and can take minutes, during which the daemon keeps answering. The browser now measures its give-up deadline from continuous unreachability rather than from the click: a reachable daemon is still working, and the old click-time budget would have reported a failure while the update was in fact succeeding. The protocol reference and both language versions of the update design doc state the pinned-readiness contract and the new deadline semantics. * fix(cli): keep an update restart observable and its payload committed An audit of the activation-time re-resolve found three defects in it. Discarding the superseded payload ran before the newer one was committed, so a failed discard fell back to a payload that might already be half removed and orphaned the one just staged, losing the update entirely. Commit to the newer payload first and discard the old one best-effort: a failed discard now leaks disk instead of losing the update. Dropping the browser's click-time deadline had also removed its only overall bound. A daemon whose process replacement does not land on the new build comes back reachable and settled on the same version, and the control then waited forever for a version change that would never arrive. Restarting is sticky until the process is replaced, so an acknowledged restart followed by any other state with an unchanged running version now terminates the wait and reports a failure. That path publishes the settled state itself, because the poll immediately after it can drop while the daemon is still coming back, which would otherwise leave a stale restarting status rendering no control at all. The rule is gated on the restart having been acknowledged: polling starts on the click, before the request resolves, and can still observe the pre-click state. Browser fixtures now model the daemon's contract — restarting from the acknowledged restart until the process is replaced — because a fixture that keeps reporting a prepared state after the click cannot exercise that distinction. Assertions are unchanged; only the stimulus became faithful. Also pin the edges the re-resolve depends on: a prerelease below the prepared release, a registry rollback, build metadata that does not change precedence and a malformed registry value must none of them restage, and an in-flight restart must survive the operator disabling automatic updates, concurrent polls and retries, and ordinary shutdown. Every mechanism added here is mutation-witnessed, including the two halves of the deadline change and the acknowledgement gate. * fix(cli): sample the restart acknowledgement when the poll starts The settled-state termination read the acknowledgement ref after awaiting the status request, which does not establish that the response was sampled after the acknowledgement. The status and restart requests are independent, so the first restart poll can be issued while the restart is still pending and deliver its pre-restart snapshot after the 202 arrives. The ref is then true, the stale snapshot looks like a daemon that settled without updating, and the control reports a false failure and clears the restart — after which a genuine version change is handled by the idle branch, so the page never reloads and the captured task URL is never restored. Sample the flag when the poll is initiated instead. A response delivered after the acknowledgement by a poll started before it is now ignored, and the next poll — initiated afterwards — is the first one allowed to conclude the replacement did not land. The regression test holds the status request open, acknowledges the restart, then delivers the old snapshot, and requires no error plus a successful reload once the new version is reported. Also restore an absolute backstop, which the move to a continuous- unreachability deadline had removed. A daemon that stays reachable but never leaves restarting — a hung activation or process replacement — would otherwise leave the control disabled forever with no error, and a page reload hides it entirely. The cap sits above the daemon's own staging ceilings (five-minute archive download, ten-minute managed npm child) so a slow but progressing update never reaches it, and a test pins both halves: the wait survives fourteen minutes of a reachable restarting daemon and ends at the cap. Both the protocol reference and the two language versions of the design doc state the sampling order and the backstop. * fix(cli): log why a click fell back, and keep the entry after giving up Three review findings on the update restart path. A registry outage at click time left no trace. The release check resolves an error result rather than throwing, so the fallback returned silently and "the registry said nothing newer" was indistinguishable from "the registry was unreachable" — both install the older prepared release. Log the classified failure before falling back. The catch below only ever sees detection or staging throw, so it now names restaging instead of sending the reader to the registry. The test that covered this used a rejected check promise, a shape the real function does not produce for an outage; it now resolves the real error result and asserts the log, and a second test pins that a staging failure is not reported as a registry failure. Giving up took the update entry with it. While the wait runs, each successful poll publishes the daemon's state, so a wedged daemon leaves it at restarting; the give-up branch cleared the restart without publishing anything, and the render gate does not display restarting, so the entry unmounted and could not return while the daemon kept reporting it. The user got one notification and then no affordance at all. Every give-up path — both deadlines and a daemon-reported error — now keeps a retryable entry on screen. The design sentence claiming the control is never hidden was untrue of these paths; it is now true of all of them. The wedged-daemon test asserted only that the spinner was gone, which a vanished control also satisfies, so it now asserts a visible enabled entry. The absolute cap was sized against a ceiling the code does not have. The standalone archive download tries OSS and then GitHub sequentially with a five-minute budget each, and the checksum and signature fetches add up to two more minutes, so one restage can spend about twelve minutes downloading and verifying before activation — not the five the docs and the constant's comment claimed. That left the fifteen-minute cap about three minutes of headroom while the text promised a slow update would never reach it. Raise the cap to twenty minutes and state the real chain in the protocol reference, both design docs and the comment. |
||
|
Shaojin WenandQwen-Coder
|
edb094fea3 |
test(managed-agent): gate lifecycle-v2 close with a stopped worker (#16123)
Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin Wen
|
481bd948de |
feat(web-shell): link channel event rows to their sessions (#16375)
* feat(web-shell): link channel event rows to their sessions Operators investigating a bound-session delivery or a published reply had to copy the session id out of the journal and find the session by hand. Journal rows that name a session now offer an "Open session" button on the Channel events page, reusing the existing in-app session navigation path. GitHub task rows previously journaled no session at all, even though the bound delivery target is known at every transition. They now record the target session and, from the binding snapshot, its owner workspace, so cross-workspace targets open correctly; outbound rows additionally stamp the channel's own workspace. The new journal/SDK field is optional: older records fall back to the journal's workspace, which is correct for the channel's reply session, the only session kind older records name. * docs: correct the locked-workspace claim in the session-link design The open-session handler's locked-workspace guard only applies to browser-notification clicks dispatched on a dedicated EventTarget; window-dispatched in-page links (markdown session links, and the new events-page button) are not subject to it. The design said the jump "honors locked-workspace mode" and that locked shells "ignore cross-workspace jumps" — both inaccurate. State the observed behavior: in a locked shell the button behaves exactly like an existing markdown session link. |
||
|
Shaojin WenandQwen-Coder
|
2feb29c856 |
fix(cli): stop Agent Host disconnects being undone and report unsaved joins (#16156)
* fix(cli): stop Agent Host disconnects being undone and report unsaved joins `DELETE hosts/connect` stopped the connection before removing its saved record. When the removal failed, for example on a held lock, the route answered 400 after aborting the running turns, and the restore loop reconnected the still-saved record a few seconds later. It now removes the record first and stops the connection only after that. A failed removal stops nothing: a held lock answers 503 for the caller to retry, and any other failure answers 400, unless the connection has no record at all (joined at boot, or a join that was not saved), which has nothing to come back from and is stopped anyway. `POST hosts/connect` swallowed a failed save of the connection record and answered as if the join would survive a restart. It still answers success, because the enrollment token is spent by then, but adds `saved: false` and `saveError`, unless an earlier record for the same connection still stands. `qwen agents join` and the web shell's add-runtime dialog warn about it instead of saying the daemon reconnects after restarts. The dialog's two join forms now have their own keys. React reused one form's inputs for the other, so an enrollment token typed on one tab showed up in a visible field of the other. Refs #16136 * fix(cli): tell an unreadable connections file from an empty one on disconnect Address review R1-1 (Critical) and R1-2/4/5/6/7/13: - R1-1: readAgentHostConnections mapped every non-ENOENT failure to [], so a disconnect whose removal failed (EMFILE/EIO/mode change) read as 'no record', stopped the turns, and let the restore loop reconnect the record still on disk. Split a strict reader (ENOENT is empty; unreadable/corrupt throws) from the resilient one the restore loop keeps, add findAgentHostConnectionChecked (found/absent/unreadable), and make DELETE stop only on a confirmed absent record, refusing on unreadable. - R1-2: widen the connections lock retry budget (~0.65s -> a few seconds) so transient contention is not reported as a permanent lost join. - R1-4: the restore loop's keep() now uses findAgentHostConnection instead of duplicating the still-saved predicate. - R1-5/R1-6: the add-runtime dialog clears the unsaved notice before join validation and on tab switch, so a stale notice cannot sit beside another form's error. - R1-7: pin a saved remote-connect control, the empty-saveError notice ('.' fallback), and the tab-switch clear. - R1-13: session-multi-agent design §3.5 now states reconnect is conditional on the record being saved (both languages). Tests: cli agent-host 30 pass, web-shell dialog 12 pass; typecheck, Prettier, ESLint clean. Critical and tab-clear mutation-witnessed (module lookup swallow and tab-handler revert each redden their pin). Co-authored-by: Qwen-Coder <[email protected]> * test(cli): pin the disconnect Critical with a real route + real store Add a real-scenario integration test: the real registerAgentHostRuntimeRoutes wired to the real agent-host-connections store over a real temporary filesystem (only the Agent Host client and program probe are substituted). A readable record is forgotten and stopped; a file made unreadable (a directory standing in for EMFILE/EIO) makes DELETE refuse with 400 and never stop, so the restore loop cannot undo the disconnect. Mutation-witnessed end to end: swallowing unreadable as absent reddens the real-route test. Co-authored-by: Qwen-Coder <[email protected]> * fix(cli,web-shell): keep Agent Host joins through a slow save Found by running two real daemons (a coordinator and a Host) on one machine against faults on the Host's connections file, and in the second review round. Connections file: - The lock used proper-lockfile's own retries, about 3.75 s in all, for every error. A holder killed while it held the lock (SIGKILL, OOM) leaves it until it goes stale, 10 s later, so a join that met it answered `saved: false` and a disconnect answered 503. Writes now retry only while the lock is held or changing hands (ELOCKED; ENOENT; EPERM and EBUSY on Windows), for up to 12 s, and fail at once otherwise. `stale` stays at the default, now spelled out, so daemons of other versions agree on it. - Writers in one daemon take turns, so two of them cannot both take over the same stale lock; one still queued 1.5 s past its deadline gives up. A writer whose lock was reported lost, or that held it for 8 s, writes nothing, and a lost lock is logged instead of taking the daemon down. - The file is read strictly inside the lock: a read error (EACCES, EMFILE, EIO) or a file a newer version wrote fails the write instead of replacing every other record. A corrupt file is still replaced. - DELETE `hosts/connect` also answers 503 for a lock lost before it wrote, and 400 with the reason for a file a newer version wrote, saying that the connection ends when this daemon stops. Deadlines: the save can now wait about 13.5 s, so the coordinator gives a remote's `/connect` 90 s instead of 20 s, the web shell gives remote-connect 120 s and its own join 90 s instead of the SDK's 30 s, and `qwen agents join` waits 90 s instead of 60 s. A CLI join that timed out, or whose connection dropped after it was sent, now says the join may still complete instead of saying that no daemon answered. Add-runtime dialog and Agents page: - Each machine's "not saved" notice has its own slot and shows only with the form or result it is about, so a tab switch neither clears it nor moves it under the other machine's form. A join's error likewise stays with the join form; one a sent join came back with also shows, labelled as this computer's join, over another machine's connected result. A close drops only the notices and errors on screen. - A join or remote connect in flight keeps the dialog busy across a close and a reopen. Meanwhile the page keeps New agent, Role templates, Mention in chat and creating an agent on a runtime unavailable, since leaving it would drop the result. Adding and replacing a runtime use separate dialogs, so a replacement does not remount a dialog whose join is in flight. - A connected result waits while a request is in flight instead of replacing the form that sent it, and a close keeps one that a joined result covered. - A sent join clears its token, so a spent token cannot be sent again, and a join that timed out says it may still be joining. The form that sent a request is read-only until it settles. A plain-HTTP link off loopback without "Allow plain HTTP" is refused before it is sent, as the daemon would refuse it, so its token stays. The design doc now defines `saved` by outcome. Refs #16136 * fix(cli,web-shell): say why a disconnect stops nothing, and what helps From the third review round. Connections file: - Reading it now says why there are no records: a read error, a file a newer version wrote (ENEWERSCHEMA), or a corrupt file (ECORRUPT). - The restore loop reads a newer version's file as none and says so once per file, instead of logging it as unreadable every 5 s. - A disconnect refused because the file cannot be used now says why and what helps: the read error itself instead of "Retry the request.", which does not help a permission problem; for a corrupt file, "Fix or remove that file, then retry." and why forgetting the connection failed; and for a newer version's file, that the connection ends when this daemon stops. A corrupt file is still not taken for "no record": a real run showed that a file read through a mount being replaced comes back empty, and the restore loop reconnected from it once the mount was gone, undoing the disconnect. `qwen agents join`: the daemon answers 2xx only once the join went through, with `connected: true`, so any other 2xx (cut off, not JSON, JSON `null`, no `connected`) now says the join may still complete, worded by whether the answer was cut off or did not confirm the join. A JSON `null` answer no longer crashes it. Agents page: while a join or remote connect from the runtime dialog is in flight, a status line under the title says why the page's actions are disabled. Tests: the remote-connect double answers each leg as its handler does, and the real-store disconnect case checks that the record left the file. * fix(web-shell): keep a remote's notice, and the join command made meanwhile From the fourth review round. Add runtime dialog: creating a join command while a remote connected from "I have its address and token" had not shown up yet replaced that wait. Once the remote showed up, it had lost its "could not save the connection" notice, and closing its result threw the join command away. - The wait keeps the remote by the workspace it answered from (`workspaceCwd`, which the roster also reports for its runtime), and a join command created before the remote shows up keeps it too. Only a runtime serving that workspace is the remote and carries its notice. While the dialog waits for the remote alone, no other computer is the one it waits for. With a join command pending too, results come in roster order. - Closing a result keeps waiting for what is left: the remote, after another computer's result; the join command's computer, after the remote's, unless the command expired and nothing else came. - An external runtime online before the connect that serves the same workspace may be the remote, back under its saved id. Then nothing waits on for it. - An answer without a workspace (an older daemon) is read as before. - A remote connect drops a join command shown before it: the coordinator keeps one enrollment token, and the connect issues its own, so that command no longer works. Tests: the real-store disconnect case checks, through the strict lookup, that the record left a file that still reads and that another workspace's record stayed. The lenient lookup reads a corrupt file as no record. --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin Wen
|
bff144a528 |
fix(channels): keep GitHub notifications unread on transient thread errors (#16333)
* fix(channels): keep GitHub notifications unread on transient thread errors A notification whose comments, events or issue could not be read was skipped while the same poll still marked every notification read and advanced the cursor, so a GitHub 5xx, network error or rate limit on one thread lost its @mention for good, without a timeline entry. Keep the cursor and the unread mark when a thread fails transiently, so the next poll reads it again. Permanent answers (404, a SAML-protected 403) still skip the thread at once, and a thread GitHub keeps failing is skipped after 30 polls so it cannot hold mark-as-read back for every other thread. First-contact issue fetches follow the same rule, and every such failure is recorded in the channel event timeline. * fix(channels): keep exhausted GitHub retry streaks until the window commits Dropping a thread's failure counter when it reached the cap re-armed the same unread notification whenever another thread still held the cursor: two threads that started failing one poll apart kept resetting each other and the window was never marked read. Keep the exhausted streak until the window commits or the thread succeeds, so the hold ends once every failing thread has recovered or been skipped, and say so in the docs. * fix(channels): separate shared GitHub limits from per-thread failures Treat a rate limit or credential failure on one thread as what it is, an answer every thread would get: hold the window and stop reading the rest of the poll, without charging the thread's 30-poll budget, so a limit is not hammered once per notification every poll. Only a 5xx (Octokit also reports network errors as 500) counts against a thread's budget. A local exception without a status skips the thread again, as before, which keeps the inbound-task persistence guards observable on their own. A held window re-reads failing threads every poll, so report a streak in stderr and the event timeline when it starts, every tenth retry and at give-up instead of on every poll. Pin the shared-limit stop, the local-exception skip, the reset after a success and the clear on commit. * fix(channels): bound the GitHub notification hold per window, not per thread - A success reset a thread's 5xx streak while the window stayed held, so two threads failing in turn held mark-as-read and the cursor forever. The 30-poll budget now belongs to the window: on the 30th poll that sees a 5xx, the threads still failing are skipped and the window can be marked read. Rate-limited polls do not spend it. - A held window re-reads every thread and comment on each poll. Permanent failures and given-up threads are journaled once per window, rate limits on their first and every tenth stopped poll, and the duplicate and mention_required skips of a re-read comment once per window. - Tests stub the first-contact issue fetch they fall through to, so they no longer pass only in file order. * fix(channels): spend the GitHub 5xx budget only on a window that can commit - A thread left unread at the cursor (its mark-as-read never applied) with nothing newer to mark read held nothing, yet each failing poll spent the window budget, which then never reset; the next new notification was skipped on its first failing poll and its comment lost. Only a window with something to commit now spends the budget. - An error a failing task store raises inside a thread is journaled as retried, not skipped: the store failure holds the whole window and the thread is read again. - Pin the 500 boundary (Octokit's network-failure status) and the stderr throttle; list github_api_error among the Channel events skip reasons. * fix(channels): read only failed or changed threads while a GitHub window is held - A held window re-read every listed thread on every poll, so one failing thread cost a read of the whole window per poll, and a secondary limit set off by that volume stopped every poll at the same thread: the window could neither commit nor reach the notifications behind it. A thread read without error is now remembered with its notification's updated_at until the window commits; it is read again only once new activity moves that timestamp, so a hold re-reads the failing threads and a poll stopped by a rate limit resumes where the last one stopped. A failing task store or audit keeps the thread unremembered. - A notification listed again at or before the cursor (its mark-as-read did not apply) has nothing new to read; a failure reading it no longer holds the window or spends its budget. - The once-per-window journal keys stop growing at their bound instead of clearing, so only the overflow repeats. - Tests pin the budget surviving a failed commit, a 403 whose only signal is retry-after, and two repositories with the same issue number. * fix(channels): journal each GitHub thread outcome under its own window key - A retried store failure, a stale notification's skip and a real skip shared one once-per-window key, so whichever came first silenced the others: a thread could be skipped, and its comment lost at the commit, with the timeline still showing only the earlier row. Each outcome now has its own key. - Only a local error (no HTTP status) counts as held by a failing task store or audit; a 404 on another thread in that poll is a skip. - A permanent answer or a stale notification reads the same next poll, so the held window remembers it; a thread it gave up on is still read again. Stale failures no longer count toward the thread's failure milestones, and the per-thread counts share the per-window bound. - A stale notification that hits a rate limit still holds the window (the threads after it were not read); the doc now limits the stale rule to 5xx. * fix(channels): read cleanly read GitHub threads again on every held poll Skipping a thread that read cleanly while its notification timestamp stayed put lost data: the comment lane defers a first-contact body to the re-read after a failed comment turn, so the body was never fed; and with GitHub's one-second timestamps or a late notification, an unchanged updated_at does not prove the thread has nothing new, so a comment posted after the read was missed before the commit. A held window is read in full on every poll again, as on main; only threads that failed with a permanent answer or were stale, which read the same next poll, are not read again until the window commits. * fix(channels): read a GitHub thread again after an error without a status A thread whose read ended in an error without an HTTP status was settled for the held window like a permanent answer. Such an error can come from the channel's own state part-way through the thread: an unreadable pending-deliveries file throws after the first comment was claimed, with no persistence flag set, so the later comments of that thread (or its comment lane after a direct dispatch) were never read while the claimed task held the window, and were lost at the commit. Only permanent HTTP answers and stale notifications are settled now; any other failure is read again, as on main. * fix(channels): read every GitHub thread again on each held poll Settling a thread for the held window after a failed read lost items main delivers. A stale notification is not empty: a late notification at the committed cursor's second can carry a first-contact body or a direct event, neither bounded by the cursor window, and its read can fail on one poll and succeed on the next. A permanent HTTP answer can be short-lived too, such as the 404 of an issue that was just created. Every thread of a held window is read again on each poll, as on main; a stale notification's 5xx still neither holds the window nor spends its budget. * docs(channels): say what the GitHub failed and skipped timeline rows mean The journal contract listed inbound rows as accepted or skipped only; GitHub thread read failures now add a failed row (the window is held and the thread is read again) beside the skipped one (the channel stopped holding the window for it). Name both in the protocol reference and the channel guide. * fix(channels): share the GitHub rate-limit test with the watch pass A secondary rate limit sent as a bare 403 (quota left, no retry-after) stopped the notification poll but not the PR watch pass, which kept issuing one rejected pulls.get per watched PR. The limit's wording now lives in the shared predicate, so both stop on the same answers. A failing thread whose count no longer fits the per-window bound was reported on every poll; it is now reported once per window. A test pins that the hold budget starts again after a restart, and the journal helpers in the test file share one reader. * docs(channels): say what a github_api_error row means, not only its cause A failed row also comes from a local error while channel state cannot be persisted, the journal schema now lists the reason on inbound failures, and a rate limit stops a held window's poll at the first refused thread rather than reading it in full. * fix(channels): report a GitHub thread its own task still holds as retried A local error inside a claimed task's run, such as an unreadable pending-deliveries file, leaves that task running: the cursor waits for it and the thread is read again, yet the timeline called the failure skipped. The poll now checks the thread's own tasks, as the store was last read or written, and reports such a failure as failed until the task settles. The restart case now saves the cursor and loads it in a fresh instance, as a real restart does, so a hold count kept in the cursor would fail it. * docs(channels): keep the task store wording for the guide's first hold exception * docs(channels): state the bounds of the GitHub held-window journal rows A skipped row for a notification listed again at or before the cursor normally loses nothing, since its activity was read before an earlier window was marked read. The design page now lists the held-window causes as examples, including an audit it cannot read and a failing mark-as-read. The once-per-window rows are tracked under up to 8,192 keys, which the guide, the protocol page and both design pages now say, and a credential failure is journaled like a rate limit. |
||
|
Shaojin WenandQwen-Coder
|
e4c570a387 |
feat(channels): edit bound-session action templates on demand (#16349)
* feat(channels): edit bound-session action templates on demand The Web Shell channel editor rendered every boundSessionActions key as its own single-line input — 80 rows for GitHub's closed vocabulary — with one dense paragraph as the only explanation and no way to enlarge the dialog. Operators typically configure one or two keys. Record fields now show only configured entries and add keys through a grouped picker whose options carry per-key descriptions; values edit in multi-line text areas with explicit delete buttons. GitHub's descriptor supplies the descriptions and role groups, mirrored through new optional option metadata on the channel-base and SDK descriptor types. The channel editor dialog gains a fullscreen toggle, and the templates field moves into the bound-sessions panel with the same delivery gating as watch events. Empty values stay editable in the draft and are filtered by the existing save serializer, so the draft/save contract is unchanged. Co-authored-by: Qwen-Coder <[email protected]> * fix(web-shell): generic record copy and smoke-tag the template editor E2E Review follow-up on #16349. The shared record renderer's empty state and add-picker placeholder were template-worded, which mislabels the options-less messageRoutes field; the defaults are now entry-worded and the GitHub/GitLab template fields override them through their field label keys. A new unit case pins the options-less path (generic copy, no picker, stored rows still editable). The new channels E2E case gains the @smoke tag so the CI smoke job actually runs it. Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin Wen
|
ef9c891b8f |
fix(ci): clear the pinned corepack cache before every install (#16351)
* fix(ci): clear the pinned corepack cache before every install The Linux test shards have been dying seconds into Install dependencies with MODULE_NOT_FOUND on the cached pnpm's bin/pnpm.mjs since #16328 pinned COREPACK_HOME to runner.temp (main runs 41876940141, 41889761157, 41927887452; six different self-hosted hosts). The pin assumed runner.temp is per-run, but self-hosted runners keep _work/_temp across jobs — the lint job's isolated-HOME step already documents this. corepack trusts an existing cache entry without re-verifying its files, so a partial pnpm tree left by an earlier job fails every later job on that host. Clear the pinned cache at the start of the shared install body in all three jobs that copy it, enforcing the per-run contract the pin documents. The guard keeps the reset a no-op where COREPACK_HOME is unset (test_build) or already run-unique (lint). The test step still reuses the cache install warms in the same job, and poisoned hosts self-heal on their next job without manual cleanup. Pin the reset and the install/test COREPACK_HOME match in the workflow meta-test, and bump the size baseline for the added lines. * fix(ci): scope the corepack cache reset to the shard-owned path Review found two over-deletion shapes in the nonempty guard: a runner that exports COREPACK_HOME would lose its entire shared cache in the build job, and hosted lint would lose the cache its pnpm-store step had just warmed, re-exposing the install to a registry outage. Match only the exact shard-owned path instead; the build and lint copies become unconditional no-ops and the three install bodies stay byte-identical. Add an execution test that runs the shipped guard fragment under a scrubbed environment across five cache shapes: it kills the dead-guard mutant the string pins survived, and a widened-guard mutant dies on the shared-cache survival cell. State the incident as observations with the corruption source undetermined, and correct the stale per-run rationale beside the pin. |
||
|
Shaojin Wen
|
8a7c0d17d8 |
fix(managed-agent): find the Flyway scripts in the module jar the server ships them in (#16175)
* fix(managed-agent): find the Flyway scripts in the module jar the server ships them in Since #15871 split the server into Maven modules, its Flyway scripts ship in BOOT-INF/lib/qwen-managed-agent-persistence-*.jar rather than in BOOT-INF/classes/db/. The upgrade command's check read only the classes, so on every archive since then its database row warned "cannot compare the Flyway scripts (no Flyway script found in ...)", and a release that changed or reordered a script the installed database ran went unflagged. migrationScripts() now also reads db/**/ at the root of each nested jar in BOOT-INF/lib/. Spring Boot stores those uncompressed, so their directories are read in place; a compressed one, which Spring Boot would not load, is skipped. A script keeps its path as its name wherever it ships, so one moved from the classes into a module compares as unchanged. The reference distribution workflow now reads the scripts out of the exact archive it built, so a later change of layout fails there rather than turning check's row into a standing WARN. * fix(managed-agent): read a Flyway script shipped twice with different contents as unreadable The reader keys the scripts on their classpath path, and the nested jars give that key a second source. Two copies of one path with different contents left whichever the central directory listed last in the map, so the database row compared one arbitrary copy, while the class loader would hand Flyway the other. Matching copies are still one script; differing ones now make the jar unreadable, which the row reports as "cannot compare the Flyway scripts". * fix(managed-agent): name both places that ship a differing Flyway script, and pin the local extra field - When one script path ships twice with different contents, the error now names the two places, BOOT-INF/classes or the nested jar, so the operator need not unpack the jar to find the second copy. - The test zips can carry a local-header extra field, as archivers add for timestamps or alignment, and the nested-jar case reads entries past one, so dropping that term from dataStart fails the suite. |
||
|
Shaojin Wen
|
b9e20830f7 |
fix(verify-pr): post flaky demotions on closed PRs and pin the blank class (#16135)
* fix(verify-pr): post flaky demotions on closed PRs and pin the blank class The closed-PR skip from #16050 also caught the flaky variants of the cancelled, failed-before-report and results-unavailable notices, and the report body of a round that wrote no report.md. Those render ❌ and, on a merged PR, name non-deterministic tests now on main, so they post unless this run's own report already carries the demotion. - publisher: the closed-PR skip leaves FLAKE_VERDICT=flaky alone, except beside this run's own report - publisher: the gates line says its tests are "not counted individually above", since each suite run now counts as one assertion (zh: 不逐条计入上面的断言) - checker tests: pin the blank class on their own (whitespace refused, U+0085 and U+FEFF accepted); before, only the shared parity table in the workflow suite caught a revert to trim() - SKILL: rewrap the over-long local-path line * fix(verify-pr): scope re-run advice to open PRs and pin the gate condition The results-unavailable notice can now land on a merged PR, where the verify lane refuses `@qwen-code /verify`: both variants now offer the re-run only while the PR is open. The flaky exemption keys on the gate verdict, which holds only because the gate step runs on a prepared round; a test now pins that condition. * fix(verify-pr): skip a flaky notice only beside a report that carries it On a closed PR, a flaky notice with no live status of its own to finalize is skipped beside this run's own report, as a duplicate of the demotion that report carries. That report can come from an earlier attempt whose gate did not diverge. A re-run attempt whose own live status is missing (that post is best-effort) then lost its ❌ to the run log, beside a report that never showed it. A flaky verdict now adds a verify-flaky marker to the header, and the closed-PR skip drops a flaky notice only when this run's own report carries that marker in its header. * test(verify-pr): pin the own-comment lookup's initializers The publish step runs under `set -u`, and a failed comment listing or bot identity lookup leaves OWN_ID, OWN_SUBSTANTIVE and OWN_FLAKY at their initial values. Dropping any of those three lines kept the suite green: no case made either lookup fail. On such a run the step would instead abort with "unbound variable" and post nothing. The upsert stub can now fail the listing or the identity lookup. Under either failure, a report posts fresh rather than patching a status it cannot see. A flaky notice on a closed PR also posts, because the lookup cannot tell whether this run's report already carries the demotion. Deleting any one initializer now fails the test on its own unbound variable, which the harness now prints with the failed exit status. |
||
|
Shaojin WenandQwen-Coder
|
9123d5ee04 |
fix(managed-agent): refuse every URL fetch refuses and say why a health probe failed (#16110)
* fix(managed-agent): refuse every URL fetch refuses and say why a health probe failed start and verify refused a listener URL only when URL.canParse rejected it, so a --server-url without a scheme (proxy.test:8443), with a user name or password, or on a port the Fetch standard blocks (6000, 10080) passed the check and then failed every probe until --timeout. They now refuse each of those before anything runs, and a test checks that the runtime's fetch rejects every URL the check refuses. A probe that is sent and still fails now says what it got: start's timeout names the last probe's answer and verify's health row names its own, such as "HTTP 503, status DOWN", "self-signed certificate" or ECONNREFUSED, where both said only "not UP". The listeners() comment names the reachesHarness option verify passes. * fix(managed-agent): refuse a Harness URL that is not http or https in upgrade start start checked the Harness URL with new URL() alone, so an opaque value such as proxy.test:8443 parsed with no host and no port: start then waited out --timeout for the Harness to listen on port 80 of an empty host. It now refuses any Harness URL that is not http or https before it starts a unit, as the preflight would fail it later anyway. The probe-answer test also runs the runtime's own fetch against a port nothing listens on, so the ECONNREFUSED wording comes from a real error rather than a constructed one. * fix(managed-agent): keep the endpoint's answer when the deadline cuts start's last probe short start's last health probe gets the 1 ms the deadline leaves it, so against a real endpoint it times out before any answer arrives, and the timeout message reported that abort instead of what the endpoint had been answering, such as HTTP 503, status DOWN. A probe that times out on a budget the deadline cut below the per-probe cap no longer replaces an earlier answer; one that runs out of its own 5 seconds still does, since the endpoint stopped answering. The test rig's fake fetch now awaits a response function, so a test can model an endpoint that answers late and honours the abort signal. Also pins the IPv6-zone diagnosis to a percent inside the brackets. * fix(managed-agent): treat a health body the deadline stalls as a cut-short probe too A health endpoint that sends its headers and then stalls the body made response.json() reject with the signal's TimeoutError. That branch did not report timedOut, so start's guard took the probe for an answer, and a probe the deadline cut short replaced an earlier HTTP 503, status DOWN with "HTTP 200, The operation was aborted due to timeout". The body-read branch now reports timedOut as well. The last-answer test gains two real-socket cases: a body that stalls after its headers keeps the earlier answer, and a refused connection after an answer replaces it, which pins the timedOut half of the guard. The design doc says when the 5-second limit counts as the probe's own, and that any other failure replaces an earlier answer. * fix(managed-agent): sanitize health text and align upgrade refusals Address review R1-1..R1-5 on the upgrade URL/health change: - R1-1: neutralize terminal control bytes (C0/DEL/C1 -> U+FFFD) in the endpoint-derived health answer before it is interpolated into start's timeout refusal and verify's health row, so a queryable endpoint cannot move the cursor, split a row, or write the clipboard (OSC-52). - R1-2: the Harness URL refusal reuses fetchRefusal when the value does not parse, naming an IPv6 zone exactly as --server-url does. - R1-3: start's timeout labels the retained value 'the last answer', matching the design doc, since a deadline-cut probe keeps an earlier answer. - R1-4: pin verify's URL refusal (listeners runs it with reachesHarness:false) with a test asserting it refuses before any probe. - R1-5: tear down the 'closes' test server via t.after so a failing assertion cannot hang node --test to the job ceiling. Tests: 63 pass; R1-1/R1-2/R1-4 mutation-witnessed (printable no-op, refusal gated on reachesHarness, zone reason reverted each redden their pin). Prettier and ESLint clean. Co-authored-by: Qwen-Coder <[email protected]> * test(managed-agent): exercise the health sanitization over a real socket Add a real-scenario pin for R1-1: a local HTTP server serves a status field carrying cursor moves, a newline and an OSC-52 clipboard write, and verify runs against it through the runtime's own fetch — the real health() -> printable() -> row() -> out() path, not the rig's fake. Asserts the printed health row keeps no control byte while the readable text survives. 64 tests pass. Co-authored-by: Qwen-Coder <[email protected]> * fix(managed-agent): mask a refused URL's password and name why verify's API reads failed Found on a real systemd deployment (real control plane, Harness, MySQL, TLS and nginx): - A refused --server-url was printed as typed, so a URL with a password, which fetch refuses, put that password in the output. The refusal now prints it as ***. - verify's session and turn rows still said only "fetch failed" where the health row above them named the cause, such as "self-signed certificate in certificate chain". API reads now take the same cause. - Text the API hands back (an error reason, a body that fails to parse, a Session ID, an event type) reached the session and turn rows with its control bytes; it is now neutralized as the health answer is. The design doc says the probe answer's control characters show as U+FFFD. * fix(managed-agent): show a health status that is no string as its JSON printable() ran String() on what the endpoint returned, and String() throws on a valid JSON object whose own toString is no function, such as {"status":{"toString":1}}. health() is not inside a try, so on a real deployment verify exited with "Cannot convert object to primitive value" before it printed its health row, and start's probe loop stopped at the first probe. A value that is no string is now shown as its JSON, which is also what the endpoint actually said; an API error reason gets the same. * fix(managed-agent): make the upgrade reports total and bounded, and refuse only what a phase dials Review round 5 traced its findings to two shared causes, fixed at the source rather than one site at a time. Endpoint text in the reports (printable, withoutCredentials): - printable() no longer throws on any parsed JSON value: a status nested deeper than JSON.stringify() recurses showed a bare "Maximum call stack size exceeded" from verify and left start's server running without its guidance; it is now shown as [unprintable]. - It also stops at 200 code points, cut after the control bytes are gone and never inside a surrogate pair, so a long status cannot scroll the report away. The item count on the session row goes through it too. - withoutCredentials() masks the whole user information, so a token in the user-name position is masked, and masks it textually when the URL does not parse. The Harness refusal masks its URL as well. URL checks (listeners, the Harness guard): - A phase is refused only over a URL it dials: start and verify probe the health endpoint, and only verify --tenant reads the API. An API on a port fetch refuses behind a separate management port no longer stops start. - The Harness URL is checked as the preflight checks it, an http(s) origin without user information, a query or a fragment, so start refuses it before it restarts a running Harness rather than after. * fix(managed-agent): take the Harness verdict from the preflight and check the API URL only when verify reads it - verify decides whether its session row reads the API before it checks the listeners, so a signed deployment, or --actor without a trusted actor header, is no longer refused over an API URL it never dials. - The configuration resolver now prints the preflight's own verdict on the Harness URL, read untrimmed as java.net.URI reads it, and start refuses on it before restarting the Harness: a host with an underscore, an empty user name or a padded value no longer restart the Harness only to fail the preflight. - withoutCredentials masks everything up to the last @ of a value the parser rejects, so a password holding / ? or # stays hidden. - printable counts code points in one pass and keeps only the head, so a huge endpoint answer costs no copy of itself. - The request an API error names is bounded like the rest of the row. * fix(managed-agent): print the Harness URL untrimmed, as the server parses it A padded qwen.managed-agent.harness.base-url fails the preflight, and the start refusal now shows the padding instead of a value that looks valid. * fix(managed-agent): judge the Harness URL only where the server reads it The resolver passed on the preflight's verdict on the Harness base URL with no regard to the Harness's state, so start refused a value that a disabled or pooled Harness leaves unread: the preflight skips its harness row before reading it, and the server never dials it. The verdict is now "skip" there. start refuses on the preflight's grounds (user information, a query, a fragment, or anything java.net.URI reads differently) only on a "false" verdict, and its own reasons only word that verdict. It still refuses a URL that is not http or https, since it waits for the Harness there. Without a verdict, as when the configuration does not resolve, the preflight judges the URL once the Harness is up. verify now checks the type of the values the API chose before using them. Building a route from a Session id, matching an event type, or reading an event sequence coerced the value first, and that throws on an object such as {"toString":1} or on an array nested deeper than the stack, so printable() never got to show what came back. A Session without a string id fails its row with that value, an event type that is not a string fails the turn row, and a sequence counts only as a number or a numeric string. The start tests now check every run's output for the password, not only the last run's. * fix(managed-agent): judge a pooled Harness URL, and read only lists from the API The previous commit gave "skip" for a pooled Harness as well as a disabled one, since the preflight skips its harness row in both before reading the base URL. A pooled server still reads that URL, though: each member's QwenHostedHarnessConnector is built from it and refuses it by the same rule as the preflight. The pool's refresh drops a member whose connector refuses it, so such a pool accepts no Session while the server reports UP. Only a disabled Harness gets "skip" now. A test pins the verdict to whether the connector builds, for a single Harness and for a pool. Since the verdict is now the server's rather than only the preflight's, the refusal says "the server does not take". verify reads Sessions, items and events only as lists. Data that is not a list fails its row and says what came back: iterating it would throw, or walk a string, and its length could be any value. A listed entry of 0, "", false or null fails the session row instead of being reported as no Session. An event sequence moves the cursor only as a whole number, and only forward: Number() reads "Infinity", "0x10" or " 4 " as numbers. The user guide now says that, when the configuration does not resolve, the preflight judges the Harness URL after the Harness starts. * fix(managed-agent): keep start's own Harness URL rules without a verdict When the configuration does not resolve there is no verdict. The previous commits then let start accept any http(s) Harness URL and left the URL to the preflight. A pooled preflight never reads it, though, and every pool member's connector refuses user information, a query or a fragment, so the pool would start and accept no Session. Without a verdict, start refuses those itself again, since the server refuses them wherever it reads the value. A "true" or "skip" verdict still asks nothing more, and the generic wording is kept for a "false" verdict. HarnessConfigurationTest now builds a pool through the real member factory and shows that the single base URL decides whether a member is accepted. The pooled half of the agreement test checked nothing the single half did not, so it is gone. verify now requires every page to carry its data as a list: a body that is no object, or one whose data is null or missing, fails the row instead of reading as no Session. A Session id of "", "." or ".." fails, since fetch resolves dot segments and an empty id reads the list. Only terminal: true ends a turn. The docs no longer say the server takes only an origin, since a path is fine. They say which component refuses the URL, and they word the sequence rule as the code applies it. * fix(managed-agent): sort the test's static imports, and refuse a --server-url query The Mockito static import sat between two JUnit ones, so the verification module's ImportOrder check failed. The static block is now sorted. start and verify append /actuator/health and the API paths to --server-url, so a query or a fragment there took the paths in. With http://proxy.test/?a the command probed /?a/actuator/health, and fetch dropped a fragment along with the path after it. Such a URL is now refused up front. A listed Session entry with no id is now shown as itself rather than as "undefined". A health answer whose JSON parse error quotes control bytes is pinned to come out sanitized. --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin WenandQwen-Coder
|
4d4e4fc9ca |
feat(channels): role-aware action templates for GitHub bound-session delivery (#16258)
* feat(channels): role-aware action templates for GitHub bound-session delivery Bound-session delivery injected a hardcoded generic prompt and routed each event to a single winner, so a PR followed by review, verify, and fix sessions in different workspaces could only ever reach one of them, with no declarative way to say what each should do. Bindings gain an optional closed-vocabulary role (review/verify/fix): the command-position backfill mints review/verify from /review and /verify-pr, fix binds explicitly through the metadata route (null clears), and a re-offer never overwrites an existing role. A new channel-level boundSessionActions record maps event classes (watch:<type> or notification reason, optionally role-qualified) to operator instruction templates rendered with GitLab-style %var% substitution into a labeled trusted block; substituted GitHub values stay sanitized, capped data. With the map configured, routing generalizes the base design's single-winner decision to one winner per role with fan-out across roles: per-(event, target) durable tasks, independent backpressure, role-scoped gone fallback, and public fallback only at zero live followers. With the map absent, every behavior and the generic prompt stay byte-identical (pinned by tests). Design: docs/design/2026-11-04-github-channel-bound-session-action-templates.md (EN/ZH); the base design's Key decision 4 carries a forward pointer. Co-authored-by: Qwen-Coder <[email protected]> * test(cli): expect the command-stamped role in backfill binding assertions The /review command now stamps role: 'review' on every binding form it mints (role-aware bound delivery). Four strict-equality assertions in the backfill suite predated the field: the Aone URL-form and unplanned-repair cases, the fork-url state-pairing case, and the capped-plan live-bridge sync projection. CI's sharded Test lane surfaced only the first before fail-fast skipped the remaining shards; the full local suites (backfill 177, server 1442, bridge 1043, transport 431, daemon-worker 182, session-list/pr-refresh 87) now pass. Co-authored-by: Qwen-Coder <[email protected]> * fix(web-shell): delete a record key when its editor row is cleared Addresses the triage review's record-renderer observation. The save serializer already filtered empty values (so no unsavable state existed), but the form state kept a touched-then-cleared key as "" until save; the editor now deletes the key on clear, matching what the upsert payload will contain. A new dialog-level test pins the end-to-end outcome for the 80-key boundSessionActions surface (and the GitLab record fields sharing the renderer). Also documents at roleByNumber that per-number uniqueness is guaranteed upstream by noteMention, so the Map constructor's last-wins semantics cannot override the first-wins command rule. Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): close the role-aware delivery review findings (C1-C5, D1-D3) C1: the first task of a multi-target comment dispatch now carries the full fan-out intent; recovery reconstructs missing siblings BEFORE applying the survivor's event-wide dedupe, so a crash between claims no longer loses the unclaimed roles permanently. C2: a durable bounded delivery ring (github-bound-delivered.json) records every private acceptance; no gone-fallback or untargeted recovery may route an already-delivered event publicly, even after every follower disappears and the daemon restarts. An unreadable ring fails closed for the public fallback. C3: delivery-time re-validation retires a role-aware comment whose role lost its match on a complete snapshot instead of injecting the obsolete role's instruction into the frozen session. C4: split-sidecar merge lets the newer half win on role, absence included, so an explicit role:null clear survives an archive/restore merge. C5: FIELD_LABEL_KEYS.github registers boundSessionActions, making the zh-CN label and description reachable. D1: the map-skip retire now requires a complete fresh snapshot; under a partial or failing scan the event defers as backpressure instead of being consumed on incomplete evidence. D2: comment-lane events resolve a lane class (comment) after the thread reason, so fix:comment catches comments delivered under mention/subscribed/author while reason-specific keys still win. D3: the worker accessor validates roles against the shared PR_BINDING_ROLES constant instead of hardcoded literals. Each correctness fix has a dedicated regression; C1/C2/C3 regressions were mutation-witnessed (removing the fix turns them red). Design docs (EN/ZH) and the user guide updated to the new semantics. Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): close the role-aware delivery re-review findings (C1 residual, R2-1..R2-4) Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): complete fanout slots durably and keep private retirement terminal (R2-2/R2-4 residuals) Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): make the fanout completion mark fail-closed with durable acceptance evidence (R2-2 write-failure gap) Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin Wen
|
8e9b429ff8 |
perf(managed-agent): index the Runtime slot lookup by isolation key (#16325)
The CSI marker probe runs on every ordinary Session write, including each activation renewal, and looks up the Session's Runtime slots by tenant and isolation key. No index covered that lookup, so every call scanned the whole slot table, and the fence's locking variant locked every Session's slots. Index ma_runtime_slot.isolation_key, as ma_runtime_binding already does. A MySQL integration test pins the probe's plans at 1,000 and 10,000 slots and checks that the fence leaves other Sessions' slots writable while still holding off a second slot for the fenced Session. |
||
|
Shaojin Wen
|
f217a8e5d9 |
fix(managed-agent): restore JournalCommit under its class-size ratchet (#16323)
#16284 pinned JournalCommit at 880 lines and #16275 grew it to 886, so every Java lane fails ClassSizeRatchetTest on main. The hosted-cleanup admission probe that #16275 added belongs with the cleanup bookkeeping it guards, so move it behind HostedCleanupReferences.containsCleanup and restore the class to its pin. Fixes #16321 |
||
|
Shaojin Wen
|
d4083f4c82 |
fix(serve): read PR sidecars directly for the bindings snapshot (#16241)
* fix(serve): read PR sidecars directly for the bindings snapshot GET /sessions/pr-bindings paginated the activity-ordered session list and had to flag every multi-page scan partial, so a workspace with more than one page of active sessions never produced a complete snapshot and the GitHub channel deferred every unmatched PR event to the dead-letter horizon instead of answering through the channel. List the workspace's *.pr.json sidecars once (the enumeration the PR refresh timer already uses), read each sidecar, and take attribution and activity from the session's persisted record, refined by the live session when it is live. partial now means an I/O failure only. Scan draining and transitioning workspaces too: their files stay readable and they still serve sessions. Exclude machine-owned agent task sessions explicitly. Update both design docs, the user guide and the adapter's partial-snapshot diagnostics accordingly. * test(serve): compare aggregated PR bindings as a set Directory listing order and the random temp directory names made the cross-workspace assertion order-dependent; CI sorted the uppercase suffix first. Compare the (workspace, session) pairs as a set. * fix(serve): harden the PR bindings sidecar scan Append bindings the bridge holds in memory but that have not reached a sidecar yet, so the window between the bridge update and the sidecar write (or a failed write) cannot read as an authoritative absence. Treat a transcript that exists but yields no records as unreadable (partial) instead of listing the session unattributed, skip an active sidecar left behind by an archived session, decide non-targets before counting failures so a machine-owned session's corrupt sidecar cannot make the answer partial, emit tombstone rows without the transcript projection, validate live timestamps before the recency comparison, and log per-session failures as one line per workspace scan. Remove the write-only prsReadFailed flag now that the snapshot reads sidecars itself, retire the last draining-workspace wording from the adapter's diagnostics, and state the real per-row cost and the reason for having no page cap in the design doc. * fix(serve): treat a failed live session list as a partial bindings answer The live list is the only source for a binding that has not reached a sidecar, so a failed read must make the answer partial instead of reporting it complete without those bindings. Merge bridge-held PR numbers into a listed non-empty row (a tombstone stays empty), decide hidden-catalog and archived ownership for empty sidecars too, compare activity timestamps with the shared chronological rule, and log a registry entry without a runtime like the other partial causes. Reword the adapter's partial diagnostics to name every cause and count listed rows, update the shared snapshot contract in channel-base, pin the corrupt-sidecar row's presence in the session-list test, and make the design docs' cost model consistent. * fix(serve): decide live-only binding ownership from the persisted record A restored agent task session carries no sourceType on its bridge entry, so the live-only branch read ownership from a field that is not there and emitted a machine-owned session as a delivery target in a complete answer. Read the persisted record first, as the sidecar loop does, and drop the isArchived guard the bridge never populates. Cap the bridge merge the way the session list does (append only while the persisted list is below SESSION_PR_LIST_LIMIT, then keep the newest) and log the no-runtime cause under test. Keep the adapter's partial hint firing while retention holds bindings an empty partial no longer lists, and stop blaming the binding lookup when the cold-empty cap was reached by successful empty partials. Name the live-list cause in the design docs' partial definition and state the two cost legs separately. * fix(serve): keep unknown sessions unlisted when merging bridge bindings A session whose sidecar or transcript could not be read is counted as unknown and left out of the answer, but the bridge merge re-listed it from its live PR subset, which made the adapter treat it as enumerated and drop the row it was retaining with the persisted numbers. Track the unknown sessions and skip them in the merge. Attribute live-only rows through the same resolver as sidecar rows so an unreadable transcript makes them unknown too, count live-only sessions in the diagnostic's denominator, and pin that transitioning and blocked workspaces with a runtime are scanned. Make the adapter's partial hint say the lane is deferred while the cold-empty gate holds instead of claiming issue events flow, and name the binding lookup when failed lookups seeded the cold-empty cap and a single empty partial lands it. State the live-merge rule and the real filesystem-leg ceiling (sessions ever bound) in both design docs. * fix(serve): decide binding ownership before any read that can fail A machine-owned session whose transcript or sidecar could not be read was counted as a scan failure before its ownership was consulted, which made every answer partial on behalf of a session that is never a delivery target. Resolve ownership from whichever record states it as soon as the id is known, and skip non-targets before the sidecar read. Key the bridge top-up by PR number and repository so same-numbered PRs from different repositories both survive. Log once per streak when the channel worker itself downgrades an answer by dropping malformed rows, and name that cause in the shared contract, the user guide and the adapter's hint. Make the failure-path cold-empty cap message say the lane stays closed while no snapshot has ever succeeded, and align the design docs' risk summary with the ever-bound cost ceiling. * fix(channels): attribute the cold-empty cap by counted failed lookups Follow-ups to the PR bindings sidecar scan review (#16241, round 6). - The GitHub channel's cold-empty cap line attributes the stall from the failed lookups counted within the streak. It used to read boundSnapshotFailureStreak, which any successful answer resets, so a run of failed lookups broken by a single empty partial was blamed on the daemon's storage. The majority cause leads and the other count is kept. The same line now says the worker reports how many rows it dropped rather than naming one. - The bindings route falls through to the live createdAt when updatedAt is present but unparseable, instead of discarding both. - Docs: the hidden-catalog "excluded without partial" promise is scoped to sessions whose ownership can be read; the retention rationale describes the sidecar projection topped up from the bridge and trimmed to the cap; the user guide's list of partial causes includes a chats directory the daemon could not list. * fix(serve): key the bridge top-up by PR number and ignore restore time - The bindings route tops a listed row up from the bridge by PR number alone, as the session list's merge does: the sidecar writers replace an entry by number when it is re-bound to another repository while the bridge keeps the replaced entry, so the repository-keyed merge returned both and kept delivering the replaced PR's events privately. - For a persisted session the live createdAt no longer counts as activity: the bridge stamps a fresh one on restore, so merely restoring an idle follower could change which session receives a PR's events. The createdAt fallback is reserved for a session with no persisted record yet. |
||
|
Shaojin Wen
|
13e65c1a6e |
feat(web-shell): link GitHub PR/Issue targets in channel events (#16226)
* feat(web-shell): link GitHub PR/Issue targets in channel events The channel events page showed the repository#number target of GitHub journal events as plain text, so operators had to copy the reference into a browser to inspect the PR or issue behind an event. Chat prompts and the PRs panel already link out; this closes the last unlinked surface. The URL is synthesized in the client from the channel config and the journal fields the GitHub adapter already records, so existing journal history becomes linkable without any daemon or journal change. The origin mapping mirrors the adapter's web origin derivation (github.com, GitHub Enterprise /api/v3, ghe.com data residency), and links pass the same safety and desktop-opener routing as chat links. Non-GitHub channels, removed channels, and malformed references stay plain text. * fix(web-shell): link GitHub events of channels inherited from user settings The events page resolved link metadata from the management listing, which is scope-local: a channel configured in user/system settings with cwd pointing at the project (the setup the GitHub channel guide recommends) has journal rows on the page but no listing entry, so its targets stayed plain text. The journal route now attaches non-secret link metadata (platform type and API base URL only) resolved from the merged settings the workers actually run with, filtered by the same cwd-ownership rule the worker grouping uses so one workspace never sees another's channel metadata. Resolution failure degrades to unlinked rows instead of failing the journal read. The page falls back to this metadata when the listing has no entry, and a source contract test now ties the client origin mapping to the adapter's so the deliberate mirror cannot drift silently. * fix(web-shell): make channel event link metadata safe and authoritative Review of the journal-metadata round found the default settings loader leaking the queried workspace's .env into the daemon's process environment (and able to rewrite a corrupt settings file) on a read-only polling route, and the client preferring the scope-local listing over the merged metadata even though system settings override workspace values. The route now resolves metadata through readSettingsSnapshot on the runtime's effective environment — side-effect-free, corruption throws into the existing degrade path — and the helper requires that view instead of defaulting to loadSettings. The page prefers the journal's merged metadata, keeping the listing as the older-daemon fallback, derives the displayed number from the thread exactly like the URL does, and strips trailing slashes from the base URL on both sides of the origin-mirror contract. The serve protocol doc now documents channelLinkMeta. * fix(cli): resolve channel link metadata against the boot environment snapshot The serve process-env guard flagged the journal route's live process.env fallback: a read-only polling route must not couple to shared mutable process state. Thread the daemon's frozen boot-time environment snapshot through the route deps and use it as the parent-process fallback, the same source the session host resolution already uses, instead of reading the live environment or adding a guard exception. * fix(web-shell): emit the resolved GitHub web origin from the daemon Review round 2 flagged that the web-origin mapping had two owners pinned only by a source-scraping mirror test, and that the boot-environment guard had no test able to fail. Retire the second owner: the channel package now exports resolveGithubWebOrigin as the single mapping owner, the adapter and the journal route both use it, and the route emits the resolved origin as channelLinkMeta.webOrigin so the browser only concatenates. The client's baseUrl derivation remains solely as the older-daemon/listing fallback, and the scraping contract test is replaced by a mapping-owner table (including whitespace-padded env-resolvable values), route origin assertions, and emitted-origin consumption tests. Pin the boot-snapshot guard behaviorally: mount() accepts a daemonEnv override and a new case resolves channels from a boot home that disagrees with the process environment — reverting to a live process.env read now reds that case. Also call the exported resolveChannelOwnerCwd instead of re-implementing the ownership predicate, and correct the protocol doc's read-scope wording for the added settings snapshot. |
||
|
Shaojin Wenandverify-round
|
e2d3243863 |
fix(web-shell): express the PR watch events dependency in the channel editor (#16236)
* fix(web-shell): express the PR watch events dependency in the channel editor The GitHub adapter rejects a non-empty prWatchEvents unless deliverToBoundSessions is enabled, but the editor never expressed that dependency: operators could tick watch events, press Save, and receive a raw English daemon refusal that read as "the selection cannot be saved". Group the bound-delivery switch and the watch-event checkboxes into one framed panel, disable the checkboxes with an inline hint until the switch is on, and mirror the runtime rule as a client-side validation backstop for hand-written stored configs that carry events without delivery. * fix(web-shell): keep stored watch selections clearable while delivery is off Review follow-ups on the bound-session panel: - A stored selection stays uncheckable-while-gated no more: with delivery off, selected events (including unknown stored values) can be cleared directly, while new selections remain disabled, so turning delivery off never strands the operator between a dependency error and disabled boxes. - Turning the delivery switch on clears the stale requiresBoundDelivery error instead of leaving the instruction under the now-on switch. - The disabled hint moves into FieldShell's existing hint slot, restoring the "description is the first paragraph" invariant; the hint gets an id, aria-describedby on the checkbox group and a persistent polite live region so screen readers hear why the boxes are gated. - The render gate now carries the same github scope as the validation mirror, and checked-but-disabled rows keep the selected border. - Tests: shutdown path, stale-error retirement, aria association, and a stored delivery-on mount; visuals scenario seeds the bound-session fields and captures the gated and enabled states. * fix(web-shell): scope dependency-error retirement to the dependency message The switch-on clear deleted the whole prWatchEvents error slot, wiping the higher-precedence invalidOption message while an unknown stored value was still ticked; and unticking the last selected event — the other remedy — left the dependency alert over an empty, valid selection. Retire only the dependency message, from both remedies, and pin the live region's persistence (empty region mounted before the hint, same node across the transition) plus the hint-less rendering of the pre-existing hint slots. * fix(web-shell): retire watch-events errors by re-deriving validity Round-4 review follow-ups, all in this PR: - R3-1: replace the string-matched dependency clear with a per-field re-validation on edit, so unticking an unknown stored value retires the invalidOption instruction it satisfies (and a remaining valid event swaps the slot to the still-unsatisfied dependency message). - R3-3: submit now scrolls the first erroring field into view even when it is not an advanced field; a stored watch-events refusal below the fold is no longer invisible. - R3-2: pin the empty-panel guard with EN/zh absence assertions on a non-GitHub descriptor. - R3-4: table-driven parity test feeds the plugin's own descriptor and config through both the editor mirror and the daemon validator (test-only devDependency, never bundled); a lone change on either side now fails the table. Each fix carries a mutation witness; 286 channel tests green. * fix(web-shell): re-derive only shown errors, in both switch directions Round-5 review follow-ups, all in this PR: - R4-1: refreshErrors re-derives only the error keys already on screen, so a remedy retires exactly the instruction it satisfies without surfacing validation the operator never submitted (submit stays the only population site), and the delivery switch re-derives in both directions so a shown instruction is never silently dropped. New tests pin pre-submit silence and shown-message survival across switch toggles. - R4-2: narrow plugin.management once at module scope in the parity test so the optional contract member stays TS18048-clean if test files ever join the package typecheck, with a clear error instead of a load-time TypeError. - Deferred note from the same round: pin the data-disabled styling hook on gated rows (dimmed) versus clearable stored rows (selected styling). Each fix carries a mutation witness; 288 channel tests green. --------- Co-authored-by: verify-round <verify-round@local> |
||
|
Shaojin Wen
|
815c1d3768 |
feat(web-shell): collapse channel editor tuning behind advanced settings (#16239)
* feat(web-shell): collapse channel editor tuning behind advanced settings The shared channel editor renders every descriptor field flat, so a fresh GitHub instance shows roughly sixteen controls and the fields needed for first setup are buried under rarely changed tuning. Move the tuning fields into one collapsible advanced panel and hide the default message route until a route exists, keeping the default view to identity, credentials, conversation isolation, and access control. Collapsing never hides state: the panel opens automatically when any advanced field is configured, shows a count badge while collapsed, and forces itself open when a collapsed field fails validation. Save and validation semantics are unchanged because both already iterate every descriptor field regardless of rendering. The watch-events dependency UX is left to #16236 so the two changes stay complementary. * fix(web-shell): forward refs through the collapsible wrappers Review of #16239 showed the three new wrappers accept Radix props including ref, but plain function components drop external refs on React 18, and the repository supports both React 18 and 19. Use React.forwardRef for the root, trigger, and content, pass each ref to its Radix primitive, and pin the contract in react18-ref-compat.test.tsx with both forward-ref type and DOM-ref assertions; a plain-function mutation turns exactly the three new type assertions red. Record the contract in both design documents as well. * fix(web-shell): keep endpoints and operators visible, make collapsed errors reachable Review round 1 on #16239 found the disclosure went too far in three places and left dead scaffolding behind. Primary endpoints (baseUrl for GitHub Enterprise / self-hosted GitLab, wsUrl for WeCom) decide where a token is sent, so they stay in Connection instead of collapsing; a fresh enterprise channel could otherwise save without its endpoint and send the token to the public API. Operators is a permission grant and stays in Access control, a deliberate product decision rather than a consequence of the key set. The panel title becomes a real h3 wrapping the trigger (ARIA makes button descendants presentational, so the previous span-in-button was invisible to heading navigation), and a rejected submit now scrolls the first erroring advanced field into view instead of only opening the panel below the fold. The record rule is unified into one exported predicate shared by validation, the save path, and the visibility/auto-open readers; the unreachable routing section scaffolding is removed; a comment records why enum keys must stay out of the advanced set. Each new mechanism carries a mutation witness (endpoint placement, heading, boolean arm, record arm, scroll), and the visuals scenarios now anchor the heading, the configured-instance auto-open, and the count badge in a real browser. Both design documents are revised in lockstep. |
||
|
Shaojin Wenandverify-round
|
30079d6c3d |
test(managed-agent): ratchet persistence and runtime-broker class sizes (#16284)
The store decomposition (#16090, #16142) and the broker extraction cluster turned the god classes into facades, but nothing stops them regrowing. Add ClassSizeRatchetTest, the class-size gate the structure review left as an open question, introduced now that the splits have landed. It pins every current class >= 800 lines in persistence/runtime-broker at its present size (ratchet-down-only), caps all classes at a global ceiling seeded at the current maximum, forces any class reaching 800 lines to be pinned, and rejects stale pins. Verification-only: no production code change. Each rule is mutation verified; the bilingual design and the structure-review ledger (open question resolved, acceptance criterion concretized) are updated in the same change. Co-authored-by: verify-round <verify-round@local> |
||
|
Shaojin Wen
|
5f0474abc4 |
test(integration): compare the ACP parity workspace home to its pre-run listing (#16266)
The guard hardcoded the two names the shared harness seeds into the workspace
home, so the next artifact that harness seeds would red every parity scenario
with a message about a runtime leak that is not there. Snapshot the listing
before the agent starts and compare against that instead, which states the
invariant without copying the fixture contents of another module.
Carries forward
|
||
|
Shaojin Wen
|
9b1fcb8756 |
fix(channels): scope the poll cursor file to the workspace (#16231)
* fix(channels): scope the poll cursor file to the workspace Polling channels (GitHub, GitLab, DWS) wrote their cursor directly under ~/.qwen/channels/ keyed by channel name, while every other per-channel state file lives under the workspace scope directory. Channel names are per-workspace configuration keys, so two workspaces running a channel of the same name shared one cursor and, for GitHub, one set of dedupe rings. Store the cursor under ~/.qwen/channels/<workspace-scope>/ with private permissions. A channel without a scoped cursor reads the legacy global file once so an upgrade keeps its position; the next save lands in the scoped path and the legacy file is left for other workspaces and older versions to read. * fix(channels): tighten the temp cursor file before renaming it writeFileSync's mode only applies when the file is created, so a pre-existing temp file with looser permissions would carry them onto the cursor. Chmod the temp file before the rename, matching the package's other state writers, and pin it with a test. |
||
|
Shaojin Wen
|
684c75b50d |
docs(managed-agent): reconcile structure-review ledger with the landed store decomposition (#16230)
The store physical aggregate split landed in #16090 (split Agent and Session
stores into immutable internal collaborators, retaining transaction facades)
and #16142 (trim delegation boilerplate), but the structure-review ledger still
listed ManagedAgentStore at 7,657 lines and ManagedSessionStore at 3,931 and
called the physical split "the remaining work". Reconcile both language
versions with origin/main
|
||
|
Shaojin Wen
|
bf2f003294 |
feat(web-shell): edit GitHub repository settings from the channel editor (#16149)
* feat(web-shell): edit GitHub repository settings from the channel editor Three settings decide how a GitHub Channel behaves and none of them could be changed from the Web Shell: which accounts may speak in a given repository, whether that repository answers without a mention, and how often the notifications API is polled. Narrowing one repository meant hand-editing settings.json on the daemon host, so the permissive channel-wide default stayed in place by inertia. The poll interval becomes a declared descriptor field, which is also what makes it persistable: the store rejects an undeclared key outright. Per-repository settings stay in the editor, because the store already validates their shapes and a new descriptor field kind would touch the SDK and every client for a shape only one platform family uses. Each repository row is tri-state and defaults to inherit. The save compares every field against the same reading of the stored object it seeded the draft from, so an untouched repository keeps its keys verbatim -- including an empty allowlist, which means nobody, and a list with no policy beside it, which inherits the policy but overrides the members. Materializing resolved defaults instead would freeze today's inheritance and stop a later change to the channel-wide default from reaching those repositories. The section map is keyed by field name, so Email's existing poll interval moves to Connection alongside GitHub's; a test pins that rather than leaving it to prose. * fix(web-shell): let a chosen repository allowlist own its empty speaker list Picking "Allowlist only" on a repository row and leaving the speaker box empty persisted the policy with no list at all. The runtime resolves a missing per-repository list through `??` to the channel-wide one, so the operator's most restrictive available action kept the wider audience while the field beside it said "Empty means nobody." The merge only wrote the list when the speaker text changed. On a freshly added row the text starts empty and stays empty, so changing only the policy skipped the write entirely -- the default state of the feature, not an edge case. Choosing the policy now owns the list too. The rule is deliberately scoped to a policy the operator actually changed: a stored allowlist with no list is a legitimate hand-written shape meaning "members inherited", and writing an empty list there would narrow a channel nobody edited. Both directions have their own case and their own mutation. * fix(web-shell): make the repository speaker list symmetric and its hint truthful Two defects in the per-repository speaker list, both in the path where the operator moves the policy without touching the text. Leaving an allowlist behind kept its empty list. An empty array is not nullish, so the runtime kept denying everyone while the row read "Inherit" and the hint promised the channel-wide list was back. No gesture the form offers could repair it, because every one of them reads back as an empty box and wrote nothing. The policy-changed branch now handles both directions: entering an allowlist owns an empty list, leaving one releases an empty list, and a list with names in it stays because the box still shows it. The hint was derived from the draft policy alone, which tells two deliberately preserved stored shapes the opposite of the truth: a stored allowlist with no list inherits the channel-wide members but was told "empty means nobody", and a stored empty list with no policy admits nobody but was told "leave empty to keep the channel-wide list". The row now asks the save path what it will own, so the copy and the persisted object cannot diverge again. Also pins the paths the previous suite left unexecuted: returning a stored require-mention and policy to inheritance, writing a list while the policy stays inherited, keeping a stored list when the policy moves to an allowlist, each row showing and editing only its own speakers, an explicitly open row, and the localized interval label. * fix(channels): bound the poll interval and stop a new bound stranding old channels Declaring pollInterval exposed two problems the field did not have while it was unknown to the descriptor. Nothing bounded it from above. Node clamps a timeout past its 32-bit range to 1ms, so "poll once a month" persisted a hot loop that burns the notifications quota and settles into error backoff. Number descriptors now carry a maximum beside exclusiveMinimum, mirrored through the SDK, the registry's metadata validation, the store and the editor, and GitHub declares the same one-day ceiling Email already enforces at runtime. PollingChannelBase also falls back past it, so a hand-written configuration cannot hot-loop either. And a stored value the runtime tolerates became unsavable. A channel holding pollInterval 0 works today because the poll loop falls back to 60s, but the editor re-sends that 0 verbatim and both the editor and the store now rejected it -- blocking an operator who only wanted to add a repository. Both skip the range check for a finite number deep-equal to what is stored. The tolerance is deliberately not generalized: an existing contract pins that an unchanged stored scalar of the wrong type is still rejected, and a type error should be fixed rather than forgiven. Also names the repositories whose settings outlive a switch away from the allowlist policy. The runtime resolves per-repository senders and requireMention without consulting groupPolicy, so those entries stayed enforced while the rows disappeared; deleting them would destroy hand-written configuration, so the editor lists them and says how to edit them instead. Closes the test gaps the review found: the delete-to-inherit paths, writing a list under an inherited policy, the array guard, per-row speaker values and two-row editing, an explicitly open row, the localized label, a mixed-case chat group id, unmodelled keys on a case-retyped repository, the descriptor field itself, and row-scoped visual assertions with the last row in frame. * fix(web-shell): make the repository hints describe what saving actually does Three places where the form promised something the save would not do, plus a ceiling that repaired too much. The speaker hint asked the projection what the current draft owns. A repository storing a speaker list with no policy beside it therefore read "empty means nobody", while clearing the box deletes the list and hands the repository back the channel-wide members -- widening it while reporting success. The hint now asks the projection about an empty box, which is the hypothetical it describes. Row edits were projected only under the allowlist policy. Editing a row and then switching the group policy rebuilt groups from the stored config, so the save closed the dialog and restored the value the operator had just changed. The non-allowlist branch projects the same overrides now, and still drops entries that carry nothing. The retained-settings alert claimed the settings still apply under every policy that is not Allowlist. Under Disabled the group gate drops every envelope before either lookup, so the alert now says they are kept but not applied, and the Chinese copy names the option the select actually renders. The interval ceiling fell back to 60s for anything above one day. Values between one day and Node's 32-bit timer limit were honoured before and are not hot loops, so a two-day GitLab interval became 1440 polls a day. Clamp instead of replacing, and state the ceiling in the GitLab and hosting-contract docs. The range message also invented a lower bound of 0 for a field declaring only a maximum, and now names only the bounds the descriptor declares. Tests: the three registry maximum-validation branches, the maximum half of the unchanged-value tolerance on both the editor and the store, the inherit sentinel mapping through both row selects, the disabled alert, the empty-box hint, and both shapes of the range message. Docs: the design no longer claims no SDK or schema change, credits the right class for speaker resolution, and the serve protocol reference documents maximum. * fix(web-shell): say what an empty speaker box saves, and sweep the invariant The inherited-speaker hint promised to "keep" the channel-wide list, but a repository storing its own list is not keeping it: clearing the box deletes that list and hands the repository the channel-wide members, which can be many more people. "Use" holds for every shape that reaches the hint. The case pinning that shape now also clears the box and asserts the request, so the copy and the save are pinned together rather than separately. Pass the group policy into the retained-settings alert instead of deriving it a second time. The two derivations were equivalent, but nothing kept them that way, and a drift between them is what made the alert claim settings still apply under a disabled policy. Add a three-part invariant sweep over twelve stored shapes crossed with every single-field edit, driven through the real save path: an untouched row reproduces its stored object byte-for-byte, the empty-box hint agrees with what that save admits, and no edit widens the admitted set unless the row's own controls asked for it. Admission resolves per-repository then wildcard, so an absent list means the channel-wide members rather than nobody. Correct the docs where they described the superseded implementation: the hint decision still named the current-draft projection, the user guide contradicted itself on clearing an explicit allowlist, the clamp's blast radius omitted Email not extending the base at all, and the files table listed fifteen of the twenty-four files this change touches. * fix(web-shell): keep repository edits on every non-Allowlist save path The branch that retains per-repository settings when the group policy leaves Allowlist was gated on the STORED policy, so a channel already open never reached it: an operator who switched to Allowlist, narrowed a repository and switched back saved successfully while the request restored the old open entry. It also iterated stored groups only, so a repository typed during that visit was never written and fell back to the wildcard. Both admitted a sender the operator had just excluded, and the alert promises those edits are kept. Projecting the draft onto conflicting aliases was the third path. The draft holds one entry per lower-cased id, so two stored spellings that disagree both received the last one's settings, turning an alice-only repository open while the conflict check stayed silent — it only runs when the final policy is Allowlist. Those entries are now left as stored, which is what the runtime resolved before and cannot widen anything. Refusing the save here instead would strand the operator: the conflict error is reported on a field that is not rendered once the policy leaves Allowlist. Both consumers now share one alias set rather than two judgements that could drift. The invariant sweep missed all three because its fixture hardcodes Allowlist, so it never varied the policy dimension. A second sweep crosses the same shapes with the stored and final policies, for a stored repository and one that exists only in the draft, asserting the runtime's own rule: speakers are resolved without consulting groupPolicy, so a row must write the same entry whichever policy the save lands on. * fix(web-shell): keep an ambiguous repository alias exactly as stored Preserving conflicting aliases from the projection was not enough: the empty-entry filter still ran, so a no-op save on a channel stored Open deleted an empty alias. The runtime matches that exact key and inherits the wildcard allowlist, while deleting it lets the case-insensitive fallback reach the wider spelling — a repository nobody edited went from one allowed sender to everyone. The same gate one type over deleted a hand-written non-record value with the same result; the store preserves an unchanged non-record value, so passing it through keeps the save a no-op instead. An ambiguous entry is now exempt from the filter whatever it holds, and a non-ambiguous empty entry still drops so a row returned to inherit leaves no stub. Skipping the draft for ambiguous ids also discarded edits the operator had actually made, and the dialog closed on success while the request restored the stored policy. An edited ambiguous row is now refused, with the error on groupPolicy because the allowlist field it used to be reported on is not rendered once the policy leaves Allowlist. The refusal is scoped to a real change against the reading that seeded the draft, so an untouched switch still saves; refusing everything would strand an operator who only changed the policy. The alias dimension is now crossed into a sweep rather than left at two examples: every stored shape against a disagreeing lower spelling, over both stored policies and all three non-Allowlist final policies, asserting that an untouched save is a byte-for-byte no-op admitting the same people through the runtime's exact-then-fallback lookup, that a real edit is refused, and that a no-op edit still saves. Two dialog cases click the same flows and assert whether onSave is reached, which the state layer cannot see. * fix(web-shell): enforce the interval floor and stop rewriting entries the form cannot show The poll interval had a ceiling but no floor. The descriptor's lower bound is exclusive, so 0.5 passed the editor and the store, and setTimeout rounds it to about 1ms — the hot loop the ceiling exists to prevent, reached from below and persistable only because this change made the field managed. The getter now clamps into both ends from one pair of exported constants, and the GitHub descriptor declares its maximum from the same export so the two cannot drift. An exclusive minimum of 1 is not available, since it would reject the documented 1, so the field description and the user doc state that a smaller positive value is raised to 1. The claim that the store preserves an unchanged non-record group entry was wrong, and it was mine: the tolerance covers the whole groups value, while a single non-record entry is refused on every save. A case against the real store now pins that. Rather than weaken per-entry validation to make the comment true, the editor refuses such a save with a message that names the entry and points at settings.json, because no row can represent it and a generic daemon error leaves nothing to act on. The entry is still not deleted, whether or not it has an alias twin: deleting the exact key the runtime matches first is a rewrite nobody asked for. The retained-settings alert enumerated storage instead of the save, so it never named a repository typed during the visit even though the save writes it, and it announced an allowedUsers list beside an inherited open policy as still applying when senderGateFor builds no allowlist for it. The list now comes from the request the save builds and is classified by resolved effect. An unrelated edit also rewrote the file: a channel that never had an allowlist lost an already-empty entry, because the filter treated every empty entry as a stub. It now distinguishes leaving Allowlist from never having had one, while a repository typed during the visit stays a placeholder that is dropped when it carries nothing. |
||
|
Shaojin WenandQwen-Coder
|
8d8217aeee |
refactor(managed-agent): narrow EmbeddedRuntimeBroker to SessionQueryStore (#16216)
* refactor(managed-agent): narrow EmbeddedRuntimeBroker to SessionQueryStore The broker holds the store only to forward it into WorkspaceRuntimeResolver and ManagedRuntimeCoordinator (resolver static + constructor), which both take SessionQueryStore, and makes no direct store call. Narrow the field and all eight constructor overloads from the AgentStateStore composite to SessionQueryStore; the sole caller RuntimeBrokerConfiguration still passes an AgentStateStore, assignable to the role, so it is unchanged. Type-only. Update the structure-review ledger in both languages: the broker moves from candidate into the single-role audit (query group), tally 22 -> 23. Full real Spring context boot (ManagedAgentServerIntegrationTest, 57 tests) resolves the narrowed bean with no ambiguity; ModuleArchitectureTest and Checkstyle clean; all modules compile. Co-authored-by: Qwen-Coder <[email protected]> * refactor(managed-agent): drop EmbeddedRuntimeBroker's write-only store field Address review R1-1: the store field was never read — this.store = store was its only occurrence besides the declaration, and the three forwarding sites (WorkspaceRuntimeResolver, ManagedRuntimeCoordinator.resolver and constructor) read the constructor parameter, which shadows the field. Remove the field and the assignment; every constructor stays narrowed to SessionQueryStore and the forwarding is unchanged. The broker still depends on SessionQueryStore through its constructor, so the ledger entry stands. Full real Spring context boot (57 tests) + ModuleArchitectureTest + 0 Checkstyle; all modules compile, confirming no read of the removed field. Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin WenandQwen-Coder
|
f30da36c6f |
feat(channels): deliver GitHub events to PR-bound sessions (#16117)
* feat(channels): deliver GitHub events to PR-bound sessions A session that follows a PR (it created, reviewed, or verified the PR and carries the PR binding) never saw GitHub channel events: the channel routed everything to its own chat_thread sessions, and pure CI failures or merge conflicts produced no notification at all. This connects the two systems. When deliverToBoundSessions is enabled, a new read-only process-global route (GET /sessions/pr-bindings) reverse-looks-up the sessions bound to a PR, and the GitHub adapter injects the event into the most recently updated bound session as a prompt instead of a channel-owned session. The injected prompt marks GitHub content as untrusted, keeps the reply inside the session (no auto-publication), and runs the same group/sender/pairing preflight as the channel path. Dormant bound sessions are woken through the bridge's resume path; only a truly deleted session falls back to the normal channel path. Queue-full is treated as backpressure and does not consume the retry budget. prWatchEvents adds edge-triggered state watching for bound PRs inside the existing poll cycle (ci_failure, merge_conflict, head_change, state_change), with per-head dedupe, a bounded grace window for late-registering checks, partial-snapshot-safe pruning, and state persisted before dispatch so restarts never re-fire a transition. Both fields default off; existing deployments are unchanged. Design: docs/design/2026-11-02-github-channel-bound-session-events.md (EN) and .zh-CN.md (ZH). Verified with 2039 unit tests across channels/base, channels/github and the cli worker/route suites, plus isolated-daemon E2E against a mock GitHub API (bound delivery, dormant wake, watch transitions, default-off regression). * fix(channels): harden bound-session delivery against incomplete scans and crash windows Address the six review findings on the bound-session event delivery, each with a regression test: - Partial binding snapshots no longer replace the routing map or the accessor's wake cache: known bindings and cwds absent from an incomplete answer are retained, so a workspace listing failure can never switch a private delivery to the public channel path or turn a queued dormant delivery into a gone-session fallback. - A binding recorded merged/closed by the independent refresh stays watched until the poll itself observes the terminal transition, so state_change is never skipped by the refresh winning the race. - An empty combined commit status (total_count 0, which GitHub reports as state "pending" for SHAs without legacy statuses) no longer counts as active CI; Actions-only and no-CI heads settle instead of re-fetching checks on every poll. - Watch events now persist a per-head fired ring, so a flapping aggregate (failure -> success -> failure on the same head) delivers ci_failure exactly once, across restarts. - The bindings route flags live-merge degraded successes (liveMergeFailed) as partial, alongside truncation and scan-cap exhaustion. - Detected watch events are queued durably (pending) in the same write as the baseline, then claimed, marked fired, and delivered; a failed claim leaves the event queued, and a crash at any point leaves either a pending event or a claimed task for recovery. The watch pass runs before generic inbound recovery so recovery cannot remove a watch task the watch pass would then re-claim. Design docs (EN/ZH) updated to match. Tests: channels/github 258, cli worker+route 139, channels/base 1652 — all green. * fix(channels): close bound-delivery lifecycle and snapshot-authority gaps A multi-round undirected + adversarial audit of the bound-session delivery feature (61 confirmed findings, probe-verified) plus a follow-up review round and the triage escalation surfaced gaps around the very mechanisms the first hardening commit introduced. This closes them in one pass: Delivery lifecycle: watch dedupe keys now identify transition instances (episode counter) so dirty->clean->dirty, close->reopen->close and SHA revisits fire again while ci_failure stays once-per-head; pending events reconcile before generic recovery and independently of the API pass; terminal task records are replaced instead of silently swallowing recurring keys; acceptance is persisted before task removal so a crash cannot replay an injection; crash loops charge one attempt per recovery pickup; queue-full and the new BoundSessionUnavailableError defer within a bounded horizon and retire to a private dead-letter log instead of freezing the shared notification cursor or vanishing silently; watch events are private-only (maintainer decision: no public fallback that would publish operator-facing prompt text). Snapshot authority: the bindings route enumerates workspace entries and flags non-active trusted workspaces and multi-page scans partial; retention is session-granular and recency-competing; the comment lane defers until the first complete snapshot; cold-cache wake treats partial absence as transient; www host aliases normalize; malformed 200 bodies fail closed. Watch detection: periodic settled-head re-check (GitHub does not bump PR updated_at on check activity), ci_failure subscription gate, startup_failure conclusion, fail-closed state reads, array-shape guard, tmp cleanup on failed writes, rate-limit fast-skip so the watch cannot pin the notification lane. Worker hygiene: channel-turn _meta classification with worker authorization, wake registration detach after the retried prompt settles, response-body cancellation and body-read deadlines; the restore route distinguishes permanent workspace absence (workspace_not_found 404) from reversible draining states. Docs: both design languages record the new decisions (private-only watch, bounded backpressure + dead-letter) and corrected semantics; user doc cost claims qualified. ~90 new regression tests pin each fix witness. * fix(channels): repair bound-delivery build, error taxonomy, and watch dedupe A second undirected + adversarial audit of the bound-delivery fix commit found a broken build and 33 further confirmed findings. This closes them all in one pass: - build: transitionInboundTask's Pick omitted deliveredAt/deferCount/ backpressure, failing the github package's tsc (vitest masked it). - cold start: gate the comment lane on the first successful snapshot (boundSnapshotSeen), not complete-only, so a permanent partial no longer stalls the lane forever. - private-only watch: route watch envelopes by identity so a delivery-cleared watch task can never leak to the public path; retire watch events symmetrically on unbind; restrict terminal-task replacement to watch keys so a cancelled comment is never revived after a crash. - error taxonomy: superseded/archived sessions are gone, the worktree-reset barrier and cold-partial absence are backpressure, on both the direct prompt and wake paths — permanent states fall back, reversible states defer instead of burning the retry budget. - cross-workspace: the worker token is a process-level relay credential admitted for any served (primary/trusted) workspace, restoring channel-turn classification for cross-workspace bound delivery. - snapshot authority: flag an untrusted primary as partial; log per-workspace scan failures. - watch robustness: fail fast on secondary rate limits; pin ci_failure once-per-head against fired-ring eviction (ciFiredSha); roll back a claim whose pending->fired save fails; reconcile pending events independently of the binding fetch; surface watch-cap overflow. - dead-letter: surface the payload and stop claiming success on write failure. - restore contract: migrate unknown-cwd tests to 404 workspace_not_found and pin reversible states (draining/transitioning/blocked) out of it. Adds 38 regression and test-gap tests; each code fix mutation-verified red without its test. Both packages type-check; design docs (EN/zh-CN) and the user doc synced. * fix(channels): harden bound-delivery snapshot authority, recovery ordering, and watch fail-fast Round-3 undirected + adversarial audit found further defects; all high-confidence ones are closed here. - snapshot authority: a bound-session Gone evicts the dead session from the binding maps before the refresh, so a permanent partial snapshot cannot retain it by recency and shadow a live follower; a Gone from the re-delivery hop falls through to the public/retire contract instead of consuming the failure budget; a binding refresh that has never succeeded logs a distinct, throttled, actionable stall message (the gate stays fail-closed). - recovery ordering: a bound task's deliveredAt acceptance mark is persisted before the cross-lane pending-deliveries read, so a corrupt or unreadable file cannot leave the task running without the mark and replay the injection; watch state drops malformed per-entry values (keeping healthy entries) instead of throwing every poll. - error taxonomy: a post-wake retry 404 is reversible backpressure (the owner may be draining; the classification keys on "the wake ran", not the detach return), not Gone; the interrupted-worktree 409s (worktree_marker_missing / worktree_reset_interrupted) are gone for delivery (a resume retry never converges); an authoritative Gone evicts the cached wake cwd. - watch robustness: fail fast on any rate limit (GitHub does not always send retry-after) and permanent per-PR 404/410/451, not just primary limits. - GHEC data-residency: map api.<tenant>.ghe.com to the <tenant>.ghe.com web host so bound delivery works there. - docs: PR breaking-change notes, dead-letter filename, Files-affected table, ci_failure SHA-revisit semantics (both design docs, EN + zh-CN), plus two test-harness isolation fixes (shuffle-safe). Adds regression + test-gap tests; every code fix mutation-verified red without its test. Deferred with disclosure: a low-confidence pre-existing sidecar enrichment silent failure in the shared session-list surface. * docs(channels): note the post-wake 404 exception in the accessor comment Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): close comment-round findings on absence, idempotency, and watch isolation An incomplete binding scan never establishes absence: empty-partial cold answers defer the lane for a bounded number of polls, and unmatched events defer recoverably (dead-letter at the horizon) until a complete snapshot lands. Scanned sessions with no bindings emit tombstone rows through the route, projection, and accessor so an observed unbind stops private delivery even on partial answers; unreadable sidecars flag partial. Prompt admission gains an idempotencyKey dedupe ring so a retry after a lost acceptance mark or a lost 202 re-injects nothing; the accessor sends the durable task id on every POST. Cross-workspace channel classification now requires the explicit bound-relay marker, scoping the process-level worker credential to its reason. 429 and other 5xx join the backpressure taxonomy. Watch hardening: reconciliation failure gates watch-task recovery, shared credential/rate-limit failures abort the whole pass (401 included), the pending CI re-check is interval-bounded, a failing Checks endpoint no longer suppresses the other detections, and pending envelopes validate with the task store's own validators. Bound prompts surface both truncation levels and the comment author; a failed dead-letter write keeps the task. Docs (user, protocol, design EN/ZH) and Web Shell i18n synced; every fix mutation-verified. Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): make the aggregate per-comment display cut visible Real-scenario verification of the truncation fixes surfaced a third silent-tail surface in the same flow: the aggregate lane caps each comment at 400 display characters with no marker, and the comment is already recorded in dispatchedComments — a long comment's tail (possibly its actionable last line) vanished from every surface while the bullet read complete. The cut now carries an explicit [comment truncated: N more characters] marker, alongside the existing prompt-level and batch-level notices. Regression test added and mutation-verified; user doc and design docs (EN/ZH) synced. Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): close round-1 audit findings on event loss and admission Undirected + adversarial audit of the merged diff found four event-loss paths and an admission-contract outlier; all fixed with regression tests and mutation witnesses: - The aggregate lane recorded dispatchedComments keys BEFORE the task claim; an unwritable task store plus the unconditional end-of-poll cursor save persisted the keys with no task, silently consuming the comments forever. Keys now land via applyTaskDedupe only after the task write succeeds. - The idempotency key is now the event identity (source chat + message id), not the task id: a crash after task removal but before the cursor save re-creates the task, and a task-scoped key let the daemon admit the same event twice. - A persistently failing Checks endpoint degraded the pending re-check interval to one doomed request per PR per poll; failed attempts now advance the interval clock, and the undefined-ci branch is bounded too. - Pruning a watch entry whose earlier transition still has a surviving task reset the episode counter; a rebind then collided keys and silently consumed the new transition into the old envelope. Prune now skips entries with recoverable watch tasks (and skips entirely when the task store is unreadable). - sendPrompt's unknown/closing admission rejections threw as rejected promises, letting the prompt route return a phantom 202 and the ring remember a non-admission; they now throw synchronously like the sibling admission paths, and the route forgets the key on any deferred SessionNotFoundError as defense in depth. Also: bounded the accessor wake-cwd cache under permanent partial, honest relabel of the routing-gate regression test, field-level body assertions, Web Shell coverage for the two new descriptor fields (EN/zh), descriptor reasonFilter description synced to the checkbox contract, and the protocol doc's ring bounds completed (1024-session cap and its lifetime exception). Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): close round-2 audit findings on watch key identity and admission The round-1 event key still collided across PRs (one repo, one session, parallel episode counters) and across episode-counter resets (prune or corruption recovery plus rebind), letting the daemon's long-lived idempotency ring silently swallow new transitions. Watch keys now bake in the PR number and detection timestamp; ci_failure keeps once-per-head via PR+SHA prefix matching in the fired ring and pending queue. Over-length event keys hash deterministically instead of earning a route 400 that burns the retry budget. The prompt route forgets the idempotency key on any pre-admission deferred rejection, keyed off the admission signal rather than the error type, covering the standalone working-directory RequestError. Permanent partial snapshots can no longer grow binding retention or watch entries without bound (1024 cap, oldest-first, surfaced on stderr). Design docs (EN/ZH) and contract comments are synced, including the ring-lifetime exception at the 1024-session cap. Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): close round-3 audit and maintainer review findings The public-routing gate trusted a historical complete snapshot: under a later partial answer an unmatched PR event could take the irreversible public path while a live follower hid in an unlisted workspace. Absence is now authoritative only on a current complete successful snapshot, or per PR when every previous non-channel binder is listed and excludes it (a listed session's PR set is complete, so its tombstone stays authoritative). Never-known PRs, omitted or retention-evicted binders, and failing refreshes defer as bounded backpressure; the new clauses are PR-scoped so issue events and channel-only bindings keep flowing publicly. The aggregate batch idempotency key identified only the first comment, so a superset batch re-listed after the cursor-save crash window deduped into the original admission and permanently consumed the new members. The batch message id is now a content hash of the sorted member identities; identical batches still dedupe across the acceptance crash window. Partial retention rebuilt only the old winners, so a listed tombstone on the newest follower emptied the routing map despite a retained alternate; the rebuild now iterates the retained session rows. Watch-entry cap eviction gained the prune's retained-binding and in-flight-task protections (an evicted entry re-baselines silently, so the earlier re-fire claim was wrong and is corrected). Disabling watch subscriptions no longer clears the reconciliation guard while unreconciled pendings exist, and the per-entry pending queue is capped at 64 with oldest overflow retired to the dead-letter. Cross-instance watch delivery is documented as per-subscription at-least-once, and the admission residual (202 before admission completes) is stated precisely in the route and contract comments. Design decisions 10/11 (EN/ZH) are synced. Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): close round-4 audit findings on absence authority and republication A partial binding snapshot can never prove a PR unbound: an unscannable workspace may hold a never-observed follower, and a listed tombstone proves only that the listed session unbound. The round-3 per-PR authority mechanism granted public routing when every observed binder was listed and excluding the PR, which a probe showed misroutes a same-poll comment irreversibly. Remove the mechanism; unmatched PR events now defer as bounded backpressure until a complete snapshot lands, while listed tombstones still stop private delivery and watching immediately. The partial/failed clauses stay PR-scoped so issue events keep flowing once the cold-start gate opens. Close a pre-existing public-reply double-publish window: a reply could land (publication audit posted), the task be removed, and the cursor save lost; the re-listed comment was claimed and published again because only recovery checked the audit. The dispatch path now honors the same audit record before claiming, fail-closed on an unreadable audit, and records the dedupe keys so the cursor advances. The worker accessor flagged a listed session partial only when ALL its PR entries failed validation; any dropped entry now degrades the answer so a partial projection never masquerades as complete. The round-3 cap-eviction task protection gains a non-vacuous witness (the protected entry moved outside the retained candidate range). Bound preflight rejections and the disabled-subscription guard now surface accurate stderr, and the protocol doc states the 202-before-admission residual. Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): close round-5 audit findings on audit fail-closed and wake classification The round-4 claim-time publication audit check claimed fail-closed but only skipped the single notification: with no task claimed, the end-of-poll cursor commit (mark-read included) advanced past the skipped event, so a persistently unreadable audit silently consumed every new public event instead of retrying. An unreadable audit now freezes the whole poll's cursor via a dedicated flag, and the next poll retries the read and the events. The audit consumption verdict treated any posting/posted/suppressed line as consumed regardless of a later terminal failed, so a reply that never landed (posting then failed) plus a lost cursor save permanently swallowed the event. The verdict is now the LAST outcome per source key: a terminal failed keeps the event retryable; a bare posting (crash mid-post) still consumes at-most-once. The gone-session public fallback bypassed the absence gate: under a partial scan an unscannable workspace may still follow the PR, making the public reply irreversible. The fallback now defers as backpressure until a complete snapshot establishes absence (delivery cleared so recovery re-resolves without a doomed POST); deliverToBoundSession returns a settled flag so runInboundTask's success path cannot retire a task the fallback deferred. The wake/resume path classified 429 and non-503 5xx as delivery failures, burning the attempt budget into an unrecoverable dead-letter on a transient resume rate limit or spawn jitter; they are backpressure now, aligned with the POST path. The watch drain stops at the first failed delivery to preserve per-PR ordering, and the worker flags ANY dropped session row partial, symmetric with the PR-entry rule. Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): close round-6 audit findings on freeze bypass and ordering The round-5 audit freeze only froze the cursor commit, but the direct lane and the first-contact body feed recorded their dedupe keys unconditionally after dispatch; the run loop's unconditional cursor save then persisted the keys, so a transient unreadable audit permanently filtered the re-listed events — the same silent consumption the freeze was meant to prevent, revived through two lanes. Both lanes now honor the dispatch refusal before recording, matching the comment lane. The audit consumption verdict treated every terminal failed as a provable no-write, but ambiguous errors (502/timeout) may have landed the comment; unlocking those keys let a crash window double-publish an irreversible public reply. Ambiguous failures now audit as failed_ambiguous and stay consumed (at-most-once); only definite no-write failures (secondary rate limit with remaining=0) unlock retry. The per-PR drain ordering held only within a single pass: the watch pass re-drains after reconcile and the next poll's reconcile runs before recovery, so a later event could still arrive before its unsettled predecessor. The drain now skips the whole entry while an earlier watch transition of the same PR has an unsettled task (a task whose key equals the pending head is that event's own crash-window claim and drains normally). A claim persistence failure propagates out of the drain so reconcile fails and the recovery guard holds instead of delivering an accepted task whose pending record is unreconciled. Disabling watch subscriptions with legacy unreconciled pendings parked the accepted tasks as backpressure so they stop freezing the shared notification cursor channel-wide; an unusable watch state container is surfaced on stderr like the corruption path instead of wiping silently. Co-authored-by: Qwen-Coder <[email protected]> * test(channels): cover the audit-freeze recovery with deliverToBoundSessions off The maintainer's P2 reproduction exercised the unreadable-audit window with deliverToBoundSessions both enabled and disabled; parameterize the R5-A1 freeze/recovery test over both modes so the public-only path is pinned too. Co-authored-by: Qwen-Coder <[email protected]> * test(channels): mock pulls.get in the direct-lane freeze test processDirectLane fetches PR meta before dispatch; without an explicit pulls.get mock the fetch throws under isolated runs, the per-notification catch skips the event, and the test never reaches the freeze guard — passing only by mock leakage from earlier tests in a full-suite run and failing (vacuously) in isolation. The mock makes the witness genuine in both run modes; the guard-off mutation now kills it isolated too. Co-authored-by: Qwen-Coder <[email protected]> * test(channels): pin the reader's failed_ambiguous consumption verdict The writer-side failed_ambiguous tests only read the audit file; no test drove the reader's consumption of an ambiguous verdict, so deleting the reader's failed_ambiguous branch kept the suite green. The distinguishing sequence is posting -> definite failed -> failed_ambiguous: the ambiguous retry must supersede the stale definite-failed verdict and stay consumed, or the double-publish window reopens. The new test seeds that sequence (plus a definite-failed retryable contrast) and drives dispatch; the reader-branch mutation now kills it. Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): let the notification cursor flow while watch reconcile is blocked Co-authored-by: Qwen-Coder <[email protected]> * test(channels): prime the comment lane in the direct-lane freeze test Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): decouple watch retires from the public reply store and sanitize dead-letter dumps Co-authored-by: Qwen-Coder <[email protected]> * fix(serve): throw synchronously on the standalone admission guard so relays never consume a phantom 202 Co-authored-by: Qwen-Coder <[email protected]> * test(serve): harden deferred-forget retries and align admission contract comments Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): protect the dispatched-comment ring from frozen-cursor eviction and sanitize error diagnostics Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): remove the hard-cap comment refusal that silently consumed events; harden the cursor ring Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): prune the comment-time map every poll, not only on dispatch Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): iterate live followers in the gone-fallback and prune the time map in finally Co-authored-by: Qwen-Coder <[email protected]> * fix(channels): exempt issue events from the whole bound-absence gate The per-event absence gate's !boundSnapshotTrusted clause was not PR-scoped, so under a permanent partial binding scan (a multi-page scan, a long-lived non-active workspace, an untrusted primary, or a corrupt sidecar) a complete snapshot never lands and unmatched issue events deferred to the 240-deferral horizon and dead-lettered without a public reply — even though bindings are PR-only and an issue's absence is provable without any snapshot. PR-scope the whole gate, cold start included; issue events route publicly at once. The labeling assumption is verified against 50 real GitHub notifications (zero subject-type/URL mismatches) plus GitHub's exclusive issue/PR number space. Sync the design EN+ZH (gone-fallback iteration, the finally prune timing, the whole-gate issue exemption) and the user doc, and align the stale R13-M1 test comment with the finally semantics. * docs(channels): qualify the issue exemption and document the audit write trade-off Round-15 audit follow-up, documentation only: The user-doc sentence added in round 14 overstated the issue exemption ("never held by the binding scan ... at once"): round 14 PR-scoped only the per-event absence gate, while the lane-level cold-start gate still holds every notification (issue events included) until the first successful binding lookup, and the empty-partial gate holds the lane up to 30 polls. Qualify the sentence to the per-event gate, state the lane-level hold explicitly, and tighten the design's stale-bindings shorthand to match (EN + ZH). Also document the publication audit's WRITE semantics in key decision 10 (EN + ZH): the write is best-effort (fail-open) so an unwritable audit store does not break the public-reply contract — a deliberate trade-off pinned by the "keeps successful publication when its audit write fails" test. Under total storage failure no durable consumption intent can be recorded, so a failure spanning the posting record, the POST, and the task removal can let recovery re-publish; the read path stays fail-closed. Notes the fail-closed write-ahead alternative and its availability cost. * fix(channels): tolerate .diff/.patch binding URLs and pin the aggregate sort parsePrBindingUrl rejected GitHub's /pull/N.diff and /pull/N.patch spellings while the refresh sweep's pullRequestKey tolerates them. The bind route accepts any http(s) URL without normalizing the path, so a manually pasted .diff/.patch binding persisted and was then skipped by the routing-map build, the gone-fallback, and the PR watch — silently degrading that PR to public replies and never watching it. Align the suffix alternation with pullRequestKey; the existing number-match guard still rejects mismatched url/number bindings. Add parameterized .diff / .patch regression tests. Also pin the aggregate content hash's order-independence: the existing R3-B1 test only fed ascending members, so removing the implementation's sort survived. Add an out-of-order batch test asserting the key equals the sorted batch's key. Documentation: key decision 11 (EN + ZH) still carried round 14's overstatement that issue events are exempt from the "ENTIRE absence gate, cold start included"; round 15 fixed the user doc and the stale-bindings paragraph but missed this numbered decision. Qualify it to the per-event absence gate (including its !boundSnapshotTrusted cold-start clause) and state explicitly that the separate lane-level cold-start gate still holds issue events until the first successful lookup. * docs(channels): document the hidden-catalog delivery limitation Round-17 audit follow-up, documentation only (EN + ZH): Document the bindings snapshot's hidden-catalog exclusion as a deliberate design limitation: the machine-owned multi-agent task sessions (agent-host / agent) are excluded by the session-list path without setting partial, and they are not interactive delivery targets, so a PR bound only to such a session takes the public channel path. The public reply is safe; the private-delivery contract does not extend to those sessions. Making the snapshot authoritative over them or gating the write path is left to the multi-agent subsystem's design. Also finish the per-event qualification sweep: the absence bullet still said issue events are exempt from "the whole gate"; qualify it to "the whole per-event gate" to match key decision 11 and the sibling surfaces. * fix(channels): address review round — wake-cache presence, watch 403 isolation, aggregate ack Resolve the review-bot findings (one Critical) plus the doudouOUC doc gap, each pinned by a regression test and an isolated mutation witness: - R2-1 (Critical): the cold-cache wake branch read the target's cwd from the FIFO-capped hint cache, so a listed, live, dormant session beyond the 1024-row cap on a complete snapshot was misreported as gone and its event posted publicly. Read presence from the fresh full snapshot instead; the cap stays a bound (fast-path hint only) and the Gone verdict is unchanged. A present session is now woken even on a partial answer (presence is authoritative); only an absent session defers there. - R1-7: a failing refresh after one successful empty partial stalled the comment lane forever (the bounded escape only advanced on the success path) and hid behind a misleading "keeping previous map" log. Advance the same escape and key the distinct stall log off the no-authoritative-map state. - R1-11: a per-repo permission 403 aborted the whole watch pass, starving every other watched PR. Abort on a 403 only when provably a shared rate limit; otherwise isolate it per PR. - R2-2: the aggregate batch's non-numeric hash messageId silently disabled the eyes reaction. Record the batch's first comment as a bounded reaction anchor resolved by the prompt-start/end overrides. - R2-18: export PR_WATCH_EVENTS and build both the validator set and the UI option list from it; add the four localized option labels (EN + ZH). - doudouOUC: document the >100-session permanent-partial steady state and add a throttled diagnostic naming the listed session count. Test gaps closed: prsReadFailed production setters (R2-3), both idempotency FIFO bounds (R2-4), the real-fetch comment-time capture (R2-5), and the editor field descriptions (R2-6). Documentation: name the third relay _meta credential key (R2-7), complete the Files-affect inventory (R2-8), and qualify the unconditional acceptance criteria to the two FIFO caps and the restart recovery (R2-9), mirrored EN + ZH. --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin WenandQwen-Coder
|
68c9fa27fb |
feat(verify-pr): add local Docker recipe with a host-side model proxy (#16077)
* feat(verify-pr): add local Docker recipe with a host-side model proxy A local /verify-pr round needs the same isolation the CI lane provides, but the skill only documented the boundary check, not how to run the model credential through it. Add scripts/verify-pr-model-proxy.mjs, a host-side relay that holds the real key and lets the container call the model with a per-run bearer token, mirroring the CI lane's inline proxy (POST /chat/completions only, nonce health check, per-chunk idle watchdog). The listen address is a flag so the proxy can bind the Docker bridge, and readiness is printed as one JSON line. The SKILL recipe pairs it with a docker run template: the token travels as QWEN_VERIFY_PROXY_TOKEN (not OPENAI_API_KEY, which the isolation gate refuses), and the container's provider entry reads it via envKey. The CI lane keeps its inline copy — the proxy holds the real key, so it must not execute a script from the PR-head checkout. Verified: 11 new pin tests plus all existing verify-pr suites (120 passed), and a real docker smoke — the gate passes inside node:22-bookworm with the proxy token, refuses OPENAI_API_KEY with exit 4, and a container POST reaches a mock upstream with the real key injected and the /v1 prefix preserved. * fix(verify-pr): harden the model proxy after adversarial audit Audit round 1 found the relay forwarded client hop-by-hop headers (connection, transfer-encoding, expect, proxy-authorization, ...) to the upstream, which RFC 7230 forbids and which fights undici's own framing for the relayed stream. Strip them, with a chunked-body test that pins the behaviour. Round 1 also found the CLI entry had no coverage: add tests for the missing-config exit code and the printed JSON coordinates (token override, base-path prefix, no real-key leak, health nonce), and document that the printed baseUrl uses the listen address, so binding 0.0.0.0 yields a URL clients cannot dial. Rounds 2 and 3 (open-ended re-read plus adversarial passes over the updated diff) found nothing further. All verify-pr suites: 123 passed. * fix(verify-pr): address review findings in the model proxy and checker Round-1 review (12 inline findings at |
||
|
Shaojin Wen
|
02b8b0108c |
feat(web-shell): show a Channel event timeline (#16106)
* feat(web-shell): show a Channel event timeline Channels reported what they did only through stderr, the publication audit, and the inbound task store, none of which the daemon could read. An operator wondering why a GitHub comment got no reply had to read the daemon log. Every Channel now appends one bounded journal per instance: the inbound messages it accepted, the ones it skipped and the machine reason for each skip, publication outcomes, inbound task transitions, and poll results. The journal holds routing decisions and identifiers only, never message text; it rotates at 1 MiB to a single backup, and a write failure is swallowed into one stderr line so diagnostics cannot break message handling. The daemon serves the journal through GET /workspace/channels/events and its workspace-qualified form, with limit, type, channel, and since filters, a 32-file and 2 MiB read bound, and the existing trust and client checks. The channel parameter repeats once per instance name because a Channel name may legally contain a comma. The Web Shell gains a Channel events page and sidebar entry that render the cross-Channel timeline for the resolved workspace, filtered by Channel and event type and refreshed every five seconds, gated by channel_management. Docs: the design in English and Chinese, the serve protocol, and the Channel user guide. Channel tests isolate QWEN_HOME so journals never land in a real home directory. * fix(web-shell): scope Channel events to the selected workspace Review found four correctness gaps in the event timeline. The page read the provider's daemon-wide primary cwd, so a Channel started in a registered secondary workspace could never be inspected. It now offers the same workspace selection as the management page and passes the selected cwd to both hooks; changing the workspace resets the Channel filter. The poll loop captured the adapter's detail before the poll ran, so the first success had no counters and every later one carried the previous poll's counters against the current timestamp. The field is now cleared before the poll and consumed after it settles, on both the success and failure paths. The 32-file read cap was applied before the Channel filter, which made an explicitly requested, quieter Channel unreachable once busier peers had newer files. Candidate files are now narrowed by the requested identities first. Channel identities were truncated to 80 code points while the management API accepts 255 UTF-8 bytes, so longer legal names never matched their own filter and names sharing a prefix became indistinguishable. The identity now keeps its full legal length; only the UI shortens it for display. Each fix has a regression test that was mutation-checked against the previous behavior. Design (both languages) and the Channel user guide now describe the workspace selection, the poll-detail timing, the filter-before-cap ordering, and the identity limit. * fix(web-shell): mask Channel events cached for another query The workspace selector changes the query without remounting the hook, and the underlying resource keeps its previous data while a request is in flight or after it fails. Selecting a second workspace therefore showed the first workspace's rows under the new selector until the new request succeeded, and kept them when it failed — including their Channel names in the filter list. Each snapshot is now tagged with the query that produced it (workspace, Channel, types, limit) and mismatched data is masked, the same way the Channel management hook already does. Two hook-level regressions drive the real hook and resource with a pending and a rejected second-workspace request; both fail without the tag. * fix(web-shell): keep Channel event reads off the daemon's critical path A verification round measured the journal read at the documented cap: one page poll took ~860 ms of synchronous I/O, an unrelated /health request waited ~790 ms behind it, and four concurrent reads serialized to 3.4 s. The route runs on the daemon that also serves sessions, so a diagnostic page must not be able to stall it. The read is now asynchronous and chunked (64 KiB awaits) under tighter bounds: 256 KiB per journal file, 4 MiB per request, 32 files, freshest first. On the same 32-file / 32 MiB fixture the read is ~175 ms, a bystander /health stays at 6 ms p50 against a 2 ms baseline, and four concurrent reads finish in ~630 ms. Two reporting gaps go with it. Anything the bounds clip — dropped files or an exhausted byte budget — now sets `truncated`, so the page cannot show a timeline that silently omits whole Channels. And a scope directory that cannot be read propagates instead of returning an empty list, so the route answers 5xx rather than claiming the workspace's Channels did nothing; a missing directory still means "nothing written yet". The page also stops polling while its tab is hidden and reloads when it becomes visible, so a forgotten tab no longer reads journals every 5 s. Tests pin the two behaviors the wire depended on but the suite did not: reading events that survive only in the rotated file, and clamping a limit above the maximum. Both new mutations are killed, as are the truncation, unreadable-directory, and hidden-tab guards; a bystander-timer test pins that the read yields to the event loop. * fix(web-shell): report a clipped journal tail as truncated The per-file read window introduced with the asynchronous read is 256 KiB, but a journal grows to 1 MiB before it rotates. `readTail` skipped the head of a taller file without telling anyone, so a single busy Channel lost its own older events while the response still claimed `truncated: false` — the exact completeness contract the previous commit set out to fix. A 1 MiB journal of 1020 records answered 255 events with `truncated: false` at `limit=500`. `readTail` now reports whether it started mid-file and the reader folds that into `truncated`. The request budget is also spent in whole tail windows instead of a final partial one, so its flag keeps the precise meaning "whole journals were left out". Three tests close the gaps this exposed: a journal taller than the tail window must report truncation even when the returned rows fit the limit; the budget fixture now uses twenty full-window journals whose combined rows stay under the record limit, so only the budget can explain the flag; and a journal file that cannot be read must fail loudly, like the directory case already does. All four previously surviving mutants — tail flag, budget flag, budget debit, swallowed file-read errors — are now killed. * fix(web-shell): bound Channel event read retention The events read parsed and retained every matching record across all journals before applying `limit`, so an all-Channels poll on a busy workspace held far more than the rows it returns. Trim the accumulator to the newest `limit` once it passes four times that. A dropped record is older than every kept one and the sort is stable, so the answer does not change: a 60-scenario differential run against the previous behaviour produced byte-identical output. Trimming counts as hitting the record limit, so it sets `truncated` too. Closes the remaining non-blocking review items as well. The provider workspace action declares the full query type it already forwards, so the interface no longer suggests that path cannot filter by Channel. The new UI strings use the established 频道 label rather than introducing a second word for Channel beside the existing sidebar entry; the design docs keep 渠道, which is what every other zh-CN design uses. The docs now state the page's own request size (200) apart from the route default (100), and the `truncated` comments describe every reason it can be set instead of only the record limit. The NUL query-key separator is documented as collision-free: instance names reject control characters, journal names are sanitized, and a path cannot contain NUL. * fix(channels): read a journal that fits the budget and keep aligned records Reserving a full 256 KiB window per journal left a small journal unread when the remainder was under one window, even when it covered that journal many times over — coverage lost in exchange for a flag that was already truthful. Reserve what the journal actually needs instead, using the size the enumeration already stats, so the budget flag still means "whole journals were left out" and a 200-byte journal is no longer skipped beside seventeen full ones. A tail window that landed exactly on a line boundary also dropped one complete record, because the first line was discarded whenever the window started mid-file. Read one leading byte and drop that line only when the byte is not a newline. Folding the byte into the first chunk keeps the read to one awaited call per chunk, which is what lets the daemon serve other requests between them — now pinned by a test that compares the event-loop turns a tall read takes against a short one over the same six journals. A synchronous chunk read fails it. Also records why an unparseable instant cannot satisfy `since` while an unfiltered read still returns it, and pins both halves. * fix(channels): collapse repeated reason-filter skips into a counted row A real end-to-end run — the actual GithubAdapter over HTTP against a local GitHub-compatible API, a real agent turn, the page in a browser — exposed an amplification the seeded-journal runs could not. The poll cursor does not advance while an inbound task is recoverable, so a task that failed and is still retryable makes the provider return the same notifications on every poll, and filtered notifications are skipped before any dedupe. Each poll therefore re-journaled the whole batch: with a 2 s poll and 300 filtered notifications the journal rotated and pushed the accepted/posted rows out of the page's 200-row window. Journal at most 20 reason_filter rows per poll and collapse the remainder into one row carrying detail.filtered, which the page renders as a count in both languages. The budget is per poll, so a genuinely new batch is still described in full. Measured against the real adapter: 15 polls served 4,500 notifications and wrote 315 rows instead of 4,500, without rotating. Against a stub that honours both `since` and GitHub's read-state semantics, the redelivery stops once the batch is marked read and a new batch gets a fresh budget. The design docs record the mechanism precisely, including that an in-flight turn cannot cause it because the poll loop awaits pollOnce. * fix(web-shell): keep one Channel event polling loop across visibility restores Restoring tab visibility while a journal request was still pending started a second run. Clearing the timer handle cannot cancel an in-flight run, so both runs settled, both called schedule(), and only the last timer handle was kept — the orphan kept polling forever. Ordinary tab switches on a slow response therefore multiplied journal reads permanently: one overlapping hide/show cycle left two requests per interval, three cycles left four. Guard the cycle with an in-flight flag so a restore during a pending request lets that request finish and reschedule instead of starting a second loop. Both read paths are time-bounded (the SDK's 30 s fetch default and the action timeout), so the guard cannot stall polling. The regression test holds every request pending, overlaps three hide/show cycles, and asserts one request per interval afterwards; removing the guard reproduces the reported four calls. |
||
|
Shaojin Wen
|
95ef544479 |
test(managed-agent): reserve Hosted gate ports below the ephemeral range (#16174)
Hosted ITs reserved each port they hand to Spring, the Harness, a Spring fixture or the Runtime Broker by binding port 0 and closing the socket. The number stayed in the kernel's ephemeral range, so anything binding port 0 or opening a connection could take it before its owner bound it; 9 of 240 recent CI runs lost a cell that way. HostedGateTestSupport.reservePort() now hands out numbers from [20000, 28000) with one counter per fork, probing each on 127.0.0.1, and falls back to kernel-chosen numbers (never repeated in the fork) on a host whose ephemeral range starts below 28000. Every Hosted reserve-and-close site uses it, and HostedMonitorRunFaultGatesIT's private copy of the same allocator is removed. HostedTeamLeadMySqlIT's boot retry looked for the Harness's old move message, which --profile hosted-harness no longer prints; it now treats the exit message as a port conflict, and its teardown deletes the attempt's rows even when settling the tenant's work fails. |
||
|
Shaojin WenandQwen-Coder
|
1301ec30b1 |
refactor(managed-agent): narrow the four deferred store consumers to their role interfaces (#16121)
* refactor(managed-agent): narrow the four deferred store consumers to their roles Complete the deferred migrations from the AgentStateStore role split (#15962). ChildWorkspaceService, WorkspaceRuntimeResolver and ManagedAutomationService now inject SessionQueryStore; TaskCancelCoordinator injects SessionLifecycleStore. Each consumer's calls were verified to be a subset of the target role. They were left on the composite only because the automated detector misreads their this(...) constructor delegation as forwarding. Type-only; the ledger note is updated in both languages. Focused build green: 84 tests (incl. the real-Spring-context ChildWorkspaceServiceTest), ModuleArchitectureTest, 0 Checkstyle violations, all 7 modules compile. Co-authored-by: Qwen-Coder <[email protected]> * docs(managed-agent): broker wiring is no longer a multi-role composite consumer Address the non-blocking review note: EmbeddedRuntimeBroker forwards the store only into WorkspaceRuntimeResolver and ManagedRuntimeCoordinator, which both take SessionQueryStore, so the ledger no longer lists broker wiring among the multi-role dependencies that keep the composite, and names EmbeddedRuntimeBroker as a candidate for the same narrowing. Both languages. Co-authored-by: Qwen-Coder <[email protected]> * docs(managed-agent): fold the four narrowed consumers into the audit tally Address review R1-1: the consumer audit still read 18 single-role consumers while a appended sentence recorded four more narrowed since, so the tally and per-role groups understated the set by four. Fold ChildWorkspaceService, WorkspaceRuntimeResolver and ManagedAutomationService into the query group (4 -> 7) and TaskCancelCoordinator into the lifecycle group (8 -> 9), raise the total to 22, and reword the appended sentence as a deferral-history note rather than a separate migration list. Both languages; mechanical count verifies 22 with the four in their role groups, matching the code's injected types. Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin Wen
|
f724e203ec |
fix(web-shell): bind config sync to the attachment, not the SSE stream (#16170)
* fix(web-shell): bind config sync to the attachment, not the SSE stream The session config-ordering domain was recreated when the event stream was opened, which happens after the load effect awaits its metadata. Because the attachment is deliberately published before that await so the transcript paints one round-trip earlier, a first-prompt config write could still be in flight when the recreation landed. Recreating the domain invalidated the write's ownership token, so a value the daemon had already confirmed was silently discarded, and the reconciliation gated on the same stale token was skipped with it. A reasoning effort chosen before the first prompt was therefore lost for good, and the composer kept advertising the pre-choice value. Establish the domain once, when the session becomes the current attachment, and let stream setup read it instead of recreating it. The window closes without moving when the attachment is published, so first-prompt latency and the early transcript paint are unchanged, and every staleness guard is left intact. Split the live-daemon settings inclusion/exclusion coverage into one test per scenario sharing a single daemon, so each scenario owns a full timeout budget and a failure names the scenario rather than reporting a bare timeout inside a twelve-navigation loop. One existing test asserted the current mode was undefined after an approval-mode write had been confirmed; that held only because the same recreation discarded the confirmed value. Its mock now reports the confirmed mode on later reads and the assertion requires that value to survive, which is strictly stronger. Fixes #16065 * test(web-shell): make the config-sync regression test discriminating Review follow-up. The reasoning regression test could not tell "the confirmed write was applied" from "a later context read republished it": its mock answered every read from the already-updated state, so disabling the write's apply while keeping the reconciliation left it green. Hold every post-write read on a deferred and assert the confirmed value while that read is still pending, so only the apply can satisfy it. Share the live-daemon settings fixture with the existing spawnDaemon harness rather than an inline spawn, poll and kill sequence. Boot failures now carry the daemon's stdout and stderr instead of a bare Playwright timeout, and the helper clears its boot timer and listeners on settle. The helper appends an initialize-timeout flag, so this is not a pure refactor; cliBin stays explicit so TEST_CLI_PATH cannot redirect it. Record why the bind-site domain reset cannot be pinned in isolation: read-sequence monotonicity and the outgoing stream's own teardown already invalidate anything captured before a re-attach. |
||
|
Shaojin Wen
|
c26a32006a |
fix(serve): keep record_artifact guidance out of Managed Runtime worker results (#16148)
* fix(serve): keep record_artifact guidance out of Managed Runtime worker results The worker builds its tools from a configuration that left artifact recording at the core default. A Hosted Session declares no record_artifact and the worker forwards only response parts, so every Shell result for a command not proven read-only told the model to call a tool it does not have, and write_file claimed artifact-like files were recorded when nothing records them. The reminder's read-only check also loaded the Bash AST parser in every sandbox for each successful Shell call. Turn artifact recording off for the worker's tools regardless of env flags, as the shared execution worker already does. The local Managed engine's provider worker, which hands artifacts to its client, is unchanged. Fixes #16147 * fix(serve): keep artifact guidance out of the CSI files worker too Add a Config option, artifactRecordingSupported, for hosts that expose no record_artifact tool and forward no artifact metadata. Unlike artifactEnabled, the artifact env flags cannot turn recording back on. The Managed Runtime worker, the CSI files worker and the shared execution worker now pass it instead of overriding isRecordArtifactEnabled on the instance. The CSI files worker's write_file and edit no longer tell the model that a file was recorded as a workspace artifact. The tests now derive that claim from formatRecordArtifactReminder() instead of a literal, cover edit, and assert that a worker Shell call makes no call into the Bash parser. |
||
|
Shaojin WenandQwen-Coder
|
47fcd5131d |
fix(acp-bridge): compact live tool-result displays for session replay (#16172)
* fix(acp-bridge): compact live tool-result displays for session replay Opening a Web Shell session whose live turn edited large files could take tens of seconds: the session/load response embeds the daemon replay journal, and each edit result event carried the whole old and new file contents twice (diff content plus rawOutput). One observed load response reached 39.6 MB although the on-disk transcript was only 4.3 MB, because the recording path truncates file-diff displays while the live ACP path emitted them raw; adaptive journal growth then retained tens of MB per session and every open re-downloaded and replayed all of it (#16171). Compact the display where it becomes wire content and rawOutput, reusing the recording-purpose compaction so warm replay shows exactly what cold replay from the transcript already shows. Small diffs stay byte-for-byte intact; oversized edits degrade to the saved-session preview. Transcript records can carry leaner displays than the live producer emits (blocked spawns have no task prompt), so the shared compaction utility is hardened to pass non-string fields and malformed list entries through instead of throwing; well-formed displays compact exactly as before. * docs: record real-daemon A/B verification for live replay compaction Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin Wen
|
a6f1885833 |
test(autofix-pr): re-run a timed-out job and lose its answer in the chain test (#16138)
The fake gh only re-ran FAILURE jobs and applied loseNextAnswer to comment POSTs only, so the helpers' handling of a timed-out job's re-run and of a re-run whose answer is lost could not be reached from the chain test. - `run rerun` re-runs FAILURE and TIMED_OUT jobs, refuses a run with no failed job, and with loseNextAnswer queues the jobs then fails the way gh words a non-403 error. - A job conclusion the fake does not list is refused as unsupported, on every path that reads a run. - FAKE_GH_WORLD, and the names derived from it, are read after the call is logged and inside the guard, so a missing or malformed world shows as unsupported instead of a bare crash. - New case: a timed-out job whose re-run's answer is lost is left for a person; when the re-run times out again, rerun refuses a second one, and status lists both. The re-run's attempt key is pinned, here and in the re-run case. - Design §12 (both languages) lists the conflict case and the new case. |
||
|
Shaojin Wen
|
14de8a035e |
feat(verify-pr): inherit --publish consent on same-session re-verification (#16112)
* feat(verify-pr): inherit --publish consent on same-session re-verification A maintainer who ran `/verify-pr <n> --publish` and then typed the "有更新了，重新验证" follow-up trigger had to confirm publishing again: the skill inherited the previous round's coordinates but deliberately excluded publish consent, so every re-verification of an already published PR repeated a confirmation the maintainer had just given. Let a follow-up round inherit publish consent from the most recent round for the same PR in the same session. The round must announce the inheritance in one line before posting, `--no-publish` on the trigger opts out and breaks the chain, and the most-recent-round semantics keep a quiet round from reviving an older consent. Cross-session rounds still require an explicit request: the phrase alone posts nothing, and a published comment found on the PR is evidence of a past round, not consent for a new one. The CI lane and watch mode are unchanged. Update the two pin tests that fixed the old per-invocation-only rule and pin the inheritance, announcement, opt-out, and cross-session boundaries. * fix(verify-pr): carry inherited publish consent through follow-up chains Review of the first round found the consent chain broke after a single inheritance: R1 invoked with --publish, R2 a bare trigger that inherited and published, R3 a bare trigger whose most recent round (R2) was not itself invoked with --publish — so R3 fell back to asking again, the exact friction this change exists to remove. Model consent as a per-PR state of the session instead of keying it to the latest invocation: a --publish invocation grants it, a fresh /verify-pr round without --publish or a trigger carrying --no-publish withdraws it, and a bare trigger keeps it unchanged, so consecutive bare triggers publish under the original grant until a round withdraws it. Watch rounds keep their own standing consent and never change this state. Also close the two review suggestions: the Hard rules bullet now recognizes a request inherited within the same session, and the argument hint lists --no-publish so the opt-out is discoverable. Pins cover the grant/withdraw/keep mechanics, the three-round chain, the watch boundary, the Hard rules clause, and the updated hint. * fix(verify-pr): grant session consent on any explicit publish request Second review round found two gaps. The design docs for the assets coordinates still stated that publishing consent is per invocation, which reads as an invariant and could silently revert the feature; both language versions now defer to the skill's session-consent rule while keeping "coordinates are not consent". And the grant keyed on the literal --publish flag, while the publish rule also accepts an explicit request in words — a words-request round published but seeded no consent, splitting the friction this PR removes by request form. The grant now rides on the explicit request, flag or words alike, and the withdrawal matches it. Also close the deep verification's coverage survivors: the withdrawal is pinned as one contiguous sentence (a fresh round keeping consent now fails), the chain's upper bound, the announcement's naming duty, and the evidence-of-a-past-round clause each have pins, and an inherited publish now carries its one-line announcement into the comment body immediately before the attribution footer, so a PR reader can tell it from a per-round request. |
||
|
Shaojin Wen
|
9336986835 |
feat(web-shell): make GitHub channel configuration usable (#16096)
* feat(web-shell): make GitHub channel configuration usable The channel editor rendered every platform through one generic form, which left the GitHub channel hard to configure and persisted settings its adapter never reads. - Render string-list fields that declare options as a checkbox list, so the fifteen notification reasons are pickable and explained instead of typed from memory. Unknown values written by hand stay visible and removable rather than disappearing on the next save. - Lock the token input while local gh authentication is selected, clear a stored token when that switch is turned on, and drop a token that an existing useLocalGh selection has already overridden. - Hide and stop persisting the direct-message policy for repository-scoped channels: the registry injects privatePolicy into every manageable channel, but the GitHub adapter only produces group envelopes, so the value never gates anything and only misleads. - Name allowlisted groups repositories, validate owner/repo, and show the GitHub Enterprise API base URL shape. - Split the catch-all credentials section into credentials, connection, and notification routing. * test(web-shell): pin the GitHub editor capture to the reason checklist The screenshot scenario now fails if the filter falls back to a free-text input, so the published picture cannot drift away from the behaviour the pull request claims. * test(web-shell): pin the section split on a non-GitHub channel The connection and routing sections key on field names, so they regroup gitlab and wecom fields too. Assert that split on a gitlab descriptor instead of leaving it implied by the GitHub assertions. * fix(web-shell): match GitHub channel values the way the adapter reads them Review round 1 on the editor found three places where the new controls disagreed with the runtime or left dead configuration behind. - The reason checklist compared values case-sensitively while the adapter lowercases both sides, so a stored COMMENT rendered as a stray sixteenth box beside an unchecked canonical one; ticking only mention then left comment notifications flowing. Compare case-insensitively and keep unknown entries in their original spelling. - The pairing guard had no test witness: deleting it left the whole suite green while a legacy privatePolicy reopened the direct-message panel for a repository channel. Pin both directions — legacy private pairing must not open the panel, group pairing still must. - Only privatePolicy was cleaned up; the equally unread allowedUsers survived every save and was still presented as an authoritative allowlist in the pairing panel. Drop the whole private-access set and stop showing it there. * fix(channels): match GitHub repository groups regardless of case The editor started validating repository allowlists as owner/repo, but a repository path is case-insensitive while the runtime matched the group key verbatim. An operator who typed qwenlm/qwen-code passed validation, saved, and got a channel that connected and then dropped every notification as not_allowlisted — the exact failure the validation was added to prevent. Normalize on both sides so neither can drift: - The editor writes the canonical lower-case key and shows stored allowlists in that form, so re-saving an older mixed-case entry migrates it. The per-repository settings are looked up under the normalized key, so retyping the case no longer discards that repository's sender policy. - GroupGate and ChannelBase accept a platform group-ID normalizer, used for the allowlist, the sender policy and the per-group overrides alike. Normalizing only admission would have let a case-mismatched repository fall back to the open sender default, which is worse than dropping it. - The GitHub adapter supplies the lower-casing normalizer; every other platform keeps matching group IDs verbatim. Pairing approvals are unaffected: they are keyed by the chat ID the platform reports, so they were already self-consistent. * fix(channels): resolve the group history limit through the same lookup Self-audit of the repository-casing fix found a third direct read of config.groups: the per-group history limit still matched the key verbatim, so a platform with case-insensitive IDs silently lost that override while the allowlist and sender policy honoured it. Route it through groupConfigFor and pin it with a case-insensitive channel test. * fix(web-shell): keep the gh switch reversible and refuse ambiguous repositories Review round 2 plus the independent verification round found three defects. - Turning local gh authentication on queues the stored token for removal, because a token outranks gh. Turning it back off left that intent in place, so Save demanded a credential the operator never chose to delete and the switch could not be undone without retyping a PAT. Flipping it back now restores the stored token. - Collapsing repository aliases could widen access. A stored allowlist may hold Owner/repo and owner/repo with different settings; the runtime resolves an exact key first, so keeping only one of them on an unrelated save dropped the effective sender restriction. The editor now refuses to save until the operator resolves the duplicate, and shows the allowlist without repeating the same repository twice. - The reason descriptions described event types instead of the subscription reasons GitHub actually reports: comment means the account commented on the thread, state_change that the account changed its state, ci_activity that a workflow it triggered completed, and manual that it subscribed by hand. Rewritten in both languages against the REST notification contract. The visual scenario also captured the dialog without scrolling, so the checklist sat below the fold and the published picture did not show the feature. It now scrolls the filter into view, asserts it is fully on screen, and captures it separately. |
||
|
Shaojin WenandQwen-Coder
|
60f5c1ed95 |
docs(managed-agent): record the completed store constructor injection in the ledger (#16118)
#16061 replaced the seven ManagedAgentStore public setters with a single @Autowired constructor holding final references, but the parent structure-review ledger still described the setters as current state and framed the conversion as future work. Update the D1 row note, the prose and the P1 store narrative to the implemented state in both languages, keep the setter investigation as pre-conversion context, and leave the physical aggregate split as the remaining work. Figures re-measured against origin/main (0 public setters, 1 @Autowired constructor, final fields, 7657 lines); prettier and bilingual heading/link checks pass. Documentation-only. Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin WenandQwen-Coder
|
64651cde11 |
docs(managed-agent): reconcile the structure-review ledger with the broker extraction (#16109)
* docs(managed-agent): reconcile the structure-review ledger with the broker extraction The binding registry and execution workflow slices (#16029, #16084) extracted the reconciliation/recovery/cancellation cluster, but the parent structure-review ledger still described a 6,329-line broker with only the cooldown and hold inventory out, and still called the cluster entangled and waiting for a registry. Update the D1 god-class rows, the surrounding prose, the mirrored test size and the P1 extraction narrative to the measured current state (4,270-line facade, nine collaborators), in both languages. Every stated figure was re-measured against the source; prettier and the bilingual heading/link checks pass. Documentation-only. Co-authored-by: Qwen-Coder <[email protected]> * docs(managed-agent): put the met Registry prerequisite in past tense Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin Wen
|
0a7f260704 |
fix(managed-agent): reach the listeners the server resolves in upgrade start and verify (#16051)
* fix(managed-agent): reach the listeners the server resolves in upgrade start and verify
start and verify derived the control-plane URL from server.env's
QWEN_MANAGED_AGENT_SERVER_ADDRESS and SERVER_PORT, and the Harness URL from
its QWEN_MANAGED_AGENT_HARNESS_BASE_URL. A port or address set anywhere else
(config/application.yml, an argument, SPRING_APPLICATION_JSON, a relaxed
SERVER_ADDRESS) or a management.server.port of its own made start wait out
--timeout on a port nothing served.
ResolvedConfiguration now also prints serverUrl, healthUrl and
harnessBaseUrl as Spring binds them: server.port, server.address and
server.ssl, with a wildcard reached on loopback, and the health endpoint by
Spring Boot's management-port rules. start resolves them before it changes
any unit, verify before its rows. --server-url still names the control
plane; a port of 0 or below refuses until it does; a configuration that
does not resolve falls back to the server.env URLs with a notice.
* fix(managed-agent): keep the health name for a blank path mapping
A blank management.endpoints.web.path-mapping.health, such as an unset
${HEALTH_PATH:}, is ignored by Spring Boot's MappingWebEndpointPathMapper,
so the endpoint stays at /actuator/health. The resolver turned it into an
empty segment and sent start and verify to /actuator, where start waited
out its timeout on a healthy server. Fall back to the name when the
mapping is blank, as Spring does.
Also from review:
- the fallback notice says only the Harness URL comes from server.env
when --server-url names the control plane;
- start's success line names the API URL next to the health endpoint;
- verify with --server-url resolves the configuration only for --tenant,
as it did before, since it does not reach the Harness.
* fix(managed-agent): refuse listener URLs fetch cannot dial in upgrade start and verify
A link-local server or management address keeps its IPv6 zone in the
resolved URL (http://[fe80:0:0:0:0:0:0:1%1]:8080), and the WHATWG URL
parser behind fetch rejects every zone spelling. The health probe caught
the TypeError as "not UP", so start waited out --timeout on a URL it could
never send. The same held for an out-of-range port, a server.env address
with a zone on the fallback path, and a --server-url that does not parse.
listeners() now checks both URLs with URL.canParse after the no-fixed-port
refusal, before any unit changes, and names the zone when that is why. The
resolver test pins that the zone is kept, so the script can say so. The
verify test lists the health probe with the session reads instead of
slicing it off.
|
||
|
Shaojin Wen
|
d36fdbdfe5 |
test(managed-agent): gate detach while a tool result is unknown (#16098)
The MCP detach must refuse before it releases anything when an operation is recovery_blocked/outcome_unknown. Start an ext://hold read, kill the stdio fixture, and call the Harness detach route; the case requires the documented refusal, no release control, and the operation still unknown. The Hosted fork moves by the case's measured 27.4 s, booked at 55 s. |
||
|
Shaojin Wen
|
7b4b353750 |
test(managed-agent): gate the two-server first release refusal (#16089)
Extend HostedMcpRestartFaultGatesIT with the FG12d cell #15750 still carried: a Session configured with two MCP servers, the first release refused before forwarding, and the same-id retry releasing both. The case adds fixture2 only for its own boot, waits for both configurations to settle, and requires three release controls: 503, the same operation id answered 200, then the other configuration's release answered 200. Both fixture processes must exit. The Hosted fork moves by the case's measured 16.2 s, booked at 32 s. |
||
|
Shaojin Wenandverify
|
4a5c487681 |
test(managed-agent): gate the expired-cursor resync answer (#16076)
* test(managed-agent): gate the expired-cursor resync answer FG9e now advances the Session stream's replay floor past a lagging client and requires one `agent.session.resync_required` frame on both the public and WebShell streams, with no event id and the floor and snapshot cursors it must reload through. The same cursor on the plain events route answers `409 cursor_expired`, and the floor itself stays readable. This closes the expired-cursor cell the FG8 tracker (#14163) deferred and #15750 carried. The case measures 21.3 s locally; the Hosted fork budget and Verify step move with it. * test(managed-agent): wait for projection coverage before advancing the floor Awaitility treats a lambda that returns without failing as satisfied, so the coverage guard must assert rather than return. Without this a poll that lands before the lead's Turn is projected would skip the advance and dereference a null window. * docs(managed-agent): correct the resync case's tracker references FG9e now carries the third case, so the relay-hold sentence must name only the two resume cases. The FG8 record no longer predicts a slice-2 landing and points at the FG9e delivery instead. * ci(managed-agent): record the resync case's measured range The 60 s fork-budget entry was written from one 21.3 s run; the final audit runs measured 21–23 s, so state the range the budget covers. * docs(managed-agent): fix the stale step-ceiling header and pin note The Hosted job's opening parenthetical still summed 291 while the tail, the Verify step and the job ceiling had moved to 292/202/325. Also say the replay-floor flag is a default pin, not something currently off. --------- Co-authored-by: verify <verify@local> |
||
|
Shaojin Wen
|
cea333133c |
fix(managed-agent): correct the child result mirror on consumption (#16074)
* fix(managed-agent): correct the child result mirror on consumption classify() mirrors the child_run delivery into ma_delivery at classification time. A parent's consuming revision can land after that snapshot, leaving a CHILD_RESULT row at `delivered/accepted/pending` forever while the record itself reads `consumed`; FG10b's awaitConsumed then times out even though the walk completed. The parent's own consumption commit now corrects that row in the same transaction, and classification reads the record row with FOR UPDATE so a consumption cannot slip between its read and the mirror write. A row not written yet is left to classification, which reads the record's current state. Adds focused H2 coverage for the mirror correction and the commit-path wiring. Issue: #15744. * fix(managed-agent): lock only the child run's record row The classification's locking read matched the session scope without the record's primary key. MySQL therefore scanned the Session's records and took next-key locks over the rows that scan touched, so an unrelated child run's commit in the same Session could wait behind a classification that had nothing to do with it. Name the record key in the lookup. A real MySQL IT holds the classification open and proves the unrelated record still commits; the same IT fails with the session-scope lookup under a lock wait timeout. Issue: #15744. |
||
|
|
2aa908dc70 |
fix(managed-agent): block ledger-less agent gap fills and pin the A6 refusal arms (#16015)
* docs(managed-agent): mark the B2 identities and live SSE arm packaged The G3 recovery-matrix status paragraph still listed B2's attempt/stream identities and live SSE view as remaining focused/integration evidence after PR #15993 packaged them into the model-round arm, and its admission-count clause predated the arm's existing admission audit. Both languages now name the packaged B2 observations and link the B2 design doc; the remaining list keeps only A3's rewritten-input/Hook half, A6 and R2. * fix(managed-agent): block ledger-less agent gap fills and pin the A6 refusal arms A6's first clause is an evidence-semantics rule: unknown producer evidence must block, never be laundered into proven absence. Two gaps violated its evidence surface. The Hook/MCP saved-batch ownership refusal — the only backstop for the extensions-enabled continuation entry — had zero tests: deleting its clause kept every existing suite green (demonstrated before pinning). And the parked-round gap fill answered a stranded agent call with "the call never ran" when the child ledger was unavailable, although only a present ledger's missing record is positive never-started proof (admission commits before dispatch); child-lane config/version drift between generations makes that arm reachable with a live child. The fill now throws the family's retriable recovery error for an owed agent fold with no ledger; journaled answers keep their silent replay and non-agent calls keep their honest fold. Three ownership cases (hooks, mcp, hooks+extensions) and four gap-fill cases pin the surface; each deletion control reds exactly its named case. Focused regression 894/894, typecheck, ESLint and Prettier clean. Bilingual design doc records the A6 evidence map and scope boundaries; the parent matrix status notes the landed focused evidence. * fix(managed-agent): widen the A6 ledger guard and exempt the terminating routes R1 review fixes, each verified against the code before fixing: - The guard covers the whole child-run ledger family (agent || workflow), matching the module's own family readers; a stranded workflow call with an unavailable ledger no longer receives the fabricated never-ran fold. - An abort exemption keeps the pre-aborted cancel and interrupted-settle fills terminating with their pre-A6 abandonment folds; only the live-signal continue route blocks. Without it the guard wedged cancel in a deterministic failure loop (regression witness demonstrated at route level). - Two route-level cases build the child-lane drift into the replacement load (suppressChildAgents on the version-0 orphan parked shape): continue blocks retriably (409) with no fabricated fold, cancel still settles. - The owner-gate matrix drops the vacuous hooks+extensions row (the harness entry's extensions clause is recorded as a follow-up) and gains both gate directions: a stale-revision commandStep refusal and a matching-revision acceptance that keeps the A3 commandStep follow-up working. Nine delete-controls red exactly their named cases on a quiet tree with known-good baselines; focused regression 8 suites 1583/1583, typecheck, ESLint and Prettier green. Bilingual design docs name all three fill callers, the residual trade-off, and the narrowed G2 scope. * fix(managed-agent): extend the A6 ledger rule to every never-ran writer R2 review fixes, each verified against the code before fixing: - The resumed-turn answer (hosted mode) and the command-wait gap fill now refuse a ledger-less family call on a live signal, mirroring the agent fill; their terminating callers are pre-aborted so cancel and settle keep terminating. - Every ledger-less family fold on the terminating paths — the pre-aborted fills and the 'all' settlement — writes a dedicated unknown-outcome answer (executionStatus unknown, record retained) that claims nothing, never "never ran" and never "cancelled". - CHILD_LEDGER_TOOL_NAMES becomes the single family-membership fact; the six inline two-name literals across recovery and the tool turn now read it, so a third ledger-backed tool joins in one place. - The owner-gate matrix gains the extensions row its comment claimed: a Hook owner with extensionConfig and no commandStep is refused, and the extensions-exemption mutant reds exactly that row. Five delete-controls red exactly their named cases on a quiet tree with failure-list adjudication; focused regression 8 suites 1587/1587, ESLint, Prettier and typecheck green. Bilingual design doc records the round. * fix(cli): preserve cancel escape for A6 recovery blocks Co-authored-by: Qwen-Coder <[email protected]> * test(cli): pin journal-only workflow ledger refusal Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: 易良 <[email protected]> Co-authored-by: Qwen-Coder <[email protected]> |
||
|
Shaojin Wenandverify
|
6e49388c4a |
docs(serve): refresh daemon usage guidance (#16056)
Replace obsolete alpha-era deployment and persistence limits with current behavior. Document authenticated background startup, token sidecar lifecycle, and service-manager choices. Co-authored-by: verify <verify@local> |
||
|
Shaojin WenandQwen-Coder
|
417e81ec4d |
fix(review): keep prose re-review requests aimed at the PR (#15998)
* fix(review): keep prose re-review requests aimed at the PR
A re-review request typed as prose ("有更新了，重新评审") carried no
target the parser could classify, and the parser did not refuse it:
every non-flag, non-number, non-URL token classifies as a file target,
so the sentence became a file path, and prose typed beside a PR number
stole the target while the number was demoted to extraTokens.
- parse-args: PR-shaped tokens now outrank file-shaped ones regardless
of arrival order; an explicit ./<number> spelling stays a file target.
- review skill: a Step 1 intent-routing clause resolves a prose request
to the PR the conversation names (non-empty inputs only, flags kept,
the CLI args file preserved), and the description plus the resume
verdict bullet exclude the updated-PR case from --resume.
- fetch-pr: the head-moved resume refusal now says the fresh run IS the
re-review and names the plain invocation for next time; the
restart-bound semantics are unchanged.
- docs: both --resume surfaces in the user guide carry the exclusion.
Pinned by 12 new parse-args cases (the key one red before the fix), a
SKILL.md pin test, and a fetch-pr stderr assertion; DESIGN.md records
the incident as "The prose re-review request".
* fix(review): fence the prose routing's local and posting boundaries
The review round on #15998 caught two defects in the new routing clause
and one regression behind its non-blocking nit:
- Option-only invocations (--fix, --effort low) matched the routing
condition and would have been retargeted into the conversation's last
PR, disarming --fix. The clause now names the bare and option-only
shapes as non-intent and leaves them to the parser verbatim; two new
parser cases pin that contract.
- The clause promised posting authority for a --comment carried through
the resolution, but the write gate re-parses the CLI's verbatim record
on purpose, so a prose record ends at submit with `unbound`. Posting
is now the resolution's hard boundary: present the resolved invocation
and ask for an explicit re-run instead of completing a review whose
submission is already refused. A new submit-suite test pins the
refusal beside the explicit re-invocation's acceptance.
- The PR-shape reorder demoted a malformed URL behind the assigned
target and lost its specific diagnostic to the generic extra-argument
line. The invalid-url refusal is hoisted ahead of the target-assigned
branch, position-independent, pinned by a dedicated test.
DESIGN.md records the review round; the PR body carries the same
boundaries in both languages.
* fix(review): guard prose routing against local-write intent, qualify scoping
The deep-verification round on
|
||
|
Shaojin Wen
|
859fd0b39c |
fix(core): keep the whole Kimi K3 generation at a 1M context window (#16048)
* fix(core): recognize the whole Kimi K3 generation at a 1M window The K3 generation is served as `kimi-k3`, `kimi-k3.1-pro` and, from Kimi Code, as the bare `k3.1-pro` / `k3.1-pro-highspeed` aliases. Two gaps left the K3.1 tiers with the wrong window: - the curated table only matched ids starting with `kimi-k3`, so the bare aliases fell through to the 256K Kimi fallback; - the models.dev catalog is consulted before that table, rounds the 1M window up to 1,048,576, and lists each K3.1 sub-version under both its dotted and dashed spellings, so the catalog value overrode the declared window and compaction thresholds fired too late. Match the generation once and share that pattern between the curated table and the catalog correction, replacing the single-id catalog entry that the next sub-version would have slipped past. * fix(core): keep Kimi's k3-256k out of the K3 1M window rule Kimi Code serves `k3-256k` at 262,144 tokens alongside the 1M `k3` and `k3.1` tiers, and the generation pattern swept it up: with no catalog entry it resolved to 1M instead of the 256K family fallback, and the generation-wide catalog correction overwrote a correct cached 262,144. Name the variant ahead of the generation row, exclude it from the shared pattern so the catalog correction cannot raise it, and pin its window in the id-keyed corrections for a catalog that omits or overstates the entry. Fixes the P1 on #16048. * fix(core): give Kimi's bare K3 aliases the same modalities as the prefixed ids Kimi Code serves the K3 generation without the `kimi-` prefix, but the modality table still matched only `kimi-k3`, so `k3.1-pro`, `k3.1-pro-highspeed` and a bare `k3` fell through to the text-only Kimi fallback while their `kimi-` spellings advertised image and video input. Match the prefix optionally, and name the 256K variant ahead of that row: Kimi Code reports image input only for `k3-256k`, no video. Addresses the R1-1 suggestion on #16048. |
||
|
Shaojin Wenandverify
|
7ab044d608 |
docs(readme): add daemon mode quickstart (#16053)
Explain how qwen serve supports shared sessions and browser/API clients. Document lifecycle commands and loopback authentication risks. Co-authored-by: verify <verify@local> |
||
|
Shaojin Wen
|
0f91cdedae |
fix(verify-pr): count suite runs and skip notices on closed PRs (#16050)
Follow-ups deferred on #16013: - A round whose only checks were existing suites had a total of 0 once suites moved to gates, and the checker refuses that. The skill now counts each suite run as one assertion, as it already did for a mutant run. - Gates that record no results (an empty list, or entries counting nothing) no longer silence the size warning. When no suite ran, the round says in the report what its count holds. - The blank-suite rule is JSON whitespace on both sides. jq's \S and JS's trim() disagreed on U+0085 and U+FEFF, so the checker accepted a list the publisher dropped. Also, a round can queue for hours: the publisher no longer posts a notice on a PR that closed meanwhile, unless it is finalizing its own live status. Four such notices landed on #16013 after it merged. |
||
|
Shaojin Wen
|
835b080f85 |
test(autofix-pr): run a watch's helpers in a chain against a fake gh (#16017)
* test(autofix-pr): run a watch's helpers in a chain against a fake gh
Add the integration test the design listed as not written yet. The
helpers run one after another as the skill runs them, through the CLI's
parser, against real git (a bare remote, the user's clone, the watch's
worktree) and a fake gh first on PATH, which they spawn as they would the
real one. The gate runs as `schedule` printed it, split by a real shell.
The fake answers from a JSON description of the PR and reads the PR's
head from the bare repository, so a push moves it. It answers only the
fields a GraphQL query selects, checks the host and the Int PR number,
and logs anything it does not know, which fails the test even when a
helper swallows the failed call.
Five cases: a watch from start to merge (with a reply whose answer was
lost), a push by someone else during a round, a re-run spent then
refused, the per-check fix budget across real pushes (one head's result
asked by commit), and the author's push access. Each also checks the
writes made to GitHub, that pushes are fast-forwards onto the PR's
branch, and that the user's checkout is unchanged.
* test(autofix-pr): check the field behind a GraphQL alias in the fake gh
The fake read an alias's response key and never the field it stands for,
so `root: nonexistentField(first: 1)` was answered like the real
`root: comments(first: 1)`. It now records the field and its arguments
behind each alias and answers only the one alias it renders. The message
for a query it does not answer no longer calls every such query a paging
one.
* test(autofix-pr): pin the conflict seam and the watch's refs in the chain test
- The fake answers `root` only through its alias: selected bare, as
`root { ... }` or `root(first: 1)`, it is a field GitHub does not have.
It also refuses a run with no job instead of answering it as a pass.
- The user's checkout is compared with its branches and worktrees too,
and only `.qwen/tmp` is left out of its status, so a branch or a file
the watch leaves behind shows. The re-run and fix-budget cases end
with the cleanup a stop runs.
- A sixth case leaves a conflict for a person: `reply` records it, the
gate stays quiet on that head and on the head the watch pushes next
while GitHub still answers UNKNOWN, and someone else's push brings it
back.
|
||
|
Shaojin WenandQwen-Coder
|
06c1f98971 |
fix(verify-pr): count test suites apart from the round's assertions (#16013)
* fix(verify-pr): count test suites apart from the round's assertions
Rounds folded whole workspace suites into assertions.json, so the
published "Scripted assertions" line measured the repo, not the round:
37 of the 252 counted reports in the 30-day census exceeded 1000, and
one round's 7737 held a few dozen checks of its own, the rest being
five suites counted again for every mutant run.
assertions.json gains gates: [{ suite, pass, fail, skipped? }], one
entry per existing test suite run as a targeted gate. The skill says
suite results and mutant runs stay out of pass/fail/total, and that a
gate red the PR causes still reaches the verdict as an assertion. The
checker refuses a malformed list and warns when over 1000 assertions
come with no gates. The publisher prints the gate totals on a line of
their own, numbers only, and does not let them decide the verdict.
Also, deferred from #15938: the paragraph for a merge-ready round with
failures now states the checker's rule instead of inviting the reader
to excuse the failures, and the artifact-check guard's empty-value legs
are pinned.
* fix(verify-pr): warn on empty gates and mirror the checker's shape rules
Address the three stage-2 triage suggestions: an empty gates list no
longer silences the suite-sized warning; the publisher's jq refuses
blank suite names and non-count skipped values (false/null) instead of
defaulting them, so a shape the checker refuses never renders totals;
and the design docs say 'all three' / '三者' where the rule count grew
from two to three.
Co-authored-by: Qwen-Coder <[email protected]>
* fix(verify-pr): give the gates shape rule one witness and honest totals
Address the five R1 review suggestions:
- one shared fixture table drives both the publisher's jq and
checkArtifacts, and both bound counts at 2^53-1, closing the 1e309
divergence where jq clamped an overflow the checker refuses
- the skipped clause prints only when some entry records the field,
so an unrecorded count is no longer published as a measured zero
- a trusted verdict over gate reds carries one advisory sentence in
both languages pointing at the assertion ledger; still verdict-inert
- pin the skill's compensation sentence and name the gates-size
warning in the local clear-list and the CI-path sentence
- bump the workflow size baseline to 375110: the parity comment, the
conditional clause and the advisory exceed the remaining allowance
Co-authored-by: Qwen-Coder <[email protected]>
* fix(verify-pr): print the skip clause only for fully recorded gates
Address the two R2 review suggestions:
- the clause now requires every entry to record skipped; a mixed list
published a partial sum as the run's skip total
- pin the advisory's trusted-verdict guard from the false side: an
untrusted run with gate reds prints the totals but no advisory
- fixtures: a mixed list drops the clause, a recorded skipped:0 keeps
it, and a timeout headline over gate reds stays advisory-free
Co-authored-by: Qwen-Coder <[email protected]>
---------
Co-authored-by: Qwen-Coder <[email protected]>
|
||
|
Shaojin Wen
|
143a8b0653 |
ci(sdk-java): run the Hosted admission gates even when the Hosted verify fails (#16010)
* ci(sdk-java): run the Hosted admission gates even when the Hosted verify fails
The admission step had no condition, so it ran only when every earlier
step passed. The Hosted verify fails most nights, and the step was skipped
on all 13 nights from 10-14 to 10-30 that the verify failed, including the
three since the Runtime Broker fault gates stopped being skipped the same
way. It ran on the other three nights and passed each time, in about two
minutes.
The step rebuilds the module with `clean verify` and needs only the CLI
bundle, the installed SDK and the MySQL service. Each gate class works
under its own random tenant, and on green nights it already runs against
the database the Hosted verify used. So it takes the condition the fault
gates and the O4 step use: run unless the run was cancelled, as long as
the SDK install succeeded.
The class check after it reads only the admission step's reports. It now
follows that step's outcome instead of the job status; otherwise it would
still skip after a red Hosted verify while the step it audits ran.
The pin for the fault-gate condition now covers both steps, and a new case
pins the class check's condition and position. The setup-step assertion
now names the offending steps in its message: vitest abbreviates the
array once a step name is long enough, and the install step's own name
already is.
* ci(sdk-java): keep the O4 gates reachable on red-verify nights
The admission step now runs on red-verify nights too, so the job's worst
case sums all four capped steps (291 min) plus the uncapped setup. Raise
the hosted-harness-mysql ceiling from 304 to 324 so a slow setup (the CLI
install retries three times) cannot cancel the job before the O4 gates,
whose !cancelled() condition would record them skipped.
Close the two pin gaps the review found: the red-verify condition test
gains the O4 row (all three conditioned steps are now pinned), and the
admission class-check test pins that the check step carries no
continue-on-error, so a dropped gate class still fails the job.
All three pins are mutation-witnessed at this head (ceiling back to 304,
O4 if: deleted, continue-on-error on the check step each redden exactly
their case); the nine sdk-java.yml parsing suites pass 709/709.
* ci(sdk-java): boot the admission gates on their own database and bump the size baseline
Now that the admission step runs after a red Hosted verify, it must not
share that verify's database. Managed schema validation refuses any table
outside the managed set, and four Hosted ITs create fg*_fire tables they
drop only during cleanup (the relay classes only while their Spring context
is active), while the isolation extension truncates only ma_ tables. A
class killed before its cleanup would leave one behind and fail both
admission gates at boot. On a MySQL 8.4.6 rig, an fg9b_fire table in
hosted_harness_test made the base command fail both classes with "Managed
schema must contain exactly the 38 target tables", while this command
passed both on a fresh qwen_admission_ci. The step now creates that
database the way O4 creates qwen_o4_ci.
The pin test adds the O4 row, pins that the class check has no
continue-on-error, custom shell or altered command, that no gate command
swallows its failure, and that admission boots on its own database. Both
steps' names are checked before use.
sdk-java.yml outgrew the ratchet: main's copy was already 3971 bytes over
its recorded 144802 without a bump, and this change adds the rest. Even
without its comments the file would exceed the ceiling, so the baseline
moves to the file's size.
* test(ci): pin the admission step whole and keep the install ahead of the verify
The admission step's command, env and shell are now pinned with literal
values. The hosted-gates class check counts a failed class as one that
ran, so an `||` fallback, a pipe, or -fn in the command or in MAVEN_ARGS
would have turned a red admission gate green, and a second -Dmysql.url
would have pointed it back at the shared database. A list of forbidden
substrings caught some of these and rejected harmless commands, so the
fault-gate and O4 rows go back to pinning only their condition and
continue-on-error; their commands are unchanged by this PR.
The three gate rows also pin that the SDK install comes before the Hosted
verify. After it, a red verify would skip the install and every gate with
it, on exactly the nights the condition is for.
Comments: the job comment opened with stale ceilings (162 + 20 + 50 + 20
= 252); it now gives the measured setup time (under 10 minutes on every
nightly from 10-14 to 10-30), states that what changed is the timed-out
Verify path, which used to skip admission, and says the CLI install makes
up to three attempts rather than retrying three times. The O4 row comment
says O4 has carried its condition since it was added.
* test(ci): derive the red-verify contract from the job and pin the setup allowance
The red-verify condition was asserted over three hand-listed step names,
so a gate added to the job later sat outside every assertion and a red
verify would skip it silently, as it skipped the admission gates. A new
case derives the contract from the steps after the Hosted verify:
- Only the two report checks that need the verify's output may run on a
green job alone. Every other step's condition must be `always()`, or
`${{ }}` around `always()`, `!cancelled()` or `failure()`, optionally
narrowed by one step's outcome. Any other condition either has no
status function, so GitHub prefixes `success() &&`, or still reads the
job status. A narrowing step must itself run regardless of the verify.
- No step after the verify carries continue-on-error.
- Every step that invokes Maven, matched as a word so folded commands,
wrappers and quoted paths count, carries the exact red-verify condition
and a ceiling, so it enters the summed step budget.
The job-budget rows now carry the setup allowance (33 for the MySQL job,
15 for failover) and assert ceiling - steps against it, so raising a step
ceiling re-derives the allowance instead of quietly spending it.
|
||
|
Shaojin Wen
|
fe30b5daaa |
test(managed-agent): pin lapsed-lease ledger writes and the IT's result-store wiring (#16018)
* test(managed-agent): pin lapsed-lease ledger writes and the IT's result-store wiring Design decision 5 says a scanner whose lease lapsed still records and settles until another claim moves the fence, but no test held that half: adding a lease_until condition to any ledger write left every case green. A new AutomationScannerTest case writes past lease_until with owner and fence unchanged, then lets another claim take the lease. WorkspaceCsiSqlWorkerMySqlIT now asserts its journal holds the result store it wired, so dropping that wiring can no longer leave the zero-projection assertion passing for the wrong reason. Decision 6 no longer says "lost its lease", which read as expiry. * test(managed-agent): cover every fenced ledger write after the lease lapses The lapsed-lease case now also releases, blocks, records a gate outcome and settles with a detail after lease_until, so a lease_until condition on any of the nine owner+fence writes fails it. The scanner's class Javadoc, the ledger's method Javadocs and the sibling test name said "lost its lease", which reads as expiry; they now name a takeover by another claim, as decision 6 does. |
||
|
Shaojin Wen
|
74bd584f0b |
fix(core): report a registry that kept changing as a PeerControllerError (#16007)
* fix(core): report a registry that kept changing as a PeerControllerError A controller registry mutation that loses its write to a concurrent writer three times in a row failed with a plain Error. The CLI treats anything that is not a PeerControllerError as an I/O fault, so `qwen sessions controllers add/remove` printed "could not write <path>: ..." for what is contention, not a permissions or disk problem. A caller branching on `error.code` saw undefined. RegistryChangedError now extends PeerControllerError with the code `registry-changed`, and its message names the file the way the sibling registry refusals do. The redo bound itself had no test: every rival-write case exercised a single redo. A new case lands a rival write inside each of five attempts and expects the third to give up with `registry-changed`, leaving only the rival's records on disk. * test(core,cli): pin the registry-changed message and the CLI error branches Nothing pinned the registry-changed message: dropping the file path, or rewording it as a disk fault, left the suite green. The case "gives up, saving nothing, when the registry keeps changing" now expects the exact message. That case's comment said a rival write lands inside each of five attempts, but only three attempts run. It now says what the rival budget is for: the rival stops after five writes, so a missing redo bound ends in a sixth attempt that succeeds instead of looping forever. The previous commit's message repeats the old description. The helper's parameter is renamed from reads to count. handleRemove's PeerControllerError branch had no CLI test, so forcing it either way left controllers.test.ts green. The add and remove blocks each get a case checking that a registry-changed error is printed as it stands, and "reports a failed write" checks that an I/O failure names the file. Two more comments are corrected. MAX_MUTATION_ATTEMPTS counts attempts in all, not redos. The CLI comment's list of self-explaining errors now covers refusals that are not about the label, such as too-many, and its other branch is said to cover a lock wait that ran out as well as an I/O error. |
||
|
Shaojin Wen
|
7bc00a3a08 |
feat(review): re-anchor on drift, run actionlint, account blockers, refuse retries (#16001)
* feat(review): re-anchor on drift, run actionlint, account blockers, refuse retries Four fixes from the 2026-10-31 audit of seven review run ledgers (#15057, #15377, #15385, #15578, #15853, #15955, #15974): - Head drift posts the completed review at the new head (the re-anchor protocol) instead of abandoning the round — the maintainer's ruling on #15578, never landed in the skill until now. The once-per-review bound and the COMMENT cap survive; the marker withholds its anchor. - script-lint runs actionlint's workflow-level checks (syntax, context availability, action refs) and gates them as [lint] Criticals; only the embedded run: shell stays deferred. #15974's unloadable workflow (runner context in jobs.env) is the regression test, measured against actionlint 1.7.12. - pr-context writes a blocker inventory; compose-review cross-checks it against the round's blockerRulings and caps at COMMENT when the walk skipped a blocker (#15974's missed review-level blocker). - submit documents --dry-run as the only sanctioned probe and refuses a re-post at the commit already landed (already-posted-this-commit) — the retry that replied this round's own roots back to themselves (#15853). The receipt records lastCommitId. Each fix carries its rule text, pin tests, and a DESIGN.md incident. * fix(review): isolate actionlint config and refresh lint on re-anchor Both Criticals from the #16001 review, reproduced against the exact PR source and closed structurally: - The newly active actionlint invocation auto-loaded the reviewed repo's .github/actionlint.yaml, so a PR-added ignore-all rule suppressed every workflow diagnostic — including the unloadable-workflow expression this linter was activated to catch. It now runs against a private mkdtemp neutral config via -config-file (measured to override the discovery), fails closed when no private config can be created, and carries exactly the two trusted compatibility exceptions the repo's own lint script runs (self-hosted runner labels, environment.deployment schema lag). The regression plants the ignore-all config in a real temporary git repository and proves the expression finding survives. - The re-anchor protocol dropped fixed authored findings but left the lint gate regenerating its findings from the old-head report, so a lint blocker the delta fixed was republished against the new commit. The protocol gains a refresh step (scratch worktree at the new head, script-lint --diff rewriting the report bound to the new diff) and a lintDiffPath compose field that makes the gate — body and ledger marker alike — hash the new diff as its freshness basis. Executable regressions pin both directions: fixed blocker gone (COMMENT, absent from body and marker), still-standing blocker keeps gating (REQUEST_CHANGES). A malformed lintDiffPath fails closed. * fix(review): base-tree actionlint config, empty-diff parse, lane provisioning Three findings from the round-3 review of #16001, each reproduced against the exact source and closed: - The private {} config blocked PR-planted suppressions but also dropped the repo's maintainer-merged compatibility exceptions (ECS runner labels, concurrency.queue, the $/ self-repository action reference), turning correct release-workflow changes into bogus [lint] Criticals. The trust boundary is now the MERGE-BASE tree's .github/actionlint.yaml (hex-shape-guarded git show, copied into the private file, {} only when the base has none); the head copy is never read, and the two trusted -ignore constants ride on top exactly as scripts/lint.js runs them. Pinned with a two-commit repo and a real-binary run showing the base exception honored while the planted ignore-all stays blocked. - An empty refresh diff (the new head reverted the PR's last change) was treated as a parse failure: the range selection fell back to the old plan's context-inclusive hunks and promoted a pre-existing finding into a blocker the gate accepted as fresh (the empty diff has a valid hash). diffParsed now separates parse success from range count; empty and deletions-only diffs classify everything pre-existing. The reviewer's exact scenario is an executable regression. - The lanes that run the gate provisioned no linters, so the new uninstalled-capping fired on CI and the real-binary regressions were skipIf'd out of every run. The review job and the unit-test lane now install the pinned, SHA-256-verified actionlint/shellcheck through the repo's own installer and promote the job-scoped dirs onto GITHUB_PATH (unconditional, advisory, Windows excluded). The 335 workflow meta-test pins pass unchanged. * fix(review): lint in the worktree cwd, guard lint.js entry, pin the sha fence Three gaps from the scripted deep verification of #16001 (50/55 assertions, 17 mutants), two of them in the round-3 fixes themselves: - F1: actionlint resolves the config's paths: entries against its cwd, and the runner spawned it in the session checkout, so every path-scoped base exception was inert while the global keys kept working — the split that hid behind the labels-only test. The runner now receives the worktree root and spawns inside it with PWD aligned. The real-binary suite gains the path-scoped fixture it was missing: a benign concurrency.queue edit that must lint clean, and an attack cell where the base exception suppresses the schema-lag finding while the planted head ignore-all still fails to hide the expression. - F2: the provisioning steps imported scripts/lint.js, whose unconditional top-level main() ran a full lint pass on import and never reached the install loop, silently under continue-on-error. lint.js now runs its CLI only when it is the entry point (import.meta.url guard, argv[1]-safe for node -e imports), and both steps assert each installer produced its binary before promoting the dir, so a future no-op install fails visibly instead of degrading to skipped tests. - M15: the option-injection fence was unobservable — git's own rejection of a malformed option matches the guard's refusal, so the guard's mutation survived. A spawn spy now pins the fence directly: non-hex merge base never spawns git; a hex-shaped nonexistent one does (positive control with the exact argv). Also acknowledges the verification's correction: the bot review's $/prepare-release-runner example does not exist at this head (the base config entry is precautionary); the queue exception at line 485 is the one reachable path-scoped case, and F1 is pinned on exactly that shape. |
||
|
Shaojin Wen
|
2a8ae9b2df |
docs(managed-agent): record the ManagedAgentStore setter investigation (#16014)
* docs(managed-agent): record the ManagedAgentStore setter investigation Complete the P1 inventory with the ManagedAgentStore finding: the seven public setters are Spring @Autowired(required=false) optional-dependency injection (four optional, two required, worktreeFiles manual), not ad-hoc mutable wiring. Converting them to constructor ObjectProvider injection is a large change to the central 7,700-line class with ~30 test wiring sites whose correctness depends on Spring runtime assembly the manual-setter unit tests do not fully exercise. Like the broker reconciliation cluster, it warrants a dedicated design pass and full integration verification before implementation rather than an incremental slice. * docs(managed-agent): count worktreeFiles among the required setter injections configureWorktreeFiles carries plain @Autowired and its bean is supplied by ManagedAgentConfiguration.managedWorktreeFiles(); the direct calls in VerificationFixtures are test setup, not production wiring. The setter inventory therefore reads four optional and three required, and the @Autowired(required = false) statement now applies only to the optional four. Both language versions move together. |
||
|
|
80273bd34a |
fix(core): extend built-in reasoning effort to DeepSeek-V4 and new Qwen prefixes (#15987)
* fix(core): extend built-in reasoning effort to DeepSeek-V4 and new Qwen prefixes Third-party gateways (PAI-EAS, internal proxies) host DeepSeek-V4 models under arbitrary hostnames, and the invitation-only qwen-latest-series previews and qwen4 family speak the same tiered reasoning_effort contract as qwen3.8-max. The hostname whitelist and exact-id tables left these routes without any effort controls. Recognize the deepseek-v4 prefix case-insensitively on any OpenAI-compatible endpoint (official endpoint keeps its own contract; non-V4 aliases stay host-gated), and route the qwen-latest-series-invite- and qwen4 prefixes through the shared tiered-effort wire gate and the stable qwen3.8-max control tiers. Explicit capability declarations still take precedence. Co-authored-by: Qwen-Coder <[email protected]> * fix(core): address review on effort prefix recognition - Normalize xhigh to high for every v4.1-flash gateway alias, matching the stable Flash id instead of falling into the legacy xhigh->max rule. - Carry the tiered Qwen manifest through opaque duplicate-endpoint routes: the route preview guard, the live ACP configuration guards, and the reasoning-configuration lookup now admit the new prefixes, with two-endpoint regression coverage. - Stand the built-in DeepSeek normalizer down on newly recognized third-party hosts whenever an explicit declaration governs (wire reshape, display state, and override clearing), preserving declared/static effort semantics; established DeepSeek routes keep their shipped interaction. - Update the duplicate-route DeepSeek assertion to the new contract and refresh the stale single-family scopings in docs and comments. Co-authored-by: Qwen-Coder <[email protected]> * fix(core): scope declaration stand-down to pre-PR routes, keep routed declarations - Limit the established-route compatibility exception to the exact model/host pairs the pre-PR recognizer accepted, so a declaration for a newly recognized alias keeps its wire semantics even on a MaaS host. - Make the stand-down self-determining: a profile-less resolved capability is a declaration, and both the effective-reasoning shaping and the DeepSeek state stand the built-in normalizer down on newly recognized combinations, covering pipeline injection, provider reshape, display, and override clearing alike. - Preserve an already-resolved declaration in the routed Qwen fallback; the tiered manifest applies only when nothing resolved. - Regressions: MaaS-alias static and configured effort, display/wire alignment, established-pair boundaries, and a live ACP duplicate-endpoint declaration case. Co-authored-by: Qwen-Coder <[email protected]> --------- Co-authored-by: Qwen-Coder <[email protected]> Co-authored-by: 易良 <[email protected]> |
||
|
Shaojin Wenand易良
|
6eef5d999e |
fix(cli): bind the PR badge when a --flag is glued to the number (#15989)
* fix(cli): bind the PR badge when a --flag is glued to the number * fix(cli): count a --repo qualifier glued to the PR number * fix(cli): narrow the glued-flag grammar to flag-shaped tails * fix(cli): anchor the glued qualifier and keep /review's strict closure * fix(cli): recognize --repo anywhere in the command's glued flag chain * fix(cli): anchor the URL fallback and share one glued-link grammar --------- Co-authored-by: 易良 <[email protected]> |
||
|
Shaojin Wen
|
94448d368d |
fix(verify-pr): keep the mutation matrix off the tree live cells measure on (#15999)
* fix(verify-pr): keep the mutation matrix off the tree live cells measure on A local round interleaved its mutation matrix with 80 live daemon cells on one tree. A build inside a mutation row propagated the mutant into dist/, and the hand-patched repair still carried the mutant expression, so every live cell and the whole matrix were withdrawn and the round shipped as inconclusive. Require a dedicated tree for the mutation matrix, gate every daemon or dist-level harness launch on a scripted dist-versus-source anchor check with a mismatch-reporting control, and forbid hand-patching dist to repair a contaminated build. Pin the new rule in the workflow meta-test. * test(verify-pr): pin the launch gate's coverage and control clauses Review on |
||
|
Shaojin Wen
|
f99062525d |
test(managed-agent): pin the private CSI receipt guards and fix design decision 5 (#16011)
#16000 documented that Sessions created through the private CSI lane record no tool receipts for projection. Pin the two places that hold: - WorkspaceCsiSqlWorkerMySqlIT now wires a ManagedToolResultStore into the journal, as production does, and asserts that no TOOL_RESULT_PROJECTION row exists after its receipt-carrying CSI commits. Without the store the capture guard was never reached. - ToolPublicationStoreTest marks the Session private (through tool_profile or runtime_request_key, since csi_guard is generated) before claiming, asserts the claim is empty and the row untouched, then clears the marker and claims the same row. Decision 5 of the automation runtime design said a scanner whose lease expired records and settles nothing. Occurrence writes check owner and fence, never lease_until, so they are refused only once another claim moves the fence. Both language versions now say so, matching the AutomationLedgerStore Javadoc corrected in #16000. |
||
|
Shaojin Wen
|
c726d2cfc1 |
docs(managed-agent): close the D1 prose finding; record the reconciliation prerequisite (#16012)
Carry forward the R3-1 review fix that was verified but not pushed before #15997 merged: the D1 structural-debt prose now matches the table (the observation cooldown and tool-hold inventory are named as extracted collaborators, not as concerns still mixed into the class), in both languages, and the service's hold-inventory javadoc defers the detailed discharge invariant to RuntimeHoldInventory instead of duplicating it. Also record the reconciliation-cluster investigation: reconciliation, recovery scanning and cancellation share the service's three concurrent maps, binding-lifecycle helpers and CAS settle, so none is a clean standalone seam. The design now names the prerequisite — a shared-state registry that owns the maps and lifecycle helpers — to be designed and reviewed on its own before those paths can be extracted, and before the M6 stamp-before-slot-drop ordering becomes a pinnable seam-level contract. |
||
|
Shaojin Wen
|
1515428391 |
fix(web-shell): proxy the PR watches route in dev (#16004)
* fix(web-shell): proxy the PR watches route in dev The dev server's proxy table had no entry for the autofix-pr routes, so `GET /autofix-pr/watches` fell through to the SPA fallback and answered with index.html. The PR watches page then failed JSON parsing on open and reported "Unexpected token '<'". Only the primary workspace's unqualified route was affected: the workspace-qualified form is already reached by the `/workspace` prefix entry. Production is unaffected because the daemon serves the page and the API from the same origin, with no dev proxy in between. The browser suites did not catch it because they stub the route at the network layer, which intercepts the request before it reaches the dev server. * test(web-shell): fold the PR watches proxy case into the coverage table Review feedback on #16004: the standalone block repeated the existing daemon-API coverage table's lookup, request shape and bypass assertion, and pinned the entry to the Goals route by reference identity. That pin was arbitrary — nothing requires the two routes to share one object — so re-objecting Goals with the spread idiom this file already uses elsewhere would have reddened the PR-watches case while the route itself stayed correct, pointing a future reader at the wrong entry. The case is now a row in the existing table, which keeps one enumerated list of the primary prefixes the dev proxy must serve. Each row carries the URL the daemon actually serves rather than a derived one. The load-bearing assertions are unchanged: deleting the proxy entry still reds exactly this row, and re-objecting Goals no longer affects it. |
||
|
Shaojin Wen
|
35daec35ea |
fix(verify-pr): keep the verdict line a plain paragraph and check the comment footer (#16002)
* fix(verify-pr): keep the verdict line a plain paragraph and check the comment footer Three verification rounds on PR #15984 shipped their opening verdict as a single Markdown heading: the round title and the whole ~950-character verdict paragraph shared one `#` line, which GitHub renders as one giant H1 — a wall of bold text instead of a readable lead. All three comments also went out without the attribution footer the publish contract requires. The skill only said "verdict line first" without pinning the Markdown shape, and the consistency checker only checked that the verdict word and head OID appear in the first ten lines, so both defects passed every gate. - The skill now pins the verdict line to a plain paragraph (a round title may lead it as plain text, never as a heading), with the measured #15984 example and a template. - Local rounds assemble the exact posted body — report content plus the resolved footer — into comment.md, post that file, and read the comment back against it byte for byte; report.md stays footer-free by contract. - The checker warns when the lead line is a heading carrying the verdict or an over-long heading (GFM indents of up to three spaces included), and when comment.md lacks a well-formed attribution footer. Warnings, not errors: a cosmetic defect must not make the CI publisher refuse an otherwise consistent verdict. - Pin tests reproduce the shipped shape from the real round-3 comment, and the workflow meta-test pins the new skill text. * fix(verify-pr): accept CRLF line endings in the comment-body checks Review of #16002 reproduced a Windows-authored comment.md failing the anchored footer regex: splitting only on \n left a stray \r on the footer line, so a valid attribution was reported missing and the local publish gate sent the artifact back for unnecessary repair. GitHub renders CRLF and LF identically. Normalize CRLF (and lone CR) to LF before the line-based checks on report.md and comment.md, and pin LF/CRLF parity: the same valid footer passes under both endings, a missing footer and a heading lead still warn under CRLF, and the reviewer's CLI probe is covered end to end. |
||
|
Shaojin Wen
|
d811674c55 |
fix(verify-pr): keep each verify round in its own comment (#15988)
* fix(verify-pr): keep each verify round in its own comment Verify jobs are serialized per PR, but their publishers queue on hosted runners, so a round's live status is often still "running" when the next round's verify job starts. The status step took that comment over and both publishers then wrote into it, so one round's report replaced the other's. A cancelled pending run's notice could likewise replace another round's live status. The status comment now carries a run marker. The status step reuses a live status only when it is this run's own (a re-run attempt), and the publisher writes only into that status, posting fresh otherwise. WEAK_BODY no longer decides anything and is removed. Also, from the #15966 follow-ups: evidence images are collected from the round the comment publishes instead of from every round, oldest first, and the pre-agent sweep comment no longer describes the head -1 ordering. * fix(verify-pr): let a re-run publisher find its own comment The publisher looked only for this run's live status, but the body it publishes carries neither the running marker nor the run marker. A publish job re-run after it had posted therefore found nothing and posted the report a second time. Every terminal body now keeps the run marker right after the leading verify marker, and the publisher matches its own run's comment whether it is still the live status or already the report. The status step still reuses only a live status. Tests now pin the status body's leading markers in order, use a re-run attempt number distinct from the run id, check the run marker in every terminal body, and check that the evidence skip count comes from the published round. * fix(verify-pr): never let a notice replace a published report A publish job re-run on its own downloads its own attempt's artifact, which the verify job never uploaded, so it always renders "results unavailable". Since the previous commit matched this run's comment whatever it held, that notice replaced the round's report. The lookup now also reports whether this run's comment carries findings, and a body without findings is posted beside it instead. A notice may still replace this run's live status or its earlier notice. Every body now opens with a header written once, before the branches: the verify marker, the run marker and, when the body carries findings, the substantive marker, ended by a blank line. The build-failure body's substantive marker moves up into it. Both lookups that choose a comment to write into read markers only from the header, so a marker quoted further down a body is text. Tests pin the notice/report gate in both directions, the header of every terminal body and that it is written once, header-only matching in both lookups, and that the status step does not reuse this run's report. * fix(verify-pr): read the substantive marker from the header for the snapshot The previous-round snapshot was the last lookup still matching its marker anywhere in a body, so a newer notice quoting the substantive marker below its header would have been snapshotted in place of the newest real report. It now reads the header like the other lookups. Build-failure reports posted before the header change carry the marker below their header and are skipped by the snapshot during the transition. Tests add that notice shape to the snapshot case, and render the reportless body to check it carries no substantive marker. |
||
|
Shaojin Wen
|
39fd1b45ba |
refactor(managed-agent): extract the runtime hold inventory (#15997)
* refactor(managed-agent): extract the runtime hold inventory Second P1 seam in RuntimeBrokerService: the durable hold inventory moves to a package-private RuntimeHoldInventory. The pagination over hold rows and the RELEASED filter (restore the close's drain obligation, never discharge — #13930 R1-2) move verbatim; the service keeps the admission, tenant resolution and error-mapping boundary, so the stage assembly and the runtime_hold_inventory_scope_failed mapping are unchanged. Six new unit tests cover the collaborator (empty ledger, released discharge, missing-record-is-not-release, non-released still holds, cursor pagination, dedup); the 199-test broker suite and the binding inventory suite stay green. Recovery scanning was investigated as the next seam and proved entangled with reconcileInContext, the SessionContext in-memory index and the CAS settle loop; the design records why it waits for the reconciliation collaborator rather than forcing a callback-heavy extraction. * docs(managed-agent): record the hold inventory in the D1 ledger; pin RELEASING Close the review's D1-table consistency finding: the structural-debt table's RuntimeBrokerService row now names both extracted seams (ExecutionObservationCooldown, RuntimeHoldInventory) and refreshes the line count to the current 6,329, in both languages, so the ledger matches the slice narrative instead of drifting per extraction. Also add the round-2 deferred probe as a test: a RELEASING session still holds (only RELEASED discharges the drain obligation). A widened discharge filter (!= READY) turns the new test red — verified by injection with byte-level restore. 224 focused tests and Checkstyle stay green. |
||
|
Shaojin Wen
|
43415ed8a4 |
docs(managed-agent): note the private CSI receipt exception and correct lease wording (#16000)
The artifacts.enabled row said committed tool receipts are always recorded for projection. Sessions created through the private CSI lane (csi-session) are the exception: ManagedSessionStore records receipts only when the commit has no CSI original, and the projection claim skips csi_guard Sessions as well. The automation.lease row now counts the lease from the claim of each definition rather than from a scanner tick, matching the scanner-threads row. The AutomationLedgerStore Javadoc no longer says an expired lease alone stops a writer: its writes check owner and fence, so they are refused only once another writer claims the definition. |