Repository navigation
refactor: apply the code quality review across the workspace - #74
Open
lauridskern wants to merge 118 commits into
Open
lauridskern wants to merge 118 commits into
lauridskern wants to merge 118 commits into
Conversation
Tooling accepted -prerelease tags that the Codex updater's schema rejects, so a prerelease would publish while every installed updater reported unavailable. No prerelease was ever tagged, so packages/shared/src/release-version.ts now defines the one plain major.minor.patch format, parser and comparator for both.
Adds android-ids.ts for the adb serial rule and avd: IDs, and device-ref.ts for the shared deviceId rule, simulator UUID check and platform-keyed target. Device IDs accept the widest current rule everywhere, so wireless adb serials reach CPU, Display FPS, logs, device choice and inspection.
SessionRegistry owns the random IDs, caps, uniqueness keys, idle expiry and idempotent closes that six session classes wrote by hand. A closing session keeps its slot until its stop settles, and closing a device or the registry reports every failed stop. RevisionBuffer and Waiters replace the long-poll waiters the buffers and streams repeat.
…egistry CPU and Display FPS share one CollectorSession for startup, monitor failure and stop, and one sample buffer, so FPS no longer keeps its own copy. Each keeps its abort timing: CPU aborts only while its monitor attaches, FPS at once. Every registry now closes like CPU after the failed-close fix: closes are shared, sessions stay reserved until stopped, and closing all of them reports every failure. Log sessions gain closeDevice. collector.ready measures each collector from open to its first recording phase, failure or cancellation.
…tor and Android streams Both stream classes run on SessionRegistry and Waiters, and share awaitOpen and closeGracefully for the socket handshake and the close-then-terminate deadline. Android streams reserve their slot while serve-emu starts, so concurrent opens cannot pass the cap.
The Android video resource dropped the request's abort signal, so a cancelled read held its long poll for a second. Reads now end on abort, as simulator frame reads do.
Share one lockfile, release profile and target directory between the telemetry wrapper, ios-fps and ios-mirror, and build each binary with -p so idevice features stay per helper. Source hashes cover the workspace files and include the Rust telemetry wrapper once, licenses come from each binary's own crate graph, and the dSYM copy is shared with the iOS log reader build.
ios-fps and ios-mirror copied the same usbmuxd, CoreDeviceProxy, software tunnel and RSD handshake sequence. The new device-tunnel crate opens it with a caller label and optional deadline and returns a typed TunnelError, so each helper keeps its own label, timeouts and messages, and the foreground detector maps DeviceNotFound instead of matching message text.
…t build time The wrapper passes Timing by discriminant, so its hand-written variants must match the C enum's order. build.rs now parses both and fails the build on a mismatch, without editing the C sources Baguette hashes.
… ladder Make the protobuf decoder an iterator that fails on truncation, overlong varints, invalid field numbers and unsupported wire types, and decode the uncompressed VCMediaNegotiationBlobV2 stream groups in one strict pass that rejects truncated uint32 fields instead of a lenient decode followed by a completeness check. Both answer formats now become one list of HEVC candidates ranked by a single precedence ladder. For uncompressed answers a payload without a type reports PayloadTypeMissing, a stream without an SSRC reports SsrcMissing, and an out-of-range payload type only wins when no payload type is usable. Split the module into offer, answer and schema, drop the test-only encoders, and build the test fixtures with the production writers.
Remove the unused RTCP sender-report parser and scope the remaining allowances to the RTP header fields the capture path does not read and to the HEVC module until its reset method gains a caller.
…Sentry files apart
npm run build:hosts built Codex through buildCodexHost, which hard-coded
.sentry/ui/codex and .sentry/server/codex, so a host-test build replaced the
release package's source maps. Every app now declares one
buildHost({ outdir, environment, sentryRoot }) in its plugin-host.json (new for
Codex and the Mac app), host tests pass .sentry/host-tests, and build:hosts,
the package steps and build:mac stage the shared runtime and call the
discovered builder instead of copying each app's page and server config.
error-diagnostics.ts holds each error's category, command, discovery, device apps, definition, Android startup and install integrity diagnostics, classifies process failures once for every vocabulary, and builds the tags and contexts capture sites send. captureServerError and captureUiError read them all. Tags are unchanged.
IosMirrorSessions repeated the hub's single pending read, closed-subscription check and delivered-generation gate across the RPC hop. Sessions are now a registry of hub subscriptions; the hub throws expected errors and the reply carries their outcome, so the client keeps them expected. The hub's unused change callback and report() are gone, and the client and service share one newline-JSON reader.
apps/codex-plugin/tooling/package.mjs repeated package-plugin.mjs's steps. Codex now packages through it from plugin-host.json data (its release folder and plugin path, launcher files, kept skill agents, symlinks and ZIP limit), each host's marketplace format lives in its app, foreign files a package must not ship come from the other hosts' declared files, and releaseArchive names every host's ZIP for the package step, release-metadata and the release workflow. The Codex package layout is unchanged.
The app read ui://mobile-dev/mac.html over the page's stdio session with its own initialize, then the page initialized again on the same session. The app now loads the bundled page directly, and the page reads the server's telemetry environment, opt-out and anonymous identity from ui://mobile-dev/mac-telemetry.json after its own initialize, then starts UI telemetry with them.
The shared server read MOBILE_DEV_HOST and compared it to "mac". The Mac server now passes telemetryHost: "mac" to runStdioServer, which keeps that label over the connecting client's name. Tags and attributes are unchanged.
Shared UI telemetry matched data-host="mac" in the page's markup. The Mac page now passes host: "mac" to startUiTelemetry, so its first measurements and every call keep the same label.
…out of shared Only the panel used them, so they now live in packages/ui/src/tracesift/ with their tests in packages/ui/test.
…nition tool-results.ts adds guarded, jsonResult, failureResult and the read-only and write annotations. registerSessionTools registers a session's batch resource with its start, read and close tools, with hooks for the logs start result's _meta and the FPS timeOrigin. Batch resource reads now end when their request is cancelled. Tool names, schemas and results are unchanged.
…e picks as invalid input
…nd iPhone video WebCodecsVideo holds what both decoders repeated: a decoder per generation, keyframe gating, one keyframe request a second while recovering, current-decoder output, and one base64 decoder.
…pers once telemetryEnabled() replaces the per-recorder MOBILE_DEV_TELEMETRY checks, and the count and gauge wrappers send nothing while telemetry is off; metric names, units and attributes are unchanged. Storage measurements start only with telemetry on, so an opted-out server no longer walks the recordings folder every five minutes, and their failures go through captureServerError. instrumentMcpServer is the one patch of setRequestHandler and applies handler wrappers inside its error accounting; tool icons become such a wrapper, which also fixes tool-icons.ts's two type errors.
recording.css and tracesift.css sit beside their components and style.css imports them, so every page still gets the same rules. The unused sidebar and chart tokens are gone from theme.css, which lets the TraceSift explorer use its package's own chart fallbacks without an override, and two reduced-motion blocks the global rule already covers are dropped.
SimulatorPanel is an external store whose snapshot SimulatorView renders: display, notice, start, screenshot and device-button state are pure functions of a typed panel state (simulator-state.ts), so a re-render no longer resets what the panel set in the DOM. Only the canvas stays imperative (device-screen.ts, device-input.ts). - device-discovery.ts lists devices, polls, and builds the picker's options with a pure deviceOptions. - stream-transports.ts holds the three transports as one table with their tools, capabilities and receive loops; telemetry reads its stream-open tools from it. - device-stream.ts opens, interrupts and closes sessions. One interrupt lifts a held touch where the input outlives the interruption and forgets it where the input is gone or the device lifted it. - device-frame-layout.ts computes the frame and screen styles from the stage, video and bezel. - The panel creates its picker, settings, app tools and annotation stores and passes them to the views, replacing the element-keyed store lookup, late-bound callbacks and the picker's synthetic change event. Hosts construct DeviceWorkspace before rendering and pass its panels to SimulatorView. Blocked input now asks the agent to repair it only on hosts with a chat. The panel tests are split by surface over one fake server keyed by tool name.
Storage and App Tools listed a device's installed apps on their own; both now ask DeviceAppsStore, which shares a listing in flight per device and lists again for each later request. The app Performance and Storage last showed on each device lives there too. Each panel keeps its own preference order: Performance its pick then the foreground app, Storage its pick while installed then the newest running app, App Tools the newest running app, its current app, the app last picked on the device, then the first.
…at send waits The App tab and Storage share an installed-apps listing per device, CPU batch processing now includes schema validation, chat sends wait for a registering composer, and TraceSift drops tool failures that arrive after their card or restore no longer applies.
Resolves the panel store against the workspace layout's new owner: DeviceWorkspace applies the WorkspaceTools layout to each panel's visibility, hosts construct it before rendering and pass its panels with fullscreen to SimulatorView, and the performance snapshot keeps its derived running and display phase on the Store base. Core tests that drive the settings and storage stores use their new constructors.
On Node 24.14 to 24.17, running the same large esbuild bundle in a third JSDOM window segfaulted in V8's garbage collector (ClearStaleLeftTrimmedPointerVisitor during a Sparkplug stack check). The runner showed it only as a file-level "test failed" in recording-reference and ui-telemetry. Under 32 to 48 parallel copies the crash hit 1-2% of runs; it never happened with --no-sparkplug, Node 22.23.2 or Node 24.18 and later. With distinct source per window it did not recur in 1,280 stressed runs.
Shared tests that exercised core's ServeEmu, app control, discovery commands, FrameTimeline and buffers now live in packages/core/test. The display FPS panel tests and the UI discovery telemetry test move to packages/ui/test. Shared keeps the tests of its own schemas and diagnostics; the command label assertions became their own core test.
The storage change feed and table paging tests now select an app through the panel's public API instead of its private update.
captureSentry owns Sentry's in-memory transport, flush and close, and restores MOBILE_DEV_TELEMETRY through setEnv. connectClient replaces the in-memory MCP client setup collector-startup used locally, waitFor the polling helpers and fakeApp the hand-written App fakes.
connectClient replaces the transport pair, client, cleanup and two connect calls in each tool test. It gained a setup hook for clients that answer elicitation requests before they connect.
…be4388ef2b2 # Conflicts: # apps/codex-plugin/server/plugin-updates.ts # apps/codex-plugin/shared/plugin-updates.ts # apps/codex-plugin/test/plugin-updates.test.ts # apps/codex-plugin/tooling/smoke-package.mjs # docs/architecture.md # docs/telemetry.md # packages/core/src/baguette.ts # packages/core/src/index.ts # packages/core/src/ios-mirror-client.ts # packages/core/src/panel-tools.ts # packages/core/src/plugin.ts # packages/core/src/serve-emu.ts # packages/core/src/server-feature.ts # packages/core/src/stdio-server.ts # packages/core/src/telemetry.ts # packages/core/test/telemetry.test.ts # packages/mcp-app/test/card-app.test.ts # packages/shared/src/error-reporting.ts # packages/ui/src/comparison-controller.ts # packages/ui/src/comparison-series.ts # packages/ui/src/components/comparison-card.tsx # packages/ui/src/components/recording-card.tsx # packages/ui/src/components/tracesift-cards.tsx # packages/ui/src/components/tracesift/contribution-chart.tsx # packages/ui/src/components/tracesift/react-contribution.tsx # packages/ui/src/components/tracesift/task-contribution.tsx # packages/ui/src/recording-controller.ts # packages/ui/src/simulator-panel.ts # packages/ui/src/style.css # packages/ui/src/tracesift-controller.ts # packages/ui/src/tracesift-panel.ts # packages/ui/test/comparison-series.test.ts # packages/ui/test/simulator-panel.test.ts # packages/ui/test/tracesift-ui.test.ts # tooling/build.mjs # tooling/host-runtime.mjs # tooling/hosts/build.mjs # tooling/package-plugin.mjs # tooling/plugin-hosts.mjs # tooling/smoke-plugin.mjs # tooling/ui-page.mjs
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.
Applies the whole-codebase code quality review: shared models replace the concepts each package had reimplemented, oversized modules are split, and the bugs the review turned up are fixed. Behavior is unchanged unless listed below.
Bundled packages are untouched. Nothing under
packages/runtimes/(the serve-emu fork, agent-device) changes, and neither do Baguette, its patch (packages/native/baguette), its build scripts, the sharedtelemetry.c/telemetry.hits build hashes, orperfetto.h. Our own code uses serve-emu differently in one place: it now passes the CLI's existing--token.Bug fixes
--token, and core sends it on every request and on the/wsupgrade. Backends the user started (port 3300,SERVE_EMU_URL) still work without one.npm run build:hostsoverwrote the release's Sentry source maps (.sentry/ui/codex,.sentry/server/codex). Host-test builds now write under.sentry/host-tests.ERR_INVALID_STATE. The upload is now one streamed Blob; this was also the flakytracesift-clienttest.docs/telemetry.mdsays.mobile_log_sourceslists Android devices and Metro targets in parallel: worst case 5 s instead of 10 s.packages/shared/src/release-version.ts). The test deployments feat(claude): show the panel in a browser and download its runtime #72 added (such as0.1.172-test.1) are accepted everywhere; the marketplace branches still take only plain releases.Structure
devices.ts,device-ref.tsandandroid-ids.ts;platformandkindare now required.errorMessagelives in one place.packages/ui/src/tracesift/.SessionRegistryandRevisionBufferreplace six hand-rolled session tables and five long-poll waiters.guarded/jsonResulttool helper andregisterSessionTools.ServerFeature.createPluginis now synchronous)..d.tsis generated.simulator-panel.ts(1020 lines) is now a store-backed panel with focused modules (transport table, typed screen state, input, layout);element-store.tsand its DOM service locator are gone.callTool,sendChat,openCursorSessionand aStorebase.OptionSelectreplaces three select components.runMcpViewfor every MCP Apps page.apps/codex-plugin/tooling/package.mjsis gone).buildHostcontract per app and one native helper registry (tooling/native-helpers.mjs).plugin-updates.tsis split.test-support.Behavior changes worth a look
[]%) on CPU, Display FPS, logs, device choice and inspect UI.docs/telemetry.md:native.frame.processnow times only depacketization.collector.ready.*metrics.Native binaries
ios-fps, ios-mirror, ios-logs, android-cpu and android-fps were rebuilt locally (Rust 1.94.1, NDK 27.1) so their
release.jsonhashes match the sources. The native artifact keys changed, so the next release run rebuilds every helper with the release toolchain, Baguette included.Verification
npm test:tsc -p tsconfig.jsonhas 0 errors (22 on main).typecheck:telemetrypasses.npm run build,packageandtest:packagepass on the integrated tree.package:claudeandtest:claude(with the runtime download and browser panel),test:cursor,build:mac,test:macandtest:updatesalso pass.build:hostsandtest:hosts: 16 passed, 5 skipped by design, including main's browser-panel specs.test:visual:dockerpasses with the existing baselines; no screenshot changed._meta).Not run:
test:stream,test:reconnect,test:logs,test:ios-mirror*,test:ios-cpu.test:codex-plugin.Merge with main
Main's #57, #70, #71, #72 and #73 are merged in and adapted to the new structure:
StoreandsendChat.plugin-store.ts.ServerFeature.Cards in Claude no longer offer "open in workspace": that button would call a tool Claude's server doesn't register.
Left out on purpose
ToggleGroupdoesn't render the same.telemetry.citems (surface mapping, unused error API, DSN injection): changing that file forces a Baguette rebuild.ps, since the bundled CLI only takes it as an argument.