Skip to content

Commit f9db36d

Browse files
authored
Merge pull request #2816 from bobleer/bob/communications-validation
fix(remote-connect): cancel relay tasks and prevent duplicate mobile commands
2 parents e917108 + 627e74b commit f9db36d

9 files changed

Lines changed: 1042 additions & 227 deletions

File tree

‎docs/architecture/peer-device-mode.md‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,23 @@ FS) and must not be mixed with Peer Device Mode.
239239

240240
## Transport
241241

242+
- The shared `services-integrations::remote_connect::relay_client` owns one
243+
cancellable connection task. Its read and write futures remain full-duplex,
244+
while heartbeat, dial, write deadlines and reconnect belong to that same
245+
lifetime. Disconnect joins cancellation; replacing or dropping the client
246+
retires the old socket and its reconnect attempts. A generation fence prevents
247+
an old connection from publishing state into its replacement. Initial dial
248+
failure returns to `Disconnected`. Reconnect restores room/account context,
249+
including a server-assigned room id, before admitting new outgoing commands.
250+
The outgoing queue holds at most 64 messages and reports saturation explicitly;
251+
its failed-socket contents are never replayed. Dial/write deadlines are 15s,
252+
heartbeat cadence is 30s, with due heartbeats taking priority over queued
253+
commands, and inbound idle detection is 75s. These transport
254+
facts do not imply authentication success or application-level acceptance.
255+
- Mobile delegated-auth recovery retries only a real Relay HTTP 401. An
256+
authenticated, encrypted host error mentioning `Unauthorized` or an upstream
257+
`HTTP 401` is an application result and must not cause a second mutation.
258+
Account-generation fences still apply before and after credential refresh.
242259
- Controller: `PeerDeviceTransportAdapter` wraps product `invoke` as
243260
`RemoteCommand::HostInvoke` over `account_device_rpc`.
244261
- HostInvoke on the controller is **priority-queued** with four requests in

‎docs/development/communications-e2e.zh-CN.md‎

Lines changed: 336 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Export reviewable coverage worksheets; this is not a test or a second registry.
2+
import { readFile, mkdir, writeFile } from 'node:fs/promises';
3+
import { dirname, resolve } from 'node:path';
4+
import { fileURLToPath } from 'node:url';
5+
6+
const root = resolve(dirname(fileURLToPath(import.meta.url)), '../..');
7+
const args = process.argv.slice(2);
8+
if (args.length !== 2 || args[0] !== '--output') {
9+
throw new Error('Usage: node scripts/diagnostics/export-communications-matrix.mjs --output <new-directory>');
10+
}
11+
const output = resolve(args[1]);
12+
const registry = JSON.parse(await readFile(resolve(root,
13+
'src/crates/contracts/product-domains/src/generated/remote-surface-registry.json'), 'utf8'));
14+
if (registry.schemaVersion !== 1 || !Array.isArray(registry.operations)) {
15+
throw new Error('Unsupported Product Operation Registry shape');
16+
}
17+
const source = await readFile(resolve(root,
18+
'src/crates/services/services-integrations/src/remote_connect.rs'), 'utf8');
19+
const commandBody = source.match(/pub enum RemoteCommand \{\n([\s\S]*?)\n\}/)?.[1];
20+
if (!commandBody) throw new Error('RemoteCommand enum was not found');
21+
const commands = [...commandBody.matchAll(/^ ([A-Z]\w*)\s*(?:,|\{)/gm)]
22+
.map((match) => match[1].replace(/([a-z0-9])([A-Z])/g, '$1_$2').toLowerCase());
23+
if (commands.length === 0 || new Set(commands).size !== commands.length) {
24+
throw new Error('RemoteCommand inventory is empty or contains duplicates');
25+
}
26+
const guide = await readFile(resolve(root, 'docs/development/communications-e2e.zh-CN.md'), 'utf8');
27+
const cases = [...guide.matchAll(/^\| ((?:ENV|RLY|SSH|MOB|BOT|PEER|DSP|COMBO|EXT)-\d+) \| ([^\n]+) \| ([^\n]+) \|$/gm)]
28+
.map((match) => [match[1], match[2], match[3]]);
29+
if (cases.length === 0 || new Set(cases.map(([id]) => id)).size !== cases.length) {
30+
throw new Error('Manual case inventory is empty or contains duplicate ids');
31+
}
32+
const csv = (rows) => rows.map((row) => row.map((cell) =>
33+
`"${String(cell ?? '').replaceAll('"', '""')}"`).join(',')).join('\n') + '\n';
34+
// A fresh directory prevents a rerun from erasing manually recorded evidence.
35+
await mkdir(output, { recursive: false });
36+
await writeFile(resolve(output, 'product-operations.csv'), csv([
37+
['operation', 'remote_workspace_policy', 'peer_policy', 'cli_peer_policy', 'cli_reason',
38+
'ssh_result', 'desktop_peer_result', 'cli_peer_result', 'composition_result', 'evidence'],
39+
...registry.operations.map((op) => [op.id, op.remoteWorkspace, op.peer.kind, op.cliPeer.kind,
40+
op.cliPeer.reason, 'NOT_RUN', 'NOT_RUN', 'NOT_RUN', 'NOT_RUN', '']),
41+
]));
42+
await writeFile(resolve(output, 'remote-commands.csv'), csv([
43+
['command', 'room_mobile_result', 'account_mobile_result', 'feishu_result',
44+
'telegram_result', 'weixin_result', 'peer_result', 'evidence'],
45+
...commands.map((command) => [command, ...Array(6).fill('NOT_RUN'), '']),
46+
]));
47+
await writeFile(resolve(output, 'manual-cases.csv'), csv([
48+
['case_id', 'steps', 'expected', 'environment', 'result', 'evidence', 'issue'],
49+
...cases.map(([id, steps, expected]) => [id, steps, expected, '', 'NOT_RUN', '', '']),
50+
]));
51+
const inventory = {
52+
registryDigest: registry.digest,
53+
productOperations: registry.operations.length,
54+
remoteCommands: commands.length,
55+
manualCases: cases.length,
56+
note: 'Inventory only. NOT_RUN is not coverage. Preserve environment and evidence per execution.',
57+
};
58+
await writeFile(resolve(output, 'inventory.json'), JSON.stringify(inventory, null, 2) + '\n');
59+
console.log(JSON.stringify(inventory, null, 2));

‎sdk/typescript/test/managed-host.test.ts‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -193,15 +193,21 @@ test(
193193
});
194194
const [chunk] = (await once(transport.readable, "data")) as [Buffer];
195195
const descendantPid = Number.parseInt(chunk.toString("utf8").trim(), 10);
196-
if (!transport.readable.readableEnded) {
197-
await once(transport.readable, "end");
198-
}
199-
assert.equal(transport.hasExited(), true);
200-
201196
try {
197+
if (!transport.readable.readableEnded) {
198+
await once(transport.readable, "end");
199+
}
200+
// Pipe EOF can arrive before Node delivers ChildProcess's exit event.
201+
// Establish actual parent exit before testing orphan-group cleanup.
202+
const deadline = Date.now() + 1_000;
203+
while (!transport.hasExited() && Date.now() < deadline) {
204+
await new Promise((resolve) => setTimeout(resolve, 10));
205+
}
206+
assert.equal(transport.hasExited(), true);
202207
await transport.close();
203208
assert.equal(await processExited(descendantPid), true);
204209
} finally {
210+
await transport.close().catch(() => {});
205211
if (!(await processExited(descendantPid, 50))) {
206212
try {
207213
process.kill(descendantPid);

‎src/crates/interfaces/app-server/src/management/owner.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1779,6 +1779,7 @@ mod tests {
17791779
openbitfun_core::native_hooks::NativeHookOverview {
17801780
enabled: true,
17811781
project_hooks_enabled: true,
1782+
remote_workspace_unsupported: false,
17821783
files: vec![
17831784
openbitfun_core::native_hooks::NativeHookFileView {
17841785
scope: "user",

‎src/crates/services/services-integrations/AGENTS.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ cargo check -p openbitfun-services-integrations --no-default-features
111111
cargo test -p openbitfun-services-integrations --no-default-features --features mcp --test mcp_contracts
112112
cargo test -p openbitfun-services-integrations --no-default-features --features remote-ssh --test remote_ssh_contracts remote_ssh_disabled_contracts::
113113
cargo test -p openbitfun-services-integrations --no-default-features --features remote-ssh-concrete --lib remote_ssh::manager::tests::workspace_
114+
cargo test --locked -p openbitfun-services-integrations --no-default-features --features remote-connect --lib remote_connect::relay_client::tests::
114115
cargo test -p openbitfun-services-integrations --no-default-features --features file-watch --test file_watch_contracts
115116
cargo test --locked -p openbitfun-services-integrations --no-default-features --features deep-research --lib deep_research::tests::
116117
pnpm run check:core-boundaries

0 commit comments

Comments
 (0)