Skip to content

fix(mcp): preserve terminal action uncertainty on disconnect - #165

Merged
link1345 merged 1 commit into
mainfrom
fix/153-terminal-disconnect
Oct 2, 2026
Merged

link1345 merged 1 commit into
mainfrom
fix/153-terminal-disconnect

Conversation

@link1345

@link1345 link1345 commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #153.

An accepted terminal click could lose its completion when the runtime shut down. MCP then returned only a generic closed/failed connection error, dropping the host request ID and whether the action had already been sent.

Semantic action transport failures now report not_sent or completion_unconfirmed, the request stage, send evidence, and available bridge/host IDs. Completion polling and legacy post-action observation stay on the original connection. Neither disconnect nor process exit is treated as operation success, and uncertain actions are not retried. Successful correlated completion, explicit bridge/host failures, and legacy null-receipt success remain compatible. Diagnostics do not include action payloads.

Validation (original head 26f4d93, final head a5edab8):

  • Current-main baseline: 989163781a0e5ce6ac629e99a7d140a47e0db1ad, sample 187117c03f2c4dc710c75a6657aac8bdf2e3ed8f, freshly built Debug addon with the pinned godot-cpp revision, official Windows x64 Godot 4.7 5b4e0cb0f, Bun 1.4.0.
  • Full Start → loading complete → Back → End → Cancel → End → OK flow with UI waits. Normal sample game/script/scene unchanged; separate nonzero-exit fixture changes only get_tree().quit() to get_tree().quit(23).
  • Transparent bridge traffic capture: normal headless 3/3 and rendered 3/3 reproduced generic disconnect errors on baseline, with accepted host request ID 6, null completion polls, final handler log and process exit 0. Nonzero fixture: one run per mode, both exit 23; rendered was ambiguous, while headless received a correlated successful completion. Fixed matrix: all eight return isError: true, completion_unconfirmed, awaiting_completion, host ID 6 and bridge command ID. No final action replay. Received completion and process exit status are separate evidence.
  • Rendered mode used OpenGL Compatibility on NVIDIA RTX 5060. These are Windows results; the issue's original Linux 1.0.7 rendered success is separate evidence. No inference that headless mode itself causes the race, or that final completion was never generated.
  • 34 MCP tests pass, including normal/abrupt close, never-connected, lost/malformed receipt, completion correlation, immediate close after success, host failure/rejection, legacy compatibility, run_test stop and bounded replay timeout. Workspace typechecks, WebMCP and Value tests pass.
  • Required independent read-only gua_auditor reviewed the cumulative change and found no actionable defects. It independently ran the initial 32-test suite and MCP typecheck; two final compatibility cases were then validated by the parent.
  • The 13 new regression cases also pass on repository-pinned Bun 1.3.14. Its pre-existing input-metadata.test.ts times out at get_version identically with frozen baseline and fixed sources on Windows (the full suite passes on Bun 1.4.0 and in PR TypeScript CI). No unrelated metadata workaround is included.

Direct connections without the logging proxy also reproduced the ambiguity on baseline in both modes (normal exit 0 and nonzero fixture exit 23, one each). Three direct fixed runs retain completion_unconfirmed, awaiting_completion, and request ID 6. The rendered normal fixed run received the correlated successful completion (request ID 6, epoch 1, frame 301, revision 7), returned isError: false, and exited 0. This confirms that a genuine completion remains successful and that the terminal result depends on timing.

Rebased cleanly onto latest main b49ed96214fff59b31f6362687f6443fd6e1da91 after packaging PR #163 merged. git range-diff confirms an identical #153 patch; packaging does not change MCP, GDScript addon, core, runtime or bridge source. New head a5edab8fbfe169965487cb90dd2ec821cd7716dc again passes 34 MCP tests and all workspace typechecks. The original automated review completed with no findings and a positive reaction; final-head CI passed all 17 jobs and automated review completed without findings, with a positive reaction and no unresolved threads. The superseded original CI run had 16 passing checks and one still compiling when canceled.

Latest-main direct smoke: normal and nonzero-exit fixtures in both modes (four runs) all retain completion_unconfirmed / awaiting_completion and host ID 6. The previously verified native addon is reused because the affected native and GDScript runtime source is unchanged by the packaging base update.

Contract validation (all tests in packages/mcp/test/action-outcome.test.ts):

Required behavior Violation Assertion and evidence
Preserve uncertainty and correlation after accepted normal/abrupt disconnect Bare error loses send/receipt state; reconnect or retry Disconnect tests assert error, completion_unconfirmed/awaiting_completion/sent, host and bridge IDs, exactly one click/connection. Baseline real-engine 7/8 traced and 4/4 direct lose diagnostics; fixed engine runs retain them.
Separate initial refusal from lost receipt Fabricated send/acceptance Initial refusal asserts not_sent/before_send/false/no IDs/zero commands; lost receipt asserts sent/awaiting_receipt/no host ID/one click.
Require matching completion False success for another request ID8 completion for accepted ID7 asserts unconfirmed ID7. Valid correlated completion remains successful.
Preserve legacy and failure compatibility; stop uncertain sequences Changed valid responses or next action replay Exact response assertions for correlated/legacy success and explicit bridge/host errors; run_test asserts one click; replay timeout75ms asserts unconfirmed ID and elapsed<1s.

Execution: final head a5edab8, Windows Godot4.7, Bun1.4.0, bun test packages/mcp/test 34pass/121assertions, bun run check all workspace checks pass; new13 tests also pass pinned Bun1.3.14. CI17/17pass and automated review completed at this exact head with positive reaction and no unresolved threads. Engine matrix: issue evidence; local JSON/traffic/process logs in evidence/{baseline-verified,baseline-direct,fixed,fixed-direct,latest-main-direct}, archived in issue-153-evidence.zip.

Latest main3ab2617 changes only AGENTS.md from tested baseb49ed96; runtime/test source remains identical. Contract mapping and evidence handoff are applied within the existing audit gate. Unverified: Linux/macOS real-engine behavior, native completion generation timing, published-package consumers. Unit transport fault injection and repository packaging CI do not prove those paths. Exit status never confirms operation success.

Selected violation-detection proof: validate-contract-mutation.py exports the unchanged final head to a disposable directory. Removing structured diagnostic serialization causes both accepted-disconnect tests to fail exactly at line66 (missing outcome/stage/send/ID); removing completion-ID validation causes the mismatched-completion test to fail at line141 (incorrect success with ID8). Logs: contract-mutation.log and contract-correlation-mutation.log. Mutations are outside the submitted checkout and restored. Startup attempts missing disposable dependency links are excluded; the final attempts reached these precise assertions.

The existing read-only gua_auditor completed the new contract-evidence handoff without actionable gaps. It independently inspected latest main's AGENTS.md, mapping, PR body, mutation script and both logs, evidence README, test source and authoritative JSON summaries/sample traces. It confirmed both deliberate violations reached the exact intended assertions, disposable source was restored byte-identically, and the submitted checkout remained clean. It inspected engine artifacts rather than repeating engine runs or mutations. Its prior independently executed validation was 32 MCP tests and typecheck; the final 34 tests / 17 CI results were supplied by the parent.

Final merge gate verified unchanged head a5edab8, current main 3ab2617, 17 successful CI checks, no unresolved review threads and positive completed automated review. PR #165 was squash-merged as 095d101 at 2026-10-02T09:25:33Z. GitHub confirms issue #153 CLOSED. No releases or deployments performed.

@link1345
link1345 marked this pull request as ready for review October 2, 2026 06:23
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-02T06:51:26.714583Z a5edab8 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@link1345
link1345 force-pushed the fix/153-terminal-disconnect branch from 26f4d93 to a5edab8 Compare October 2, 2026 06:49
@link1345
link1345 merged commit 095d101 into main Oct 2, 2026
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.

MCP: 終了ボタン押下後の切断が接続失敗だけで返り、操作の完了状態を判別できない(Gua 1.0.7)

1 participant