Refactor PR 1: architecture ledger + shared test fixtures#990
Merged
Conversation
Foundation for the god-object refactor's anti-regression mechanism. - docs/ARCHITECTURE.md: living ownership ledger (schema + rows) and AGENTS.md pointer - ArchitectureLedgerConsistencyTests: validates ledger schema, guard-test Type.Method format, and that named guard tests actually exist in the tests source - tests/OpenClaw.TestSupport (Shared-only): TempDirectory, EnvironmentScope, CliHarness, FakeMcpServer, SettingsDataBuilder - GatewayRecordBuilder lives in OpenClaw.Connection.Tests so the shared lib stays Connection-free (no transitive Connection dep on CLI tests) - Migrated GatewayRegistryTests to TempDirectory; de-duplicated the WinNode FakeMcpServer No production (src/) changes: tests and docs only. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First PR of the god-object refactor. It establishes the anti-regression foundation the rest of the plan depends on: a living architecture ledger plus a shared test-support library. This is deliberately the safest PR to go first because it makes zero production changes.
docs/ARCHITECTURE.md— living ownership ledger (machine-readable table): ownership rules, god-object map, "when you touch X → extract toward Y", and single-source owners.AGENTS.mdnow points at it as required reading and forbids re-addingclosedresponsibilities.ArchitectureLedgerConsistencyTests— validates the ledger schema,guard_testType.Methodformat, and that named guard tests actually exist in source (renaming/deleting a guard without updating the ledger fails CI).tests/OpenClaw.TestSupport(Shared-only) —TempDirectory,EnvironmentScope,CliHarness,FakeMcpServer,SettingsDataBuilder.GatewayRecordBuilderlives inOpenClaw.Connection.Testsso the shared lib stays Connection-free (no transitive Connection dep leaks onto the CLI tests).GatewayRegistryTests→TempDirectory; the WinNodeFakeMcpServerinternal copy de-duplicated to the shared one.Scope / behavior guarantee
No
src/(production) files changed or added — every change is undertests/,docs/, or the solution file. The shipped app's runtime behavior cannot change because none of its code changed. The one intentional non-additive change is test-only:FakeMcpServernow retries the port bind (strictly more robust).Validation
Anti-regression guard, proven by negative test (temporarily broke the ledger, confirmed CI fails, then restored):
Ledger_ids_are_uniqueFAILS:Duplicate ledger ids: test-temp-dir.Named_guard_tests_exist_in_test_sourcesFAILS:no method 'NoSuchGuardMethod' was found in a type 'TestSupportFixtureTests'.Real behavior proof
Ran this PR's build in no isolation (owned the Release single-instance), enabled Local MCP Server, then exercised the live server. Runtime is unaffected by the change (as expected — no
src/diff), and the MCP contract this PR actually touches (theFakeMcpServerdedup) works end-to-end.Live app / connection (
app.status):{ "connectionStatus": "Connected", "overallState": "Ready", "operatorState": "Connected", "nodeState": "Connected", "nodeConnected": true, "nodePaired": true, "gatewayVersion": "2026.6.11", "sessionCount": 1, "nodeCount": 1 }MCP discovery —
winnode --list-tools: 52 tools returned (system.*, app.*, canvas.*, device.*, screen.*, stt.*, tts.*).MCP invocation —
winnode --command:device.info→"appVersion": "0.6.13-bkudiess-refactor-architecture-plan.1"(confirms the live server is this branch's build).system.which {"bins":["cmd","git"]}→ resolved both paths.Raw MCP JSON-RPC over
http://127.0.0.1:8765/:tools/list→jsonrpc 2.0, 52 tools.tools/call device.status→isError=false, real system snapshot returned.Runtime log (
%LOCALAPPDATA%\OpenClawTray\openclaw-tray.log, tokens redacted):Computer-use UI proof — captured the Permissions page (non-personal) via
app.navigate permissions. Copied UI diagnostics from the live window:Rubber-duck review
Dual-model adversarial review (GPT-5.5 + Claude Opus 4.8) on the diff. All consensus findings fixed before this PR:
SettingsDataBuilderaliasing (returned a shared instance) → now replays overrides onto a fresh instance perBuild(); independence test added.Type.Methodformat + source-existence enforcement.FakeMcpServerport TOCTOU race → retry on bind failure.TestSupport→Connectionlayering leak → madeTestSupportShared-only; builder moved toConnection.Tests.Not verified / blocked
nodes invokepath not separately exercised; node connection to the live gateway (Connected,nodePaired: true) is shown inapp.status, and MCP discovery/invocation covers the command contract this PR touches.Cleanup
The no-isolation proof run used the real profile: Local MCP Server was toggled on for the proof and restored to its prior value (off) afterward, and the temporary tray instances were stopped. The other worktree's tray instance was stopped to free the single-instance lock and was not relaunched.
Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com