Home

mweinbach / agent-coworker

publicmweinbach/agent-coworker
Overview Code History Branches Pull requestsIssuesInsights
main
HomeOverview Code PRsIssues

chore: post-Rust-rewrite dead code cleanup (+ tracing, Windows fs hardening, notification wiring)

merged
Stack 2/2
#393 opened by mweinbachchore/post-rust-dead-code-cleanup→main
Conversation20
mweinbachopened this pull requestAuthor· last week
Commits
0
Files changed…

Post-Rust-rewrite cleanup: removes dead, unused and redundant code left over after #389 (the Rust core rewrite), and fixes a few bugs found along the way. Net change is +3.6k / −57.8k lines across 616 files, split into three commits by area.

Bug fixes found during the audit

  • Rust logs were silently dropped. 23 files emit tracing:: events, but no subscriber was ever installed. cowork-server and cowork-cli now install a stderr subscriber (RUST_LOG, default warn, ANSI only on a TTY). Electron reads the handshake from stdout only, so stderr is safe.
  • Windows path-safety gap. The server's /cowork/fs/* routes used a plain File::open on Windows (no FILE_FLAG_OPEN_REPARSE_POINT), and same_file_identity always returned true. The three copies are replaced by one shared cowork-storage::safe_open with the hardened behavior.
  • Unwired notifications. Rust emits cowork/todos, cowork/log, error and cowork/session/backupState, but the desktop never subscribed to them. They now reach the existing reducer branches.

Rust (refactor(rust): …)

  • Removed ~35 unused crate dependencies (cargo-machete is clean) and ~50 pub items nothing referenced, along with compat-only params and aliases.
  • One provider connection resolver, cowork-providers::connection (select_provider_transport / resolve_provider_connection). It replaces about six copies across harness skill_improvement and the server's turn, memory, fork and agent routes; behavior is unchanged.
  • Windows sandbox setup sync: cowork-providers now calls cowork-sandbox's implementation instead of keeping its own copy.
  • Small helpers deduped: now_iso, pick_boolean, host_platform, web-search options and the reasoning-effort list.
  • New __sandbox-exec --print-command: prints the exact transformed launch as JSON. It shares one prepare path with real execution, and is what lets the TS sandbox go.
  • Rename: cowork-server/src/turn_runner.rs → jsonrpc_turn.rs, so it's no longer confused with the harness turn runner.
  • Model-stream leftovers: removed model_stream_chunk from the backpressure classification and the never-called persist_model_stream_chunks. The DB tables and migrations are kept.
  • Deleted the unused prompts/memory-{generator,consolidator}.md. They describe a tool-calling loop that the Rust generator doesn't implement.

Desktop / mobile (refactor(desktop,mobile): …)

  • Model-stream path removed. The Rust server never emits model_stream_chunk / model_stream_raw, so their handling and the ~2.7k-line replay stack are gone (modelStream*, openaiResponsesReplay, googleInteractionsStreamParts).
  • Other legacy protocol handling removed:
    • SessionEvent types with no Rust producer;
    • legacyToolLogs;
    • legacy sidecar manifests.
  • Shared helpers: chatRetry, toolFeedState, markdownPreview, activityGroups and tool-card helpers move to src/shared for both apps. Mobile's copy of markdownPreview had drifted.
  • Unused dependencies removed: seven [@univerjs](/univerjs)/preset-sheets-* packages, plus mobile clsx and ws.
  • Dead config and code removed:
    • COWORK_HARNESS_TERMINAL_LOGS;
    • test-only components and exports;
    • migration leftovers in build_desktop_resources.ts, including the docs bundling that never shipped.

Root TS, tooling, docs (chore: …)

  • Test-only src/ modules deleted (23 files) along with their tests: idempotency ledger, tool retry, provider continuation, think tags, approval, subprocess and others. All of them have Rust equivalents.
  • TS sandbox removed. Office WASM qualification now gets its sandbox launch from the Rust --print-command. Only src/platform/windowsSandboxHelper.ts and scripts/scratchRoots.ts remain.
  • Pricing: pricing.ts / costTracker.ts are trimmed to what the desktop uses.
  • Single protocol schema: crates/cowork-protocol/src/ws_jsonrpc_schema.json is now the only copy; the identical 1 MB docs/generated copy and its generator are gone.
  • Repo housekeeping:
    • .claude/skills is now symlinks into .agents/skills;
    • removed stale hooks, .cursor state, examples/, the vestigial package.json files field, duplicate scripts and historical TS-era docs;
    • CI, docs, knip and biome config are updated to match.

Behavior changes to review

  • Transcripts saved by the old TS server still replay. The 2.7k-line live model-stream replay stack is replaced by a small transcript-only decoder, store.legacyTranscript.ts. It rebuilds assistant replies, reasoning and tool rows from assistant_message / reasoning / model_stream_chunk. Raw provider frames (model_stream_raw) are ignored, because normalized chunks mirror them. legacyTranscriptId lookups are kept (review feedback).
  • test/office-wasm.integration.test.ts / linuxReadView now expect the Rust seccomp re-exec launcher, and bind-mount that binary read-only into the positive read view.
  • Mobile markdown preview now uses the desktop's bold-wrapper repair.

Verification

  • Static checks all clean:
    • bun run typecheck (root, desktop, tooling), mobile typecheck, bun run check and bun run knip;
    • bun run docs:check and bun install --frozen-lockfile;
    • cargo fmt, cargo check --workspace --all-targets (0 warnings), cargo clippy --workspace --all-targets -D warnings;
    • cargo-machete.
  • Windows cross-check: cargo check --target x86_64-pc-windows-gnu passes for storage, tools, providers, sandbox and server.
  • bun run test: everything passes except 5 tests in 3 files, which fail the same way on main in the dev VM:
    • no unzip;
    • no staged sidecar binary;
    • no reaping of grandchild processes;
    • no default rustup toolchain outside the repo.
  • cargo test: everything passes except tests that need bubblewrap or LibreOffice, which the dev VM lacks and CI installs.
  • Not run locally: Playwright quality gates (no Electron) and the real sandboxed office qualification (no bubblewrap). Both rely on CI.
cursor[bot]commented· last week

This PR is too large for Bugbot to review. It changes 54,113 lines and 3,957,463 characters. Split the change into smaller pull requests to get a review.

chatgpt-codex-connector[bot]commented· last week
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

ReviewStatusCommitReview trigger
📝 Code Review✅ Completed 2026-11-12T02:52:36.961587Z56f372fNew commits
🔒 Security Review✅ Completed 2026-11-12T02:57:01.819390Z56f372fNew commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Review

chatgpt-codex-connector[bot] · last week · 2 file comments

2 open

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9ff9861a98

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Review

mweinbach · last week

Review

mweinbach · last week

Review

chatgpt-codex-connector[bot] · last week · 1 file comment

1 open

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d91841ccbf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Review

mweinbach · yesterday

Review

chatgpt-codex-connector[bot] · yesterday · 1 file comment

1 open

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d5104d1b7b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Review

mweinbach · yesterday

Review

chatgpt-codex-connector[bot] · yesterday · 2 file comments

2 open

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 56f372ff03

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sign in to comment.

Stack

2/2
1

Merge readiness

Checking mergeability after the indexing worker computes the current branch state.

Autopilot

Debug

Reviews

Approved0
ReviewersNone yet

Configure an OpenRouter key in repository settings.

apps/desktop/src/app/store.feedMapping.ts
lines 218-222
View file
  1. chatgpt-codex-connector[bot]· last week
  • apps/desktop/src/app/store.actions/thread.tslines 261-262
  • apps/desktop/src/app/store.legacyTranscript.ts
    lines 236-237
    View file
    1. chatgpt-codex-connector[bot]· last week
    apps/desktop/src/app/store.legacyTranscript.ts
    lines 225-228
    View file
    1. chatgpt-codex-connector[bot]· yesterday
    apps/desktop/src/app/store.legacyTranscript.ts
    lines 22-23
    View file
    1. chatgpt-codex-connector[bot]· yesterday
  • apps/desktop/src/app/store.legacyTranscript.tslines 188-191
  • Update README.md

    #125 mobile-fixes ← main

    Updated 5 months ago

    merged
    2

    chore: post-Rust-rewrite dead code cleanup (+ tracing, Windows fs hardening, notification wiring)

    P2 Badge Preserve assistant replies when hydrating saved transcripts

    For existing local transcripts containing assistant_message or model-stream events, removing these payload types makes mapTranscriptToFeed() replace saved replies with unhandled-event system rows rather than assistant messages. hydrateThreadSelection() still uses this mapper through its local-transcript fallback when no harness snapshot is available, so affected historical chats lose their displayed answers, and buildContextPreamble() omits those answers when continuing the conversation. Preserve decoding of persisted events until those transcripts have been migrated.

    Useful? React with 👍 / 👎.

  • mweinbach· last week

    Fixed in b213a73. Old transcript events are decoded again, but only for transcripts, in apps/desktop/src/app/store.legacyTranscript.ts. Replay now restores assistant_message, reasoning / assistant_reasoning / reasoning_summary, and model_stream_chunk text, reasoning and tool parts, so it no longer shows "Unhandled event" rows. A final assistant_message that repeats streamed text is skipped. model_stream_raw and other retired diagnostic events are ignored. This means buildContextPreamble() gets the assistant replies again. Tests are in store-feed-mapping.transcript.test.ts.

  • View file
    1. chatgpt-codex-connector[bot]· last week

      P2 Badge Retain original transcript IDs until their files are migrated

      For previously rekeyed threads, older transcript events can remain in the file identified by the persisted legacyTranscriptId; the previous rekey implementation explicitly retained that ID. This change stops looking up that file and also removes the field from persistence sanitization, without migrating the transcript files. When hydration falls back to local history, PersistenceService.readTranscript() reads only the requested filename and returns an empty array for missing files, so these threads now show incomplete or empty history. Retain the legacy lookup or migrate the files before dropping the field.

      Useful? React with 👍 / 👎.

    2. mweinbach· last week

      Fixed in b213a73. I restored legacyTranscriptId everywhere it was removed: the type, the desktop and Electron persistence sanitizers, the bootstrap schema and selection fallback, control-socket session merging and selection, and the jsonRpcWorkspace rekey bookkeeping. transcriptIdsForThread reads [legacyTranscriptId, sessionId, id] again, so rekeyed threads still load their original transcript files. The related test assertions are back too: archived-session-auto-delete, control-socket.sessions, thread-reconnect, and the fixtures.

    P2 Badge Restore citation annotations when replaying legacy text

    For saved transcripts whose citation metadata is carried by a text_end chunk, this default branch discards its annotations instead of attaching them to the streamed assistant message. The new transcript-only decoder restores reply text, but unlike the previous assistant_text_end handler, it never restores these annotations. Local-transcript hydration consequently displays answers without their clickable source citations, since FeedRow.tsx passes item.annotations to DesktopMarkdown. Preserve the text_end annotation handling in this decoder.

    Useful? React with 👍 / 👎.

  • mweinbach· yesterday

    Fixed in d5104d1. applyLegacyModelStreamChunk now handles text_end chunks by validating part.annotations as an array of record objects and attaching them to the matching streamed assistant message (ignoring commentary phase chunks). Covered by a regression test in apps/desktop/test/store-feed-mapping.transcript.test.ts.

  • P2 Badge Restore streamed tool arguments during legacy transcript replay

    For historical transcripts containing tool_input_start followed by tool_input_delta chunks without a final tool_call, this decoder silently drops every argument delta. Local-transcript hydration therefore restores the tool result but loses the command or file path that produced it. The previous mapper supported this sequence, explicitly covered by the removed coalesces multiple tool-input deltas and applies the authoritative final error test. Accumulate and decode these deltas into the matching tool item's arguments instead of ignoring them.

    Useful? React with 👍 / 👎.

  • mweinbach· yesterday

    Fixed in 56f372f. applyLegacyModelStreamChunk now accumulates tool_input_delta chunks per tool key, decodes them with parseStructuredToolInput into the tool item's args (including on tool_input_end), and avoids clobbering streamed args when tool_call omits input. Covered by a regression test in apps/desktop/test/store-feed-mapping.transcript.test.ts.

  • P2 Badge Replay native search activity from saved provider frames

    For historical transcripts containing native web searches, model_stream_raw frames are not redundant with the normalized tool chunks: the previous replay path reconstructed nativeWebSearch items from response.output_item.added/done events carrying web_search_call. Silencing these frames drops the search query, result, and source metadata during local-transcript hydration. The removed maps native web search raw events into visible tool feed activity regression test contains this exact sequence. Preserve transcript-only decoding for provider-native activity that normalized chunks do not reconstruct.

    AGENTS.md reference: AGENTS.md:L83-L83

    Useful? React with 👍 / 👎.

    View file
    1. chatgpt-codex-connector[bot]· yesterday

      P2 Badge Reset step-local stream identities on repeated start events

      When a saved turn contains multiple model steps that reuse a text stream ID, resetting only when turnId changes concatenates separate assistant replies. The removed multi-step replay test records start → text_delta(s1, "progress note") → finish → start → text_delta(s1, "final answer"); this decoder produces progress notefinal answer, then also appends the final assistant_message because its paragraph-separated text does not match. This corrupts both the restored feed and buildContextPreamble(). Handle repeated start boundaries by clearing step-local identities while retaining the history needed for final-message deduplication.

      AGENTS.md reference: AGENTS.md:L83-L83

      Useful? React with 👍 / 👎.

    #393 main ← chore/post-rust-dead-code-cleanup

    Updated yesterday

    merged