feat(server): write cdp_port to server.json for auto-discovery

[Nikhil (shadowfax92)]

Summary

• Adds optional cdp_port to the ~/.browseros/server.json discovery file written on server startup, alongside the existing server_port.
• Lets external tools (CLI, agents) auto-discover the CDP WebSocket port instead of only the HTTP server port.

Design

ServerDiscoveryConfig gains an optional cdp_port?: number. In main.ts, the startup writeServerConfig() call now passes cdp_port: this.config.cdpPort ?? undefined. The CDP port is optional in config (cdpPort: portSchema.nullable(), set only via --cdp-port / BROWSEROS_CDP_PORT), so when absent the key is omitted from the JSON entirely rather than written as null.

Test plan

• bun run typecheck passes across all packages.
• Start the server with --cdp-port <port> and confirm ~/.browseros/server.json contains cdp_port.
• Start without a CDP port and confirm the key is omitted.

[Nikhil (shadowfax92)]

Greptile (@greptileai) review

[Nikhil (shadowfax92)]

Claude (@claude) review

[claude]

Claude finished Nikhil (@shadowfax92)'s task in 59s —— View job

---

Code Review

• Read changed files and gather context
• Analyze changes and formulate review

---

Clean, minimal change overall. A few observations:

main.ts — redundant ?? undefined

// main.ts:98
cdp_port: this.config.cdpPort ?? undefined,

By the time execution reaches writeServerConfig, this.config.cdpPort is guaranteed to be a truthy number. Lines 58–61 perform an early exit if it's falsy:

if (!this.config.cdpPort) {
  logger.error('CDP port is required (--cdp-port)')
  process.exit(EXIT_CODES.GENERAL_ERROR)
}

So ?? undefined is dead code at this call site — it will never convert null → undefined in practice. The intent (omitting a null key from JSON output) is valid in general, but doesn't apply here.

This is harmless and slightly defensive against future reordering, so it's not a blocker — but it could mislead a reader into thinking cdpPort might be null at this point. Fix this →

logStartupSummary not updated

logStartupSummary() (line 232–237) only logs the HTTP server URL. Since CDP port is now a first-class discoverable value, logging it there alongside the HTTP server URL would improve observability during startup — especially useful when debugging external tool connectivity.

cdp_port?: number — optional typing is correct

Making it optional in ServerDiscoveryConfig is the right call for forward-compatibility: consumers reading older server.json files (written before this change) won't have the field. The optionality accurately reflects the read-side contract even though the write side always provides it now.

Overall

LGTM with the above as nits. No bugs, no security issues, no type unsafety introduced. The JSON.stringify-drops-undefined behaviour is the correct mechanism to omit the key when it would be absent.

---

[greptile-apps]

Greptile Summary

Exposes the CDP WebSocket port in ~/.browseros/server.json so external tools (CLI, agents) can auto-discover it alongside the existing HTTP server port.

• ServerDiscoveryConfig gains an optional cdp_port?: number field, and main.ts passes this.config.cdpPort when calling writeServerConfig() on startup.
• Because cdpPort is already enforced as non-null by an early-exit guard before writeServerConfig is reached, the ?? undefined coercion in main.ts is redundant and can be simplified to a direct assignment.

Confidence Score: 4/5

Safe to merge; the change is additive and writes a new optional field to the discovery JSON without touching any existing logic.

The only observable wrinkle is that the ?? undefined coercion in main.ts is dead code — the early-exit guard already ensures cdpPort is non-null before writeServerConfig is called — but this has no effect on runtime behaviour.

main.ts line 98 — the redundant null-coercion is worth cleaning up for clarity.

Important Files Changed

Filename	Overview
packages/browseros-agent/apps/server/src/main.ts	Passes cdp_port to writeServerConfig; the ?? undefined guard is redundant since the CDP port is already enforced as non-null by the early-exit check at lines 58–61.
packages/browseros-agent/packages/shared/src/types/server-config.ts	Adds optional cdp_port field to ServerDiscoveryConfig — clean, minimal change with correct typing.

Sequence Diagram

sequenceDiagram
    participant Main as Application.start()
    participant Guard as CDP Port Guard
    participant CDP as CdpBackend
    participant HTTP as HTTP Server
    participant FS as writeServerConfig()
    participant JSON as server.json

    Main->>Guard: if (!cdpPort) exit(GENERAL_ERROR)
    Guard-->>Main: cdpPort guaranteed non-null
    Main->>CDP: connect(cdpPort)
    CDP-->>Main: connected
    Main->>HTTP: createHttpServer(serverPort)
    HTTP-->>Main: listening
    Main->>FS: "writeServerConfig({ server_port, cdp_port, url, ... })"
    FS->>JSON: JSON.stringify → server.json

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
packages/browseros-agent/apps/server/src/main.ts:98
The `?? undefined` is a no-op here. The guard at lines 58–61 exits the process when `cdpPort` is falsy (null, 0, or undefined), so by the time `writeServerConfig` is reached, `this.config.cdpPort` is guaranteed to be a valid non-null number. The null-to-undefined coercion (which would suppress the key in `JSON.stringify`) can never activate in practice.

```suggestion
        cdp_port: this.config.cdpPort,
```

_{Reviews (1): Last reviewed commit: "feat(server): write cdp_port to server.j..." | Re-trigger Greptile}

[greptile-apps]

Greptile Summary

Exposes the CDP WebSocket port in ~/.browseros/server.json so external tools can auto-discover it alongside the existing HTTP server port. The change adds an optional cdp_port?: number field to the ServerDiscoveryConfig interface and passes it through writeServerConfig() on startup.

• ServerDiscoveryConfig gains cdp_port?: number; JSON.stringify will omit the key when the value is undefined, which is the correct omit-vs-null behavior.
• In main.ts, cdp_port: this.config.cdpPort ?? undefined is passed to writeServerConfig() — the ?? undefined is unreachable because a missing CDP port causes process.exit() four lines earlier.

Confidence Score: 4/5

Safe to merge; the change is additive and confined to startup config writing with no effect on request handling.

Both files have minimal, well-scoped changes. The only notable issue is a redundant ?? undefined expression that is never evaluated at runtime, which can be simplified.

No files require special attention; main.ts has the minor dead-code expression on line 98.

Important Files Changed

Filename	Overview
packages/browseros-agent/apps/server/src/main.ts	Passes cdp_port to writeServerConfig; the ?? undefined fallback is unreachable dead code since the process exits at lines 58–61 when cdpPort is falsy.
packages/browseros-agent/packages/shared/src/types/server-config.ts	Adds optional cdp_port?: number to ServerDiscoveryConfig interface — minimal, correct change.

Sequence Diagram

sequenceDiagram
    participant Caller as CLI / Agent
    participant Server as BrowserOS Server (main.ts)
    participant FS as ~/.browseros/server.json

    Server->>Server: Check cdpPort (exit if missing)
    Server->>Server: Connect CDP backend
    Server->>Server: Start HTTP server
    Server->>FS: "writeServerConfig({ server_port, cdp_port, url, ... })"
    FS-->>Caller: auto-discovery read
    Caller->>Server: Connect via HTTP (server_port)
    Caller->>Server: Connect via CDP (cdp_port)

Loading

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
packages/browseros-agent/apps/server/src/main.ts:98
The `?? undefined` fallback is dead code. By the time execution reaches this line, `this.config.cdpPort` is guaranteed to be a truthy `number` — the guard at lines 58–61 calls `process.exit()` (a `never`-returning function) whenever `cdpPort` is falsy, so TypeScript also narrows the type to `number` here. The `?? undefined` adds noise without any runtime or type-level effect.

```suggestion
        cdp_port: this.config.cdpPort,
```

### Issue 2 of 2
packages/browseros-agent/apps/server/src/main.ts:232-238
**Startup summary omits the CDP port.** Now that `cdp_port` is written to `server.json` for external tool discovery, the "Services running:" summary would be more consistent if it also logged the CDP endpoint. Operators inspecting startup output currently have to scroll back to the earlier "Connected to CDP on port X" line to find it.

_{Reviews (2): Last reviewed commit: "feat(server): write cdp_port to server.j..." | Re-trigger Greptile}

[github-actions]

❌ Tests failed — 1/1063 failed

Suite	Passed	Failed	Skipped
✅ agent	91/91	0	0
✅ build	10/10	0	0
✅ eval	95/95	0	0
✅ server-agent	246/246	0	0
✅ server-api	63/63	0	0
✅ server-browser	4/4	0	0
✅ server-integration	9/10	0	1
✅ server-lib	252/253	0	1
✅ server-root	59/62	0	3
❌ server-tools	228/229	1	0

Failed tests

• server-tools — observation tools > get_page_content returns markdown text

View workflow run

[greptile-apps]

The ?? undefined fallback is dead code. By the time execution reaches this line, this.config.cdpPort is guaranteed to be a truthy number — the guard at lines 58–61 calls process.exit() (a never-returning function) whenever cdpPort is falsy, so TypeScript also narrows the type to number here. The ?? undefined adds noise without any runtime or type-level effect.

Suggested change

	cdp_port: this.config.cdpPort ?? undefined,
	cdp_port: this.config.cdpPort,

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/browseros-agent/apps/server/src/main.ts
Line: 98

Comment:
The `?? undefined` fallback is dead code. By the time execution reaches this line, `this.config.cdpPort` is guaranteed to be a truthy `number` — the guard at lines 58–61 calls `process.exit()` (a `never`-returning function) whenever `cdpPort` is falsy, so TypeScript also narrows the type to `number` here. The `?? undefined` adds noise without any runtime or type-level effect.

```suggestion
        cdp_port: this.config.cdpPort,
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!