Skip to content

fix(*): make the machine form's connect cancellable and bounded, and reword 'No link' - #1043

Merged
0xKT merged 12 commits into
EverMind-AI:mainfrom
Handsome-wzw:fix/machine_connect_feedback
Oct 24, 2026
Merged

0xKT merged 12 commits into
EverMind-AI:mainfrom
Handsome-wzw:fix/machine_connect_feedback

Conversation

@Handsome-wzw

@Handsome-wzw Handsome-wzw commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Two of the three high-severity findings from the 2026-10-20 user-perspective walk of the Machines tab (the third, the narrow-viewport fold, is a separate PR):

  1. The machine form's connect could not be waited on or stopped. It showed one static line and one disabled button for as long as the server took, and for ever when the socket went quiet without a close event (seen once on 2026-10-20).
    • Server: the proof a subagents.add, subagents.toggle or subagents.update runs before it writes is one task held in _RUNNING under the entry's name, the map a test's run is held in (_run_once), so subagents.test_cancel reaches it. That task covers the login-shell capture, the one real prompt, and, for an acp row that answered and whose record wants it, the capability record (_record_after_the_answer), which is a second launch and handshake of up to two start windows. A cancel anywhere in that stretch makes the connect refuse with cancelled: true in its error data and write nothing; the refusal says whether the cancel came before the agent answered or while its capabilities were being recorded. One run per name at a time: a Test during a connect's proof or record is refused as a second test is, and a connect during a Test is refused before anything is spent. The row reports test_running meanwhile. A cancel of the call itself (its connection went) keeps unwinding as cancelled instead of composing a refusal for a socket that is gone.
    • Page: the form's line counts the seconds, the hint says what the wait is for and its limit, and Cancel asks the server to stop the run. The stop request is sent, not awaited (the transport gives a call no timeout of its own), and its answer is read alongside an 8 s grace. Only a stop that answers it stopped the run, or the connect's own refusal as cancelled, is said as "Cancelled; nothing was written". A stop that answers nothing was running means the connect is past the point where it can be cancelled: the form says it is too late to cancel, takes Cancel away and keeps waiting for the connect's own answer; still unanswered at the ceiling, it ends as "Not stopped", which says the entry may have been written. A stop with no answer within the grace, or a failed one, ends as "Cancel not confirmed", which says the same and is corrected if the stop's answer or the connect's cancelled refusal arrives later. After ten minutes with no answer (above the server's own worst case of 420 s for the slowest preset once the record is counted) the form asks the run to stop itself and points at the Agents page. A connect that lands after its form stopped waiting is told in a toast, takes down the note that said it was not written, and the roster is read again. A stop's late answer only corrects the note of the press it belongs to.
  2. Every card said No link when the tab opened, which read as a fault on healthy machines. The resting state now reads Not connected; the link opens when work is sent (the Chinese line says the same), and the docs say when a link is opened.

Contract summaries of subagents.add, subagents.update, subagents.toggle and subagents.test_cancel updated (no shape change; both generated clients unchanged); i18n keys added in en and zh; docs (en/zh), CHANGELOG and CONTEXT updated.

Type

  • Fix
  • Feature
  • Docs
  • CI / tooling
  • Refactor
  • Other

Verification

New cases: 8 in tests/test_rpc_subagents.py (among them a cancel during the capability record that answers cancelled: true and writes nothing, and a Test refused while a connect records the same name) and 14 in ui-web/src/features/extAgents/ExtAgentsPage.test.tsx (counting, Cancel, the ceiling, a stop that never answers on both paths, too late to cancel on Connect and Reconnect, not stopped, cancel not confirmed and its later corrections, and which press a late answer may correct). The two capability-record cases fail against the server code from before the record was moved into the run (assert 'OpenCode' in {}).

Run on the PR head:

  • uv run --frozen --python 3.12 --all-extras pytest -q tests/test_rpc_subagents.py: 171 passed.
  • npx vitest run src/features/extAgents/ExtAgentsPage.test.tsx src/features/extAgents/AgentsBody.test.tsx src/features/extAgents/store.test.ts (ui-web): 3 files, 220 passed.
  • npx tsc --noEmit -p tsconfig.json (ui-web): clean. npx eslint src/features/extAgents (ui-web): clean.
  • npm run lint:i18n and npm run lint:rpc (ui-tui): both in sync.
  • scripts/check_source_language.py and scripts/check_large_files.py on the PR's merge-base range, and git diff --check: all exit 0.
  • CI on the head: 21 passed, 2 skipped.

Real machine, on the first revision (isolated gateway, the branch's node installed on a second machine in its own directory, real Codex and Claude Code adapters): the tab opens with the new resting-state line on both machines; a Codex connect cancelled at 0.7 s refused with cancelled: true, the form said cancelled and offered Connect again, nothing was written; a second Codex connect left alone still answered the normal refusal at 2.8 s; Reconnect of a switched-off Claude Code entry cancelled at 1 s ended the agent process on the machine (rc -9, nothing left running) and left the entry off.

  • Relevant tests pass locally
  • Relevant lint / type checks pass locally
  • User-facing docs or screenshots are updated when needed

The docs box is left open: the machines page (en/zh) and CHANGELOG give the ten-minute ceiling, but the connect entry in CONTEXT.md still says the form stops the run after five minutes.

Risk

  • Security impact considered
  • Backward compatibility considered
  • Rollback path is clear for risky changes

Low. No new method, parameter or input; subagents.test_cancel already existed and only reaches more of the same run. The proof path behaves as before when nothing cancels it; the one new refusal (a connect while a Test of the same name runs, or the reverse) replaces a second real prompt that would have raced the first in _RUNNING. Holding the capability record inside the run means a Test pressed during a connect's record is refused until the record ends, where it used to start a second launch. The page's ceiling only fires after the server's own longest wait. The English state_off wording changed; nothing matches on it. No wire shape changed, so reverting the squash commit rolls it all back.

Related Issues

Follow-up to #1007 and #1019 (machine-side connect form). Findings 1 and 2 of the 2026-10-20 Machines tab walk; finding 3 (narrow viewport) is the next PR.

@Handsome-wzw
Handsome-wzw requested review from 0xKT and LivXue October 21, 2026 06:36

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: the five-minute ceiling can still hang while sending the cancellation request.

I found one blocking issue, marked inline. I covered the repository rules and CONTEXT vocabulary, the complete diff, the runtime and web callers (including the gateway/transport), relevant history, compatibility of the existing RPC shape, whether tests were weakened, and the source/store architecture boundary.

Verification: PYTHONUTF8=1 uv run --frozen --python 3.12 pytest tests/test_rpc_subagents.py -x passed (168 tests); the three affected web test files passed (209 tests); web type-check, lint, and RPC generation checks passed (lint reported four pre-existing warnings in untouched files); source-language and large-file gates passed; git diff --check passed.

Comment thread ui-web/src/features/extAgents/store.ts Outdated

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

The new commit fixes the previously reported boundedness defect: the grace timer is armed before the stop RPC is sent, and the regression test proves both the ceiling and manual-cancel paths return even when both calls remain unresolved. I rechecked the full resulting diff and the new delta against AGENTS.md and CONTEXT vocabulary, callers and transport behavior, relevant history, backward compatibility, test integrity, and the documented source/store architecture; I found no new issue worth raising.

Verification: the three affected web test files passed (210 tests); web type-check, lint, and generated-contract checks passed. Lint reported only four pre-existing warnings in untouched files, and git diff --check passed.

@LivXue

LivXue commented Oct 21, 2026

Copy link
Copy Markdown
Member

No blockers in the reviewed scope.

Scope: I reviewed only the raven/agent/** portion of this PR -- 0 of 16 changed
files. The remaining 16 files were read as context but are not covered by this
verdict, and no finding here can carry an inline note inside the review zone.

This is the PR's second head. f14d0f369f0c adds one commit over ca8abe4550a9:
it sends the machine form's stop request inside its bounded wait instead of
awaiting it first (ui-web/src/features/extAgents/store.ts:632-650), and adds a
web case that holds the connect and the stop open together. The one finding left
is Nonblocking, and it is not cosmetic: inside the window it describes, a second
press spends a second real prompt before the server refuses the duplicate.

The blocking finding on the previous head was gloryfromca's: awaitRun awaited
cancelRun before the grace timer existed, so a socket that went silent without
closing hung the wait a second time. f14d0f369f0c sends that call without
awaiting it and starts the grace from the send; his thread is resolved and his
review at this head clears it. It is not re-filed here. The finding below is the
remaining side of the same call -- the stop is now sent, but its answer is still
not read.

Confirmed findings:

  • Nonblocking -- the form says the connect was cancelled while the write is
    still going to land, and leaves that sentence standing after it lands. When
    the stop request finds no run to stop, cancelRun answers cancelled: false
    and awaitRun (store.ts:647) discards that answer, so after the 8 s grace
    it returns stopped: 'cancelled' anyway. The form then shows "Cancelled;
    nothing was written. It can be connected again."
    (MachineCards.tsx:547-552, i18n/messages.json:5351), drops
    mconnectPending, and offers the preset again; when the write lands,
    landedLate (store.ts:669-675) toasts "connected" while that sentence stays
    on the form. The window is the capability record that follows a first add's
    proof: the proof ends and takes the name out of _RUNNING, then
    _refuse_unless_it_answers runs record_capabilities before the write
    (raven/rpc/methods/subagents.py:1162-1174), so subagents.test_cancel
    (:1554) has nothing to cancel and answers cancelled: false. A second press
    inside that window spends a second real prompt and is then refused by
    set_agents with duplicate sub-agent name(s). The ceiling's path has the
    same shape. The comment in connectOnMachine's 'stopped' branch
    (store.ts:709-714, "a second press while the first run still goes is refused
    server-side, before anything is spent") does not hold inside that window.

Verification:

  • Reproduced at ca8abe4550a9. Two pytest probes on that head's archive, with a
    ping_agent that answers ok, capabilities_wanted true, and
    record_capabilities held open, printed _RUNNING while the add records capabilities: [] and subagents.test_cancel then -> {'cancelled': False}, and
    a second concurrent gate call issued a second real run
    (['OpenCode', 'OpenCode']). One vitest probe with cancelRun answering false
    and a 20 ms grace printed mconnectStopped: "cancelled", pending: [] and the
    preset offered again, then toasts: ["gui.machine.connected_said ..."] with
    the note still saying cancelled.
  • Re-run at f14d0f369f0c, in that head's archive tree: all three probes print
    the same output. The pytest probes imported
    /tmp/pr1043_head/raven/rpc/methods/subagents.py, and a control case asserts
    that import path, so the pass is this head's file.
  • Code-reading only, not measured: how long the capability record takes on a
    live machine, and the claim that a live ping_agent reaps its subprocess when
    it is cancelled (raven/agent/subagent/probe.py, unchanged here). The Python
    probes stub ping_agent; they prove the cancel does not reach that call, not
    what the real call does on its way out.
  • Ring 3 and ring 4 are untouched by this PR (prctl.py zone --rest,
    prctl.py critical): no sensitive-zone or critical-zone change, and no
    sign-off is owed.

Checked and deliberately not reported: the wider reading of test_running, whose
readers in ui-web and the TUI draw it as "Testing" and let Esc cancel it (the
PR's stated intent); the state_off reword, which stateLine draws only for the
quiet "off" status; the two test files whose comments still say the machine
sheet uses "two source verbs" when it now uses three; and the ambiguity in
subagents.update's summary about which name test_cancel takes when the call
also renames the entry.

@LivXue

LivXue commented Oct 21, 2026

Copy link
Copy Markdown
Member

Windows compatibility review (head f14d0f369f0c8a821a6bac90f62ca082bd417b88, base d9c0a1d74222825ad621a0fd138b691c0edd537d).

Scope: Inspected all 16 changed files (CHANGELOG.md, CONTEXT.md, docs-site/docs/machines.md, docs-site/docs/machines.zh.md, i18n/messages.json, raven/rpc/methods/subagents.py, rpc-schema/openrpc.json, tests/test_rpc_subagents.py, ui-tui/src/i18n/messages.generated.ts, and 7 ui-web/src/features/extAgents/* files) and verified exact-head source extraction and reverse patch applicability on native Windows (verify-source.ps1).

Findings

  • No Windows compatibility issue was found in the inspected scope. _run_once in raven/rpc/methods/subagents.py tracks the connect proof task in the existing in-memory _RUNNING map using asyncio.ensure_future and current_task().cancelling(), and the frontend changes (ui-web/src/features/extAgents/store.ts, MachineCards.tsx, source.ts) use standard JS timers and promises without platform-specific assumptions. (The non-blocking cross-platform race between _run_once completion and record_capabilities before set_agents is already covered in the earlier review comment #issuecomment-6311098343.)

Verification and limitations

  • Native Windows 11 / Python 3.13.12 (uv run pytest ... -n 0 -x): 27 passed, 0 failed across the new connect-cancel, test_running, and machine-connect cases in tests/test_rpc_subagents.py.
  • Frontend vitest suites and live remote-machine connects were not executed in this scheduled Python check.
Exact verification commands
# suite: 27 passed, 0 failed, 0 errors, 0 skipped; exit 0
# Working directory: C:\Users\livxu\AppData\Local\Temp\raven-windows-review\045a318bc8c44406a153d58b0fe1b6be\pr1043\source\head
uv run pytest tests/test_rpc_subagents.py -k 'cancel or proving or proves or test_running or machine' -n 0 -x --tb=short --basetemp 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\045a318bc8c44406a153d58b0fe1b6be\checks\1043-suite\pytest' --junitxml 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\045a318bc8c44406a153d58b0fe1b6be\checks\1043-suite\junit.xml'

Comment thread ui-web/src/features/extAgents/store.ts
@0xKT

0xKT commented Oct 21, 2026

Copy link
Copy Markdown
Member

Not a blocker (review of f14d0f36, merged onto e1c44e57): two smaller points from the same pass, worth folding into the fix for the thread above.

Severity of this one finding, not a verdict on the pull request. Any blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text.

  1. The five-minute ceiling is not the server's worst case.
    • The comment above CONNECT_CEILING_MS (ui-web/src/features/extAgents/store.ts:183-191) says five minutes is "the server's own worst case for the add -- the node launch, the preset's start window and the answer, then the capability record -- so a connect that is merely slow is never cut short".
    • For a preset with a 120 s readyTimeoutMs, the proof alone may take the start window plus the 60 s answer (raven/agent/subagent/probe.py:71). The record then bounds initialize and session/new by 120 s each, plus a 10 s close (raven/acp_client/capabilities.py:59, :958, :1035, :1080). That passes five minutes before the node launch is counted.
    • A slow but healthy connect can be stopped by the form.
  2. The run's lifecycle is now written twice.
    • _run_once and subagents_test each check for a live run, register the task in _RUNNING and pop it identity-checked (raven/rpc/methods/subagents.py:1223/:1233/:1246 and :1486/:1505/:1517).
    • If the fix moves the capability record under the run, one helper that owns that sequence would keep the two from drifting.

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: resolve the open post-ping cancellation race before this merges.

The newly opened review thread changes my standing stance from clean to blocker. It reports a measured path where _RUNNING is cleared after the proof ping, capability recording continues outside the cancellable run, subagents.test_cancel returns false, and the UI nevertheless announces that nothing was written before the add lands. I am not re-grading another reviewer's blocker; while that thread remains unresolved, my earlier clean verdict no longer reflects the whole record.

Verification on the unchanged head: tests/test_rpc_subagents.py passed (168 tests), and the three affected web test files passed (210 tests). Those green suites do not exercise the newly reported post-ping capability-record window.

Comment thread ui-web/src/features/extAgents/store.ts Outdated
@LivXue
LivXue force-pushed the fix/machine_connect_feedback branch from f14d0f3 to cb1be2c Compare October 23, 2026 06:41

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: the two open post-ping cancellation threads still need their authors or a maintainer to settle them on this head.

The code on cb1be2c8be38 addresses the reported path: capability recording now remains inside _RUNNING, so cancellation reaches the entire pre-write run, and the web form reads cancelled: false, keeps waiting, and guards late corrections by form press. I found no additional defect in this revision. I cannot downgrade or resolve blockers opened by other reviewers, and both threads remain open, so the whole record is still blocking until their owners or a maintainer clear them.

I reviewed the full main-to-head diff and the five new fix/test/docs commits against AGENTS.md and the Runtime/Web CONTEXT vocabulary, callers and transport behavior, relevant history, backward compatibility, test integrity, and the source/store architecture. Verification passed: tests/test_rpc_subagents.py (171 tests), the three affected web test files (217 tests), web type-check, lint, generated-contract check, source-language gate, large-file gate, and git diff --check. Lint reported only four pre-existing warnings in untouched files.

@LivXue
LivXue requested review from 0xKT and gloryfromca October 23, 2026 07:54
@LivXue
LivXue force-pushed the fix/machine_connect_feedback branch from cb1be2c to 4dab418 Compare October 23, 2026 08:14

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: the two existing post-ping cancellation threads still need their authors or a maintainer to settle them on this head.

I reviewed the new 4dab41806aea revision against github/main, including the project rules and domain context, the full diff, the delta from the previously reviewed implementation, affected callers, history, backward compatibility, and whether tests were weakened. I found no new code defect: capability recording remains inside the cancellable server run; a false stop answer keeps waiting; a missing or failed stop answer now reports an unconfirmed cancel rather than claiming nothing was written; and a later true stop answer or the connect's own cancelled refusal corrects that note.

The two existing blocker threads from @0xKT and @LivXue remain open. Their original concrete failure appears addressed by this revision, but those findings are theirs to close or downgrade, so the whole-record stance remains blocking until they or a maintainer settle them.

Verification on this exact head: uv run --frozen --python 3.12 pytest tests/test_rpc_subagents.py -x (171 passed); the three affected web test files (220 passed); type-check; generated-contract check; source-language check; large-file check; and git diff --check all passed. Lint completed with zero errors and four pre-existing warnings in untouched files.

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: the two existing post-ping cancellation threads still need their authors or a maintainer to settle them on this head.

I reviewed 2d8d7c81f278, including the project rules and domain context, the full PR diff, the two-commit delta from the prior reviewed head, affected callers and timing bounds, history, backward compatibility, and whether tests were weakened. I found no new defect. The ten-minute ceiling exceeds the explicit 420-second proof-plus-capability request budget, and the server now accurately distinguishes cancellation during capability recording from cancellation before the proof answer.

The existing blocker threads from @0xKT and @LivXue remain open without a reply or settlement. This revision still appears to address their reported failure, but those findings belong to their authors or a maintainer to close or downgrade.

Verification on this exact head: the RPC suite passed 171 tests and the three affected web files passed 220 tests. Type-check, generated-contract, source-language, large-file, and diff checks passed. Lint completed with zero errors and the same four warnings in untouched files.

@LivXue

LivXue commented Oct 23, 2026

Copy link
Copy Markdown
Member

Windows compatibility re-review at head 2d8d7c81f2789741354b8a6c92c6ac6a5fb70db0, base/merge base 8a7ea1ccf6da6114b5f8670e18228e9f3a23e940.

Inspected the complete 16-file PR diff and the revision changes during this run: RPC add/toggle/update and cancellation callers, probe/ACP/CLI cleanup paths, machine form state/source/components, contracts, translations, docs, tests, and discussions. No newly introduced Windows compatibility defect was found in this scope.

Native Windows verification (Python 3.13.12, Node 25.9.0):

  • All 8 added RPC regression cases passed, including cancellation during capability recording, no config write after cancellation, concurrent-run refusal, caller cancellation, and the wire error frame.
  • 220 Web cases passed, including false/missing/failed stop replies and late corrections. TypeScript and generated-contract consistency passed (248 methods).
  • 1 real-process integration case passed: a native .cmd launcher, interpreter, and worker completed ACP ping/verification and released their temporary working directories. This checks successful handshake cleanup; live vendor/SSH cancellation was not exercised.
  • All 4,664 checkout files and Git modes/tree matched this exact head; all 16 changed files were rechecked and git diff --check passed.

These results add Windows evidence to the existing post-ping cancellation discussion. They are a scoped compatibility report, not a general merge verdict.

Limits: UTF-8 mode was enabled. The full RPC suite was not completed: earlier broad runs at the superseded head hit two Unix-oriented fixture assumptions (an extensionless shell shim and /opt/node20/bin), each also failing on that revision's exact base. The current head was rerun with the focused cases above. No live remote machine/provider, interactive TUI, Python 3.12, or full repository suite was tested.

Exact verification commands and results

Each Python invocation used an isolated uv environment and temporary profile/Raven/OAuth/temp directories. UTF-8 mode was enabled and imports were asserted to belong to the selected checkout.

$run = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\e1c5fcc69a324287bf90beac9314f3a4'
$env:UV_PROJECT_ENVIRONMENT = Join-Path $run 'python-env'
$env:UV_PYTHON = '3.13'
$env:PYTHONUTF8 = '1'

# 1043-current-changed: 8 passed, 0 failed, 0 skipped; exit 0
Set-Location -LiteralPath 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\e1c5fcc69a324287bf90beac9314f3a4\pr1043\head-current'
$env:PYTHONPATH = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\e1c5fcc69a324287bf90beac9314f3a4\pr1043\head-current' + ';' + (Join-Path $run 'probes')
uv 'run' '--frozen' '--no-sync' 'pytest' 'tests/test_rpc_subagents.py::test_cancel_stops_the_run_that_proves_an_add_and_the_add_writes_nothing' 'tests/test_rpc_subagents.py::test_cancel_reaches_the_capability_record_after_the_answer_and_the_add_writes_nothing' 'tests/test_rpc_subagents.py::test_a_test_is_refused_while_a_connect_records_the_capabilities_of_the_same_name' 'tests/test_rpc_subagents.py::test_cancel_stops_the_run_that_proves_a_switch_on_and_the_roster_says_it_is_running' 'tests/test_rpc_subagents.py::test_a_test_is_refused_while_a_connect_proves_the_same_name_and_the_connect_lands' 'tests/test_rpc_subagents.py::test_a_connect_is_refused_while_a_test_of_the_same_name_runs_and_asks_the_agent_nothing' 'tests/test_rpc_subagents.py::test_a_connect_whose_own_call_is_cancelled_ends_its_run_and_writes_nothing' 'tests/test_rpc_subagents.py::test_a_cancelled_connect_refuses_on_the_wire_with_cancelled_in_its_data' '-p' 'no:randomly' '-n' '0' '-x' '--tb=short' '--basetemp' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\e1c5fcc69a324287bf90beac9314f3a4\checks\1043-current-changed\pytest' '--junitxml' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\e1c5fcc69a324287bf90beac9314f3a4\checks\1043-current-changed\junit.xml'

# 1043-current-windows: 1 passed, 0 failed, 0 skipped; exit 0
Set-Location -LiteralPath 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\e1c5fcc69a324287bf90beac9314f3a4\pr1043\head-current'
$env:PYTHONPATH = 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\e1c5fcc69a324287bf90beac9314f3a4\pr1043\head-current' + ';' + (Join-Path $run 'probes')
uv 'run' '--frozen' '--no-sync' 'pytest' 'tests/integration/test_subagent_connect_windows_real_process.py' '-m' 'integration' '-p' 'no:randomly' '-n' '0' '-x' '--tb=short' '--basetemp' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\e1c5fcc69a324287bf90beac9314f3a4\checks\1043-current-windows\pytest' '--junitxml' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\e1c5fcc69a324287bf90beac9314f3a4\checks\1043-current-windows\junit.xml'
$bare = Join-Path $run 'objects.git'
git --git-dir=$bare diff --check 8a7ea1ccf6da6114b5f8670e18228e9f3a23e940..a773b5a124652b53d99d085d35a82a4ecef3e5f8 # exit 0
Web verification commands
# 1043-web-tests: exit 0
Set-Location -LiteralPath 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\e1c5fcc69a324287bf90beac9314f3a4\pr1043\head-current/ui-web'
npm.cmd 'test' '--' 'src/features/extAgents/AgentsBody.test.tsx' 'src/features/extAgents/ExtAgentsPage.test.tsx' 'src/features/extAgents/store.test.ts' '--no-file-parallelism'
# 1043-web-types: exit 0
Set-Location -LiteralPath 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\e1c5fcc69a324287bf90beac9314f3a4\pr1043\head-current/ui-web'
npm.cmd 'run' 'type-check'
# 1043-web-generated: exit 0
Set-Location -LiteralPath 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\e1c5fcc69a324287bf90beac9314f3a4\pr1043\head-current/ui-web'
npm.cmd 'run' 'gen:check'

Dependencies reused this trigger's isolated installation after byte-identical package.json/package-lock.json checks; source imports and commands used the current checkout.

Handsome-wzw and others added 11 commits October 23, 2026 09:42
…reword 'No link'

The machine form's connect showed one static line and one disabled button
for as long as the server took -- legitimately the node launch plus a 120 s
start window plus 60 s for the answer -- and for ever when the socket went
quiet without a close event (seen once on 2026-10-20). And every card said
"No link" when the tab opened, which read as a fault on healthy machines.

Server: the proof prompt a subagents.add, subagents.toggle or
subagents.update sends is held in _RUNNING under the entry's name, the map
a test's run is held in (_run_once), so subagents.test_cancel reaches it;
the connect then refuses with cancelled: true in its error data and writes
nothing. One run per name at a time: a Test during a connect's proof is
refused as a second test is, a connect during a Test is refused before
anything is spent, and the row reports test_running meanwhile. A cancel of
the call itself keeps unwinding as cancelled.

Page: the form's line counts the seconds, the hint says what the wait is
for and its limit, Cancel stops the run server-side, a cancelled run is
said as a cancel rather than as the agent failing and is not kept on the
row, and after five minutes with no answer the form stops the run itself
and names the Agents page; a landing that still arrives is toasted. The
resting state reads "Not connected; the link opens when work is sent".

Contract summaries of the four subagents.* methods updated (no shape
change); i18n keys added; docs en/zh, CHANGELOG and CONTEXT updated.
…wait

The wait on a connect's write ends on Cancel or at the ceiling, and then
stops the run server-side. That stop request was awaited before the grace
timer was even created, and the transport gives a call no timeout of its
own -- so on the socket that went silent without closing, the case the
ceiling exists for, the wait hung on the stop request exactly as it had
hung on the connect, and the form stayed busy for ever a second time.

The stop is sent, not awaited; the grace runs from that moment, whatever
becomes of the call. A case holds a connect and a stop that never answer,
on the ceiling's path and on the Cancel press alike.
An acp row that answered the gate's prompt is measured before the add,
switch or update writes, and that record is a second launch and handshake
of up to the preset's start window. It ran after _run_once's finally had
taken the name out of _RUNNING, so a subagents.test_cancel pressed then
answered cancelled: false and the caller went on to write the entry the
form had just said was not written.

The record now runs inside the same held task as the prompt: a cancel
during it ends the handshake and the gate refuses with cancelled: true,
and a Test pressed meanwhile is refused as during the prompt. A record
that fails is still logged and the connect still stands on the ping.
…lled

The machine form's wait sent its stop request and dropped the answer, so a
Cancel that found nothing running still ended, after the grace, in
"Cancelled; nothing was written" with Connect enabled, while the connect
went on to write the entry.

The stop is still sent without being awaited, and its answer is now raced
against the grace. When it says nothing was running, the connect is past the
point where it can be cancelled: the form says it is too late to cancel,
drops the Cancel it cannot honour, and keeps waiting for the connect's own
answer up to the ceiling (one more grace when the ceiling sent the stop).
Unanswered by then, the wait ends as "Not stopped", which says the entry may
have been written. A false answer that arrives after the grace turns a
cancel or timeout note into that one, and a connect that lands after the
wait ended takes the note down besides the toast.

New i18n keys in en and zh; the TUI catalogue is regenerated.
…lable

The changelog entry, the machines page in en and zh, and the CONTEXT entry
now say that a connect's capability record runs inside the run a Cancel
reaches, and that a Cancel which finds nothing left to stop is said as too
late to cancel while the form waits for the connect to finish.
A stop's answer can arrive after its wait has ended. Two cases pin what it
may still change: an earlier press's late answer leaves a later press's note
alone while that press's own answer still corrects it, and a false answer
that arrives after the connect landed does not put the too-late note back.
…e guard

subagents.test_cancel's docstring now names the capability record as part
of the run it reaches, as its contract summary does, and the page's
cancelRun says what false leaves the connect in. cancelConnect no longer
checks the too-late state: the form draws no Cancel then, so nothing calls it.
… the run

The machine form's wait read the stop's answer for a false, but a Cancel
whose grace ended with the stop still unanswered, or with the call failing,
was still said as "Cancelled; nothing was written". Only a stop that
answered it stopped the run knows that, so such a Cancel now ends as
"Cancel not confirmed", which says the entry may still be written and where
to look. A stop that answers after the grace corrects the note: true turns
it into the cancel, false into the too-late note as before.

The timeout note no longer says the run was stopped, only that the form
asked it to stop. New i18n keys in en and zh; the TUI catalogue is
regenerated.
…itten

The changelog entry and the machines page in en and zh now say that a
Cancel whose stop gives no answer is said as not confirmed, and that the
five-minute ceiling asks the run to stop rather than saying it stopped it.
A Cancel whose stop failed ended as "Cancel not confirmed", and a refusal
from the connect itself that said it was cancelled, arriving after the
grace, left that note standing although it is the server saying nothing was
written. It now turns the note into the cancel, as a late true answer from
the stop does.

The hint shown while the connect runs no longer promises that Cancel writes
nothing: it says Cancel asks the run to stop and the form says whether
anything was written. The TUI catalogue is regenerated.
The machine form stopped a connect after five minutes and said that was the
server's own worst case, but the capability record an acp preset is
measured with after it answers adds two requests of up to its start window
each: 120s + 60s for the ping and 240s for the record is 420s for the
slowest preset, before the link's own calls. A connect that was merely slow
could be stopped by the form itself. The ceiling is now ten minutes.

The hint, the timeout note, the changelog, the machines page in en and zh,
and the add, update and toggle contract summaries and docstrings now count
the record's time. The TUI catalogue is regenerated.
…swer

A cancel that reached a connect while its capabilities were being recorded
refused with "was cancelled before it answered", although the agent had
already answered. The run now notes when the answer came, and the refusal
says "after it answered, while its capabilities were being recorded" then.
The case that pins the record's cancel now checks the whole sentence.
@LivXue
LivXue force-pushed the fix/machine_connect_feedback branch from 2d8d7c8 to 54bbdf9 Compare October 23, 2026 09:45

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: the two existing post-ping cancellation threads still need their authors or a maintainer to settle them on this rebased head.

I reviewed 54bbdf916cf2 against the freshly fetched github/main. Range-diff shows all 12 PR commits are patch-identical to the previously reviewed revision; the only revision change is rebasing onto the newer target. I also rechecked the project rules and domain context, full PR diff, callers, history, backward compatibility, and test changes. No new defect or integration regression was found.

The existing blocker threads from @0xKT and @LivXue remain open without a reply or settlement. The code still appears to address their reported failure, but those findings belong to their authors or a maintainer to close or downgrade.

Verification on this exact head: the RPC suite passed 171 tests and the three affected web files passed 220 tests. Type-check, generated-contract, source-language, large-file, and diff checks passed. Lint completed with zero errors and the same four warnings in untouched files.

@LivXue

LivXue commented Oct 23, 2026

Copy link
Copy Markdown
Member

Windows compatibility re-review at head 54bbdf916cf2f1a7ec7c2539e69d8bce7dd8d687, base/merge base a7e43a07729395311fcfc16dfae2e2bc467d8ada.

No new Windows compatibility issue was found in the inspected scope. Covered the complete 16-file PR diff, RPC add/update/toggle and cancellation callers, probe/ACP process cleanup, machine-form store/source/components, tests, contracts, translations, and docs. The complete diff is byte-identical to the previously inspected revision; I checked the intervening base changes and reran verification at this exact head. The post-ping cancellation discussion is resolved; the capability record remains inside the cancellable run, and the UI distinguishes false or missing stop replies.

Native Windows results (Python 3.13.12 with UTF-8 enabled; Node 25.9.0): 8 RPC regression cases, 1 real-process ACP cleanup case, and 220 Web cases passed. TypeScript and generated-contract checks passed. All 4,664 checkout files matched the head tree; all 16 changed files were reverified and git diff --check passed.

Limits: focused RPC cases only; the existing integration case exercises successful local .cmd launcher/interpreter/worker cleanup, not a live vendor or remote SSH cancellation. No full repository suite, interactive TUI, or Python 3.12 run. This is a scoped compatibility review, not a merge verdict.

Exact verification commands (all exit 0)

The commands below ran in an isolated exact-head checkout, with temporary profile/Raven/OAuth/temp directories and imports checked against that checkout.

$run = 'C:/Users/livxu/AppData/Local/Temp/raven-windows-review/927b085ec728402f877567018999273f'
Set-Location -LiteralPath (Join-Path $run 'pr1043/head-tree')
# 8 passed
uv 'run' '--frozen' '--no-sync' 'pytest' 'tests/test_rpc_subagents.py::test_cancel_stops_the_run_that_proves_an_add_and_the_add_writes_nothing' 'tests/test_rpc_subagents.py::test_cancel_reaches_the_capability_record_after_the_answer_and_the_add_writes_nothing' 'tests/test_rpc_subagents.py::test_a_test_is_refused_while_a_connect_records_the_capabilities_of_the_same_name' 'tests/test_rpc_subagents.py::test_cancel_stops_the_run_that_proves_a_switch_on_and_the_roster_says_it_is_running' 'tests/test_rpc_subagents.py::test_a_test_is_refused_while_a_connect_proves_the_same_name_and_the_connect_lands' 'tests/test_rpc_subagents.py::test_a_connect_is_refused_while_a_test_of_the_same_name_runs_and_asks_the_agent_nothing' 'tests/test_rpc_subagents.py::test_a_connect_whose_own_call_is_cancelled_ends_its_run_and_writes_nothing' 'tests/test_rpc_subagents.py::test_a_cancelled_connect_refuses_on_the_wire_with_cancelled_in_its_data' '-p' 'no:randomly' '-n' '0' '-x' '--tb=short' '--basetemp' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\927b085ec728402f877567018999273f\checks\1043-changed\pytest' '--junitxml' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\927b085ec728402f877567018999273f\checks\1043-changed\junit.xml'
# 1 passed
uv 'run' '--frozen' '--no-sync' 'pytest' 'tests/integration/test_subagent_connect_windows_real_process.py' '-m' 'integration' '-p' 'no:randomly' '-n' '0' '-x' '--tb=short' '--basetemp' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\927b085ec728402f877567018999273f\checks\1043-windows\pytest' '--junitxml' 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\927b085ec728402f877567018999273f\checks\1043-windows\junit.xml'
Set-Location -LiteralPath (Join-Path $run 'pr1043/head-tree/ui-web')
npm.cmd 'test' '--' 'src/features/extAgents/AgentsBody.test.tsx' 'src/features/extAgents/ExtAgentsPage.test.tsx' 'src/features/extAgents/store.test.ts' '--no-file-parallelism'
npm.cmd 'run' 'type-check'
npm.cmd 'run' 'gen:check'
$bare = Join-Path $run 'objects.git'
git --git-dir=$bare diff --check a7e43a07729395311fcfc16dfae2e2bc467d8ada 38c083b7f655985e5eac4b8f71abefe945e94e9e

@gloryfromca gloryfromca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blockers; this can merge as far as I am concerned.

The whole-record blocker is settled. @LivXue verified the current head against the original post-ping cancellation scenario, documented that both the server run coverage and the client's stop-answer handling now hold, and resolved both outstanding threads on the maintainer's word. My own earlier cancellation-RPC thread is also resolved, so no open objection remains from my side.

Fresh verification on the unchanged 54bbdf916cf2 head: tests/test_rpc_subagents.py passed 171 tests, and the three affected extAgents web files passed 220 tests.

@0xKT

0xKT commented Oct 23, 2026

Copy link
Copy Markdown
Member

Not a blocker -- notes from round 2 on 54bbdf9 (merged onto main 2cbacba; the round-1 thread is closed as fixed).

Severity of this one finding, not a verdict on the pull request. Any blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text.

  1. CONTEXT.md still says five minutes. CONTEXT.md:432-433 (added by this PR) says the connect form "stops the run itself after five minutes with no answer"; store.ts:200 is CONNECT_CEILING_MS = 10 * 60_000, and the i18n strings, both machines docs and the CHANGELOG say ten. 2c8500c raised the value and missed this one copy. The number is now written out in seven places beside the constant; the Runtime CONTEXT.md is also an odd home for a page-side timing (CONTEXT-MAP.md routes Web UI terms to ui-web/CONTEXT.md).

  2. A no-answer ending with the drawer closed is toasted as a refusal. With the form open, timeout / unconfirmed / missed each get their own line ("it may still be written"). With the drawer closed, connectOnMachine / reconnectOnMachine return {landed: false, current: false} for those same endings (store.ts:794-800, :859-866), and MachineCards.tsx:457-459 toasts "Not connected: {name}" -- a firm claim about an outcome the page does not know; if the add then lands, a "connected" toast follows. Narrow (drawer closed and a socket silent for the whole ceiling), but it is the same class of sentence round 1 was about, so a "not confirmed" line there would match the open-form wording.

Smaller: the ten-minute ceiling and the seconds counter have no test that fails if they change; how the source reads the server's two cancel answers is untested (two mutants survive every ui-web test); docs and CHANGELOG say only the stop's answer can confirm a cancel, though since c6bcf41 the connect's own cancelled refusal does too; the post-refusal model-menu read on the connect path runs outside _run_once.

Checked and fine: the server's worst case for an add-able machine preset is 420 s (ready 120 + answer 60 + 2 x 120 for the record), under the page's 600 s; _RUNNING is always cleared, including on exceptions and cancellation; tests/test_rpc_subagents.py + schema match: 683 passed.

@0xKT
0xKT merged commit bd0479f into EverMind-AI:main Oct 24, 2026
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants