Repository navigation
feat(remote): remote --report in rc-v0.3.1, with its tests in the stack's structure - #820
Merged
Merged
Conversation
…g private in it Three tests for the diagnostics zip that support asks a user for: - test-redact.js: redact() masks a planted JWT, token / apikey / password values, a Bearer value, an email, a UUID, the home folder, the user name and the host name, and leaves a plain status line unchanged. - test-remote-device-log.js: the device log writes every line with a UTC timestamp, masked, including lines no list knows. Only the private kinds, each printed as the device prints it, stay out: a tool call keeps its name and outcome but not its arguments, metadata, results or error details, also with an empty tool name or one with a space or a colon; the ready block's email; the sign-in link and code; an error object's properties (a spawn error's arguments carry the session tokens). It rotates at 1 MB and, past the last file, keeps exactly DEVICE_LOG_FILES files named by deviceLogName(), and keeps debug lines without --debug while the terminal output stays the same. - test-remote-report.js: `remote --report` against a local stand-in (mcp-info, Supabase REST, realtime websocket; Date header 90 s ahead) writes the zip to the home folder with report.txt, report.json and the device log. It reads the clock skew and runs the network checks: a Supabase REST 401 (no sign-in) reads "reachable", a 503 "answered with a server error", a closed port "not reachable". It counts Desktop Commander MCP processes (a stand-in running its dist/index.js counts; with `remote`, or another app's dist/index.js, it doesn't). It reports npm's version, the Node executable and the entry script (home folder as ~) with their install kinds, the device id once in the Device section, and only yes/no facts about the rest of device.json. After saving it uploads the same bytes, with the user id (the saved token's sub) and the device id as headers, and prints the report id. --no-upload sends nothing; without device.json there are no id headers; a 500 keeps the zip and says "Not sent"; a silent server is aborted by the timeout. The address is the server's diagnosticsUrl (https only), else the default; without a server address the report is still sent. Every existing device log file goes into the zip, oldest first. A session without tokens is no signed-in data, and on macOS / Linux the zip is saved owner-only (0o600). It never starts sign-in or a refresh, and planted tokens, emails, the home path and the user name never reach the zip. Each test sets its own temporary HOME / USERPROFILE (main's runner gives tests none) and uses no network. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Users report that Remote Desktop Commander shows offline while it runs, that calls time out, or that it disconnects often. Nothing on their machine records why: the device writes only to the terminal, without timestamps, mixed with tool arguments and results, the email and sign-in codes. `desktop-commander remote --report` collects what support needs into ~/desktop-commander-report-<date>.zip, sends it to support, and exits. It doesn't start the device and never opens sign-in, so it also works when sign-in is broken. - report.txt and report.json: versions (Desktop Commander, Node, npm, OS), the Node executable and the entry script (home folder as ~) with their install kinds, the clock skew from the server's Date header, DNS / TLS / mcp-info timings, whether Supabase REST is reachable (any HTTP answer; a 5xx is a server error) and a realtime websocket heartbeat, whether a proxy is set, whether Desktop Commander MCP is running now (count and earliest start, from the process list), the device id, yes/no facts about the saved session, and telemetryEnabled and clientId. - device-log/: the device's history. A normal `remote` run now passes its console output through diagnostics/device-log.ts, which writes every line, masked and timestamped, to ~/.desktop-commander-device/remote.log, rotating at 1 MB (DEVICE_LOG_FILES = 3 files, named by deviceLogName(), no list). It drops only the private kinds: a tool call's arguments and results (its name and outcome stay; the rules key on their fixed words, so an empty name, or one with a space or a colon, can't let them through), the ready block's email, the sign-in link and code, and the properties of error objects (a spawn error's arguments carry the session tokens). Debug lines are kept without --debug; the terminal output doesn't change. - diagnostics/upload.ts: after saving, the zip is POSTed to DC_DIAGNOSTICS_URL (tests), else the diagnosticsUrl that /api/mcp-info names (https only), else https://diagnostics.ds-c09.workers.dev, with the user id (the saved access token's sub, decoded locally) and the device id as headers and a 30 s timeout. The terminal prints the report id the Worker answers, or "Not sent (<reason>)" and keeps the zip. --no-upload only saves. Nothing is refreshed, nothing signs in, device.json is only read. The zip is saved owner-only (0o600), like the device log; "signed-in data" means a token is there. - diagnostics/redact.ts: one masker for JWTs, token / key / password values, Bearer values, emails, UUIDs, the home folder, the user name and the host name. New code is in src/remote-device/diagnostics/; device.ts and remote-channel.ts are unchanged. The zip is written with pizzip, already a dependency. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… Worker An integration test (npm run test:integration) for the upload, against the live services: 1. remote --report exits 0, saves the zip in the home folder, and prints "Sent to Desktop Commander support. Report id: <8 chars>" with the Worker's alphabet (no 0, 1, I or O). 2. remote --report --no-upload saves the zip and sends nothing. 3. A body that isn't a zip, POSTed straight to the Worker, gets 415. It runs with a temporary HOME / USERPROFILE whose device.json looks like a real sign-in, with ids fixed and reserved for tests (never real Supabase ids): device 00000000-0000-0000-0000-000000000001 and a fake, unsigned access token whose sub is 00000000-0000-0000-0000-000000000000. The CLI only reads sub; nothing signs in or refreshes. Its uploads land like any report, under <user>/<device>/<YYYY-MM-DD-HHMM>-<id>.zip. The upload goes to the default address; DC_DIAGNOSTICS_URL and MCP_SERVER_URL can point it elsewhere. The report id is printed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… most Review: the rotation could leave any number of log files; keep the last 7 days, no more. Each UTC weekday now has its own files: remote-mon.log, rotating at 1 MB into remote-mon.1.log and remote-mon.2.log as before. The first write on a weekday whose remote-<day>.log is over a day old (last week's) removes that day's 3 files before today's lines start. The names are a fixed set, so the log holds at most 7 days x 3 files, 21 MB, and nothing lists the folder. remote --report packs the 21 names that exist, oldest first by modification time. DeviceLog takes the clock and the rotation size as options, so the tests move the day without sleeping. They cover the names, rotation within a day, a new day, last week's files removed for that weekday only, a same-day restart that appends, and two weeks that never leave more than 21 files. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ames
Review: the upload goes through /api/mcp-info's diagnosticsUrl only, with no
fallback; remove DEFAULT_DIAGNOSTICS_URL.
uploadReport() now takes the address from its options only. The built-in
default, the defaultUrl option and the DC_DIAGNOSTICS_URL override are
gone. When the server names no address, nothing is uploaded, the zip stays
saved, and the terminal says "Not sent (the server named no upload
address)". Until production's remote-dc-mcp sets DIAGNOSTICS_URL, reports
stay local.
A refused upload shows the Worker's message from its {code, message}
answer: "Not sent (the server answered 429: too many reports, try again in
a minute)".
The server's address is used if it is https, or http on this machine
(127.0.0.1, localhost, ::1): the tests' stand-in now names its own address
in /api/mcp-info instead of an environment variable. The integration test
saves a report with the CLI and uploads it with the Worker's address it
holds itself, then checks that a non-zip is refused with the Worker's
message.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review: a Node path that no rule recognizes was reported as "global install"; it should be "unknown". Only the known global-install paths now give "global install": Program Files/nodejs, /usr/local/bin and /usr/bin. nvm, fnm, Volta, asdf, mise, Homebrew and Claude Desktop stay as they were, and anything else is "unknown"; the report shows the path itself anyway. nodeKind() is exported and tested with sample paths. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review: use a timestamp instead of savedHoursAgo. report.json's device.savedHoursAgo becomes savedAt: device.json's modification time as ISO (UTC). report.txt shows it as "saved YYYY-MM-DD HH:MM UTC", in the same form as the device log's times. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… --report #813's report test brought its own HTTP + websocket server and spawned the CLI with a hand-built env. The shared helpers now cover what it needs, all off by default, so the tests already using them behave as before. helpers/remote-stand-in.js: startRemoteStandIn() takes serverAheadSec (the Date header runs that far ahead), diagnostics (mcp-info names the stand-in's POST /diagnostics, which keeps each upload and answers with a report id, or with an error set by failUploads()) and realtime (the websocket opens and phoenix heartbeats are answered). A test can point mcp-info at another Supabase (supabaseUrl) and set the REST root's status (restRootStatus). Every request is kept in `requests`. helpers/remote-device.js: runRemote(env, args, { execPath, timeoutMs }) runs `desktop-commander remote <args>`; runLogout() is its --logout. package.json: ws as a devDependency, at the lockfile's 8.18.3. The stand-in's realtime websocket uses it; until now it came only through other packages. test-remote-device-logout.js, test-remote-device-session-without-id.js and integration/remote-device-restart.js pass unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
#813 wrote both for main, whose runner gives tests no temporary home, so each set HOME / USERPROFILE for the whole file and ended with process.exit(). On rc they follow the stack's structure: runTests() and runIfMain(), and no file-wide HOME. test-redact.js: every case reads the runner's home (redact() reads os.homedir() on each call and only reads). The one case that needs a home not given by its real path (macOS's temporary folder is under /var, a link to /private/var) sets HOME / USERPROFILE itself and restores them, as test-error-sanitization.js does, then removes its folder. The user and host name checks, which passed silently for a name under 3 characters (redact() leaves those alone), are each a case that says so with skip(). test-remote-device-log.js: its log folders come from createTempDir(); the home is only read, for the default folder and the masking. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…tand-in #813 wrote test-remote-report.js for main: its own temporary home, its own HTTP + websocket server, the CLI spawned with a hand-built env, an in-process HOME swap, and process.exit() after in-process fetch() calls, which can abort Node on Windows. On rc it follows the stack's structure. - Home: the files are planted in the home the runner gives; outside one the test skips (isTestHome()). The cases that need another home (the 21 log files, no device.json, a session without tokens) get one from createTestEnv(). The diagnosticsUrl case calls collectReport() in this process with the runner's home, so only MCP_SERVER_URL is set and restored around it. - device.json is written with writeDeviceConfig(), holding a session from the stand-in's login(); its ids are the stand-in's. - Stand-in: helpers/remote-stand-in.js with serverAheadSec 90, diagnostics and realtime; its `requests`, `uploads`, `heartbeatsAnswered`, failUploads(), restRootStatus and supabaseUrl replace the test's own. - The CLI runs through runRemote(), with execPath for the nvm-link case. - The two checks that passed silently now say so with skip(): the zip's file mode on Windows, and the user name when it is under 3 characters (a case of its own). - The device.json.lock comment said nothing on main creates the lock. On rc a device's save and `remote --logout` hold it (device.ts); the report still says nothing about it. - runTests() and runIfMain(); the stand-in closes in a finally. The assertions are #813's. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
#813's integration/remote-report-upload.js made its own temporary home and env, spawned the CLI itself, and ended with main().catch(process.exit). On rc the integration runner gives every file a temporary home: the test writes device.json there with writeDeviceConfig() (outside one it skips), runs the CLI with runRemote(), and ends through runIfMain(). The reserved test ids, the Worker address and the three cases are unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…F-8 BOM (#692) Desktop Commander reads a config.json saved as "UTF-8 with BOM" (Notepad, PowerShell 5's Set-Content -Encoding UTF8): parseConfig() in config-manager.ts drops the leading U+FEFF (e316877). The report parses the file with a plain JSON.parse, which rejects it, so for such a file it says "telemetry: not set · client id: none" while Desktop Commander has both. test-remote-report.js: one case, with the file written as in test-config-bom.js (02fff89). On this commit it fails: report.json has settings { telemetryEnabled: null, clientId: null } instead of the saved { telemetryEnabled: false, clientId: … }. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…oes (#692) For a config.json saved as "UTF-8 with BOM", the report said "telemetry: not set · client id: none" although Desktop Commander reads both: the report parsed the file with a plain JSON.parse, which rejects the leading U+FEFF. report.ts reads it through parseConfig(), the one config-manager.ts reads it with (e316877), now exported. No new BOM handling. report.js is still imported only by `remote --report`, so test-startup-imports.js passes. test-remote-report.js: the BOM case passes (it fails on the previous commit), with the other cases. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
1 participant
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.
base:
rc-v0.3.1· brings in #813remote --report(#813, merged onmainas576db4b5) comes into rc-v0.3.1. Its 4 test files were written formain, without #781's test structure; here they follow it. On the way, the report now reads aconfig.jsonsaved with a UTF-8 BOM, as Desktop Commander already does on rc.What this does
10631170), not a copy, so git sees them as already there when rc goes back tomain. No conflicts. Inremote.ts, rc'sremote --logout(under device.json's lock) stays next to #813's--report. #813'sfsimport goes with the old logout line, its only use.main's runner gives tests no temporary home. Now the files are planted in the runner's home (outside one, the test skips), and cases that need another home get one fromcreateTestEnv(). One redact case needs a home that isn't given by its real path (the macOS/varlink): it sets HOME itself and restores it, astest-error-sanitization.jsdoes.test-remote-report.jshad its own HTTP + websocket server and a hand-built env. The sharedremote-stand-in.jsgets options for the report, all off by default, andremote-device.jsgetsrunRemote().runIfMain()instead ofprocess.exit(), which can abort Node on Windows after afetch()(the report test makes some in-process). The checks that passed silently (a user or host name under 3 characters, the zip's file mode on Windows) now say so withskip().parseConfig()fromconfig-manager.ts(#692,e3168771), so it gets what Desktop Commander gets.Where to look
test/helpers/remote-stand-in.js:serverAheadSec(the Date header runs ahead),diagnostics(mcp-info names the stand-in'sPOST /diagnostics, which keeps uploads and answers with a report id or afailUploads()error),realtime(the websocket opens and heartbeats are answered);supabaseUrlandrestRootStatuscan be set; every request is kept inrequests. Off by default:test-remote-device-logout.jsandtest-remote-device-session-without-id.jspass unchanged in both full suites,integration/remote-device-restart.js5/5 on Windows.test/helpers/remote-device.js:runRemote(env, args, { execPath, timeoutMs });runLogout()is its--logout.test/test-remote-report.js: the planted home, the stand-in, and the new BOM case. Thedevice.json.lockcomment said nothing creates the lock; on rc a device's save andremote --logouthold it.test/test-redact.js,test/test-remote-device-log.js,test/integration/remote-report-upload.js.src/config-manager.ts:parseConfig()is exported, unchanged.src/remote-device/diagnostics/report.ts:settings()uses it.report.jsis still imported only byremote --report.package.json:wsas a devDependency, at the lockfile's 8.18.3 (the stand-in's websocket; it was there only through other packages).How to verify
Answers that change
None: tool answers and descriptions are unchanged.
Commits and test results
1c2e590710631170) into rc-v0.3.1.43ce52cbrunRemote(), andwsas a devDependency.b4930029test-redact.jsandtest-remote-device-log.jsuse the runner's home.b7c78709test-remote-report.jsuses the runner's home, the shared stand-in andrunRemote().62592ac967f643801770b835parseConfig().npm test: 171/171 on both. Checks skipped for the platform, missing rights or a missing tool: 8 on Windows, 10 on macOS.test-redact.js,test-remote-device-log.js,test-remote-report.jsandtest-startup-imports.js: 4/4 on both. One skip on Windows (the zip's file mode).remote-report-upload.jsagainst the live Worker: 1/1 on both (report idsJ4K8VT2MandPJCEGMSY; a non-zip is refused with "the server answered 415: not a zip file").c862375c, this branch before a last rewrite. It differs from1770b835only in 4 skip texts: each repeated the file name the runner already prints.67f64380on both machines (report.json settings{ telemetryEnabled: null, clientId: null }) and passes at1770b835.b4930029,b7c78709and1770b835build and their own tests pass.🤖 Generated with Claude Code