fix: bundle pinned OfficeCLI per desktop target#331
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🧰 Additional context used🧠 Learnings (21)📓 Common learnings📚 Learning: 2026-04-28T05:36:18.200ZApplied to files:
📚 Learning: 2026-04-28T04:56:21.528ZApplied to files:
📚 Learning: 2026-04-28T04:56:18.533ZApplied to files:
📚 Learning: 2026-04-28T08:29:02.858ZApplied to files:
📚 Learning: 2026-04-28T04:38:11.771ZApplied to files:
📚 Learning: 2026-04-22T08:49:47.800ZApplied to files:
📚 Learning: 2026-04-28T04:56:21.338ZApplied to files:
📚 Learning: 2026-04-28T04:38:05.946ZApplied to files:
📚 Learning: 2026-04-28T07:27:49.810ZApplied to files:
📚 Learning: 2026-04-28T04:56:13.350ZApplied to files:
📚 Learning: 2026-04-28T05:36:25.456ZApplied to files:
📚 Learning: 2026-04-25T09:51:18.951ZApplied to files:
📚 Learning: 2026-04-29T13:27:28.494ZApplied to files:
📚 Learning: 2026-04-27T11:19:24.963ZApplied to files:
📚 Learning: 2026-04-20T14:36:31.032ZApplied to files:
📚 Learning: 2026-04-28T11:24:35.312ZApplied to files:
📚 Learning: 2026-04-27T11:18:47.596ZApplied to files:
📚 Learning: 2026-04-28T05:36:24.561ZApplied to files:
📚 Learning: 2026-04-29T10:34:22.399ZApplied to files:
📚 Learning: 2026-04-27T12:59:49.844ZApplied to files:
🪛 ast-grep (0.42.1)packages/desktop-electron/scripts/prepare-officecli.ts[warning] 66-66: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns. (regexp-from-variable) 🔇 Additional comments (4)
📝 WalkthroughWalkthroughAdds explicit pinned OfficeCLI bundling: a manifest entry, a prepare script that downloads and SHA256-verifies platform-specific OfficeCLI assets, CI workflow steps to run that preparation before desktop packaging, inclusion of third-party notices in app resources, and tests validating manifest resolution, workflow ordering, and env handling. Changes
Sequence DiagramsequenceDiagram
participant Workflow as CI Workflow
participant Script as prepare-officecli.ts
participant Manifest as bundled-tools.json
participant GitHub as GitHub Releases
participant FS as File System
Workflow->>Script: bun ./scripts/prepare-officecli.ts --platform, --arch
Script->>Manifest: Read OfficeCLI version & asset mapping
Manifest-->>Script: Return asset name for (platform, arch)
Script->>GitHub: GET SHA256SUMS for release
GitHub-->>Script: SHA256SUMS text
Script->>Script: parseSha256Sums -> expected hash
Script->>GitHub: GET release asset (asset name)
GitHub-->>Script: Binary bytes
Script->>Script: compute sha256(binary) and compare
alt hash matches
Script->>FS: write binary -> ../resources/tools/, set perms
Script->>Script: if host matches target -> run binary --version (OFFICECLI_SKIP_UPDATE=1)
Script-->>Workflow: print destination & success
else hash mismatch
Script-->>Workflow: error/exit non-zero
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 1/3 review remaining, refill in 29 minutes and 56 seconds. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/desktop-electron/scripts/download-tools.ts`:
- Line 189: The call to prepareOfficeCli(platform as SupportedPlatform, arch as
SupportedArch) should be guarded so unsupported targets (e.g., linux-x64) don't
throw and abort the whole script; add a check (e.g., isOfficeCliSupported or a
whitelist for desktop platforms like 'win32'/'darwin') before invoking
prepareOfficeCli, avoid force-casting platform/arch to
SupportedPlatform/SupportedArch, and when the platform is unsupported simply
skip the OfficeCLI step (optionally log a debug/info message) so other tool
downloads continue.
In `@packages/desktop-electron/scripts/prepare-officecli.ts`:
- Around line 68-70: The version check using stdout.includes(normalized) can
false-positive on substrings; modify the check in prepare-officecli.ts to
perform an exact comparison (e.g., trim stdout and compare equal to normalized)
or match with an anchored regex (e.g., ^normalized$) so only an exact version
string passes; update the conditional that throws the Error for OfficeCLI
version mismatch to use this exact-match logic with the existing variables
normalized, expectedVersion and stdout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6234525e-f118-46dc-bd89-78f09e55c453
📒 Files selected for processing (16)
.github/workflows/build.yml.github/workflows/desktop-smoke.ymlREADME.mdREADME_CN.mdTHIRD_PARTY_NOTICES.mdpackages/desktop-electron/bundled-tools.jsonpackages/desktop-electron/electron-builder-app-update.test.tspackages/desktop-electron/electron-builder.config.tspackages/desktop-electron/resources/tools/officeclipackages/desktop-electron/scripts/download-tools.tspackages/desktop-electron/scripts/prepare-officecli.test.tspackages/desktop-electron/scripts/prepare-officecli.tspackages/desktop-electron/scripts/release-workflow-contract.test.tspackages/opencode/src/tool/bash.tspackages/opencode/test/github/desktop-smoke-workflow.test.tspackages/opencode/test/tool/bash-env-source.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: smoke-macos-arm64
- GitHub Check: typecheck
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-windows-app
- GitHub Check: unit-desktop
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-app
- GitHub Check: unit-opencode
- GitHub Check: e2e-artifacts
- GitHub Check: analyze-js-ts
🧰 Additional context used
📓 Path-based instructions (2)
packages/opencode/**/*.ts
📄 CodeRabbit inference engine (packages/opencode/AGENTS.md)
packages/opencode/**/*.ts: UseEffect.gen(function* () { ... })for Effect composition
UseEffect.fn("Domain.method")for named/traced effects andEffect.fnUntracedfor internal helpers; these accept pipeable operators as extra arguments to avoid unnecessary outer.pipe()wrappers
UseEffect.callbackfor callback-based APIs
PreferDateTime.nowAsDateovernew Date(yield* Clock.currentTimeMillis)when you need aDatein Effect code
UseSchema.Classfor multi-field data in Effect schemas
Use branded schemas (Schema.brand) for single-value types in Effect
UseSchema.TaggedErrorClassfor typed errors in Effect schemas
UseSchema.Defectinstead ofunknownfor defect-like causes in Effect code
InEffect.gen/Effect.fn, preferyield* new MyError(...)overyield* Effect.fail(new MyError(...))for direct early-failure branches
UsemakeRuntimefromsrc/effect/run-service.tsfor all services; it returns{ runPromise, runFork, runCallback }backed by a sharedmemoMapthat deduplicates layers
UseInstanceStatefromsrc/effect/instance-state.tsfor per-directory or per-project state that needs per-instance cleanup; do work directly in theInstanceState.makeclosure whereScopedCachehandles run-once semantics
UseEffect.addFinalizerorEffect.acquireReleaseinside theInstanceState.makeclosure for cleanup (subscriptions, process teardown, etc.)
UseEffect.forkScopedinside theInstanceState.makeclosure for background stream consumers — the fiber is interrupted when the instance is disposed
PreferFileSystem.FileSysteminstead of rawfs/promisesfor effectful file I/O in Effect services
PreferChildProcessSpawner.ChildProcessSpawnerwithChildProcess.make(...)instead of custom process wrappers in Effect services
PreferHttpClient.HttpClientinstead of rawfetchin Effect services
PreferPath.Path,Config,Clock, andDateTimeservices when those concerns are already inside Effect code
For backgroun...
Files:
packages/opencode/src/tool/bash.tspackages/opencode/test/tool/bash-env-source.test.tspackages/opencode/test/github/desktop-smoke-workflow.test.ts
packages/opencode/test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (packages/opencode/test/AGENTS.md)
packages/opencode/test/**/*.test.{ts,tsx}: Use thetmpdirfunction fromfixture/fixture.tsto create temporary directories for tests with automatic cleanup. Useawait usingsyntax to ensure automatic cleanup when the variable goes out of scope.
When using thetmpdirfunction with git repository support, pass thegit: trueoption to initialize a git repo with a root commit.
Use theconfigoption intmpdirto write anopencode.jsonconfig file during test setup by passing a partial Config.Info object.
Use theinitoption intmpdirto define custom setup functions that can return extra data accessible viatmp.extra, and use thedisposeoption for custom cleanup logic.
UsetestEffect(...)fromtest/lib/effect.tsfor tests that exercise Effect services or Effect-based workflows.
Useit.effect(...)when the test should run withTestClockandTestConsole. Useit.live(...)when the test depends on real time, filesystem mtimes, child processes, git, locks, or other live OS behavior.
Prefer Effect-aware helpers fromfixture/fixture.tsover building manual runtimes in tests: usetmpdirScoped()for scoped temp directories,provideInstance(dir)(effect)for low-level binding without directory creation,provideTmpdirInstance(...)for single temp instance binding, orprovideTmpdirServer(...)for tests that also need the test LLM server.
Defineconst it = testEffect(...)near the top of the test file and keep the test body insideEffect.gen(function* () { ... }). Yield services directly withyield* MyService.Serviceoryield* MyTool.
Avoid customManagedRuntime,attach(...), or ad hocrun(...)wrappers in Effect tests whentestEffect(...)already provides the runtime.
When a test needs instance-local state, preferprovideTmpdirInstance(...)orprovideInstance(...)over manualInstance.provide(...)inside Promise-style tests.
Files:
packages/opencode/test/tool/bash-env-source.test.tspackages/opencode/test/github/desktop-smoke-workflow.test.ts
🧠 Learnings (30)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:311-328
Timestamp: 2026-04-28T05:36:18.200Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the ripgrep binary download flow (`HttpClientRequest.get` → `fs.writeWithDirs` → `extract`) does NOT verify a pinned SHA-256 checksum. Adding per-platform hash verification (15.1.0 × 6 platforms) requires a dedicated manifest + `crypto.subtle.digest` step and is intentionally deferred to a follow-up PR. Existing mitigations: (1) `VERSION` is pinned to `"15.1.0"`, (2) downloads are over HTTPS from GitHub Releases, (3) PawWork production builds ship a bundled `rg` binary so this download path is a last-resort fallback (system PATH → cached → download). Do NOT re-flag the missing checksum verification in this file until the dedicated follow-up PR lands.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/patch/index.ts:337-346
Timestamp: 2026-04-28T04:38:05.946Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/patch/index.ts`), the BOM-transition surfacing gap — where `Bom.split` strips BOM before building `unified_diff`/`new_content` but `Bom.join` later re-attaches BOM on disk write, meaning BOM changes are not reflected in the diff payload — is intentionally deferred. PR `#270` is an upstream-sync graft; fixing the issue here would mix refactor + bugfix intents and drift the diff from the upstream baseline needed for clean future grafts. A dedicated follow-up PR (or upstream report) will address this. Do NOT re-flag the missing BOM-change surfacing in `ApplyPatchFileUpdate`/`ApplyPatchFileChange` as a blocking issue in PR `#270` or in future sync PRs that carry the same upstream baseline.
📚 Learning: 2026-04-24T17:08:46.780Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 224
File: packages/app/src/i18n/zh.ts:0-0
Timestamp: 2026-04-24T17:08:46.780Z
Learning: In Astro-Han/pawwork PR `#224`, the first-occurrence `PawWork 爪印` branding rule originally specified in issue `#196` was superseded by an updated Chinese-branding spec. On all zh UI surfaces in `packages/app/src/i18n/zh.ts` (e.g., `dialog.model.unpaid.freeModels.title`, `session.new.subtitle`, `sidebar.gettingStarted.line1`), the correct and intentional target is fully localized `爪印` branding — no `PawWork` prefix. Do NOT flag these strings as missing the first-occurrence `PawWork 爪印` rule in future reviews.
Applied to files:
README_CN.mdREADME.md
📚 Learning: 2026-04-24T17:12:26.774Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 224
File: packages/desktop-electron/electron-builder.config.ts:14-18
Timestamp: 2026-04-24T17:12:26.774Z
Learning: In Astro-Han/pawwork, the `localizedMacDisplayNameByChannel` map in `packages/desktop-electron/electron-builder.config.ts` is intentionally kept separate from `localizedAppDisplayName` in `packages/desktop-electron/src/main/app-display-name.ts`. The former is a build-time packaging helper; the latter is a runtime UI helper that localizes the current app name by locale. Coupling them would introduce a build-time dependency on runtime main logic. Do not suggest deduplicating or sharing this mapping — the explicit local table is covered by focused regression tests in `electron-builder-app-update.test.ts`.
Applied to files:
packages/desktop-electron/electron-builder-app-update.test.tspackages/desktop-electron/scripts/prepare-officecli.test.tspackages/desktop-electron/electron-builder.config.tspackages/desktop-electron/scripts/release-workflow-contract.test.tspackages/desktop-electron/scripts/prepare-officecli.ts
📚 Learning: 2026-04-22T08:49:47.800Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/desktop-electron/src/main/index-sidecar-source.test.ts:3-11
Timestamp: 2026-04-22T08:49:47.800Z
Learning: In `packages/desktop-electron/src/main/index-sidecar-source.test.ts` (Astro-Han/pawwork), the test intentionally uses `expect(source).toContain` / `expect(source).not.toContain` string matching against the raw `index.ts` source text as a lightweight sidecar contract guard. The maintainer has explicitly chosen not to introduce an AST parser (e.g., `babel/parser` or acorn) for this purpose. Do not flag these string-based assertions as fragile or suggest converting them to AST-based matching.
Applied to files:
packages/desktop-electron/electron-builder-app-update.test.tspackages/opencode/test/tool/bash-env-source.test.tspackages/desktop-electron/scripts/prepare-officecli.test.tspackages/desktop-electron/scripts/release-workflow-contract.test.tspackages/opencode/test/github/desktop-smoke-workflow.test.ts
📚 Learning: 2026-04-28T05:36:25.456Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/tool/grep.test.ts:16-22
Timestamp: 2026-04-28T05:36:25.456Z
Learning: In `packages/opencode/test/tool/grep.test.ts` (Astro-Han/pawwork, PR `#270`), the test suite intentionally uses a manual `ManagedRuntime.make(Layer.mergeAll(...))` setup and raw `bun:test` test cases rather than the `testEffect(...)` harness. The migration to `testEffect` + `it.live(...)` + `provideTmpdirInstance(...)` is a whole-file rewrite deferred to a dedicated sweep PR that will migrate all tool tests still on the manual ManagedRuntime pattern together. Do NOT re-flag the ManagedRuntime setup in this file as needing harness migration until that sweep PR lands.
Applied to files:
packages/desktop-electron/electron-builder-app-update.test.tspackages/opencode/test/tool/bash-env-source.test.tspackages/desktop-electron/scripts/prepare-officecli.test.tspackages/desktop-electron/scripts/release-workflow-contract.test.tspackages/opencode/test/github/desktop-smoke-workflow.test.ts
📚 Learning: 2026-04-28T04:56:21.338Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/tool/tool-define.test.ts:2-9
Timestamp: 2026-04-28T04:56:21.338Z
Learning: In `packages/opencode/test/tool/tool-define.test.ts` (Astro-Han/pawwork, PR `#270`), the test suite intentionally uses a `ManagedRuntime.make(Layer.mergeAll(Truncate.defaultLayer, Agent.defaultLayer))` wrapper and raw `bun:test` `test(...)` cases rather than the `testEffect(...)` harness. This code was adopted wholesale from upstream via the graft strategy (per `project_upstream_strategy.md`). Migrating to `testEffect` is deferred to a follow-up PR so as not to mix refactor + harness-migration intents and to avoid drifting the diff from the upstream baseline needed for clean future grafts. Do NOT re-flag the `ManagedRuntime.make` usage in this file as needing harness migration until that follow-up lands.
Applied to files:
packages/desktop-electron/electron-builder-app-update.test.tspackages/opencode/test/tool/bash-env-source.test.tspackages/desktop-electron/scripts/prepare-officecli.test.tspackages/desktop-electron/scripts/release-workflow-contract.test.tspackages/opencode/test/github/desktop-smoke-workflow.test.ts
📚 Learning: 2026-04-29T13:27:28.494Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 329
File: packages/app/src/pages/session/session-view-controller.test.ts:5-45
Timestamp: 2026-04-29T13:27:28.494Z
Learning: In Astro-Han/pawwork (`packages/app/src/pages/session/session-view-controller.test.ts` and related Solid + Bun test files), the Bun + Solid test environment does NOT reliably exercise `createMemo((current) => ...)` signal recomputation the way a browser runtime does. Adding signal-driven transition tests (e.g., using `createSignal` + `createRoot` to flip reactive inputs) is misleading in this environment because the reactive invalidation/recomputation path is not faithfully replicated. The correct strategy is: cover pure transition logic with plain unit tests (e.g., `nextSessionViewState`), and cover the browser reactive path with E2E tests (e.g., session-switch E2E spec). Do NOT re-flag the absence of signal-driven `createSessionViewController` tests in this environment as a gap.
Applied to files:
packages/desktop-electron/electron-builder-app-update.test.tspackages/desktop-electron/scripts/prepare-officecli.test.tspackages/opencode/test/github/desktop-smoke-workflow.test.ts
📚 Learning: 2026-04-27T11:19:24.963Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/tool/websearch-auth.test.ts:0-0
Timestamp: 2026-04-27T11:19:24.963Z
Learning: In `packages/opencode/test/tool/websearch-auth.test.ts` (Astro-Han/pawwork), the tests intentionally use a small local `runWith` runner with raw `bun:test` and `Effect.runPromise` rather than the `testEffect` harness. Each test case injects a custom in-memory `Auth.Service` layer; switching to `testEffect` would be style-only churn without changing risk coverage. Do not flag these tests as needing harness migration.
Applied to files:
packages/desktop-electron/electron-builder-app-update.test.tspackages/opencode/test/tool/bash-env-source.test.tspackages/desktop-electron/scripts/prepare-officecli.test.tspackages/desktop-electron/scripts/release-workflow-contract.test.tspackages/opencode/test/github/desktop-smoke-workflow.test.ts
📚 Learning: 2026-04-28T04:56:18.533Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:172-175
Timestamp: 2026-04-28T04:56:18.533Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the `files dies on nonexistent directory` test hardcodes `/tmp/nonexistent-dir-12345` as the missing-directory path. This is upstream-inherited behaviour adopted wholesale via the graft strategy (`project_upstream_strategy.md`). The fix — replacing the hardcoded path with a platform-neutral guaranteed-nonexistent child path derived from the `tmpdir` fixture — is intentionally deferred to a follow-up PR or upstream report, to avoid mixing bugfix + refactor intents and drifting the diff from the upstream baseline future syncs need. Do NOT re-flag the hardcoded `/tmp/nonexistent-dir-12345` path in this file as a blocking or actionable issue until the follow-up PR lands.
Applied to files:
packages/desktop-electron/electron-builder-app-update.test.tspackages/opencode/test/tool/bash-env-source.test.tspackages/desktop-electron/scripts/release-workflow-contract.test.tspackages/opencode/test/github/desktop-smoke-workflow.test.ts
📚 Learning: 2026-04-28T05:36:24.561Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:0-0
Timestamp: 2026-04-28T05:36:24.561Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), after the streaming-Service refactor of `ripgrep.ts`, there is only one code path that strips `RIPGREP_CONFIG_PATH` from the spawned environment (`delete env.RIPGREP_CONFIG_PATH` at `src/file/ripgrep.ts` line ~155). There is no longer a separate "worker" vs "direct" execution mode, so a single test titled "ignores RIPGREP_CONFIG_PATH from spawned env" (with an explanatory comment) is the correct and complete coverage. Do NOT flag the absence of a separate worker-mode RIPGREP_CONFIG_PATH test as a gap.
Applied to files:
packages/desktop-electron/electron-builder-app-update.test.tspackages/opencode/src/tool/bash.tspackages/opencode/test/tool/bash-env-source.test.tspackages/desktop-electron/scripts/release-workflow-contract.test.tspackages/opencode/test/github/desktop-smoke-workflow.test.ts
📚 Learning: 2026-04-28T04:38:21.935Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/test/file/ripgrep.test.ts:9-10
Timestamp: 2026-04-28T04:38:21.935Z
Learning: In `packages/opencode/test/file/ripgrep.test.ts` (Astro-Han/pawwork, PR `#270`), the tests intentionally use a local `run` helper (`effect.pipe(Effect.provide(Ripgrep.defaultLayer), Effect.runPromise)`) rather than the `testEffect(...)` harness. This file was adopted from upstream wholesale as part of an upstream-sync graft; migrating to `testEffect` + `it.live(...)` was deferred to a separate follow-up PR to avoid mixing refactor and harness-migration intents within the sync. Do NOT re-flag the local `run` wrapper as needing harness migration until that follow-up lands.
Applied to files:
packages/desktop-electron/electron-builder-app-update.test.tspackages/opencode/test/tool/bash-env-source.test.tspackages/desktop-electron/scripts/release-workflow-contract.test.tspackages/opencode/test/github/desktop-smoke-workflow.test.ts
📚 Learning: 2026-04-27T11:18:47.332Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/tool/mcp-exa.test.ts:1-186
Timestamp: 2026-04-27T11:18:47.332Z
Learning: In `packages/opencode/test/tool/mcp-exa.test.ts` (Astro-Han/pawwork), the tests intentionally use raw `bun:test` async cases with `Effect.runPromise(...)` and per-case `HttpClient.make(...)` fakes rather than the `testEffect(...)` harness. The maintainer has explicitly decided not to migrate, because the HttpClient fake wiring is itself the behavior under test and switching to `testEffect` would be style churn without changing risk coverage. Do not flag these tests as needing harness migration.
Applied to files:
packages/desktop-electron/electron-builder-app-update.test.tspackages/opencode/test/tool/bash-env-source.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use the `config` option in `tmpdir` to write an `opencode.json` config file during test setup by passing a partial Config.Info object.
Applied to files:
packages/desktop-electron/electron-builder-app-update.test.ts
📚 Learning: 2026-04-27T11:18:47.596Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 271
File: packages/opencode/test/agent/agent.test.ts:440-447
Timestamp: 2026-04-27T11:18:47.596Z
Learning: In `packages/opencode/test/agent/agent.test.ts` (Astro-Han/pawwork), all agent-permission tests intentionally use the manual `tmpdir()` + `Instance.provide(...)` pattern. Do not flag individual tests in this file for conversion to `provideTmpdirInstance(...)` or `provideInstance(...)`; a full harness migration would be a separate PR if the pattern ever needs to change.
Applied to files:
packages/desktop-electron/electron-builder-app-update.test.tspackages/desktop-electron/scripts/prepare-officecli.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use the `init` option in `tmpdir` to define custom setup functions that can return extra data accessible via `tmp.extra`, and use the `dispose` option for custom cleanup logic.
Applied to files:
packages/desktop-electron/electron-builder-app-update.test.tspackages/opencode/test/tool/bash-env-source.test.tspackages/desktop-electron/scripts/release-workflow-contract.test.ts
📚 Learning: 2026-04-23T08:51:04.230Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 186
File: packages/opencode/test/plugin/workspace-adaptor.test.ts:139-144
Timestamp: 2026-04-23T08:51:04.230Z
Learning: In Astro-Han/pawwork (`packages/opencode/src/util/filesystem.ts`), the `Filesystem` utility does NOT expose a `remove` or `unlink` helper. The established repository pattern for auth.json teardown in tests (e.g. `provider.test.ts`, `amazon-bedrock.test.ts`, `workspace-adaptor.test.ts`) is to combine `Filesystem.write` with `node:fs/promises unlink`. Do not flag this mixed usage as inconsistent — it is the correct and intentional pattern.
Applied to files:
packages/desktop-electron/electron-builder-app-update.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Use the `tmpdir` function from `fixture/fixture.ts` to create temporary directories for tests with automatic cleanup. Use `await using` syntax to ensure automatic cleanup when the variable goes out of scope.
Applied to files:
packages/desktop-electron/electron-builder-app-update.test.ts
📚 Learning: 2026-04-29T10:34:22.399Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 327
File: packages/opencode/test/pty/pty-session.test.ts:124-136
Timestamp: 2026-04-29T10:34:22.399Z
Learning: In `packages/opencode/test/pty/pty-session.test.ts` (Astro-Han/pawwork, PR `#327`), the `bun-pty` backend re-merges the parent process environment internally after `withoutInternalServerAuthEnv` strips auth values. This means `OPENCODE_SERVER_USERNAME` and `OPENCODE_SERVER_PASSWORD` will be **present-but-empty** (not fully unset) inside the PTY session. Using a `-unset` shell default expansion (e.g. `${OPENCODE_SERVER_USERNAME-unset}`) would never trigger, so asserting `username=unset` / `password=unset` would always fail. The correct assertion strategy is to check `expect(text).not.toContain("secret")` and `expect(text).not.toContain("PawWork")`. Do NOT suggest converting to `-unset` expansion or asserting `username=unset` / `password=unset` for PTY auth-env tests in this file.
Applied to files:
packages/opencode/src/tool/bash.tspackages/opencode/test/tool/bash-env-source.test.tspackages/opencode/test/github/desktop-smoke-workflow.test.ts
📚 Learning: 2026-04-27T08:58:00.665Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/src/session/prompt.ts:531-538
Timestamp: 2026-04-27T08:58:00.665Z
Learning: When using Effect (e.g., `yield*` with `Effect`-style generator yielding), only use `yield* new SomeErrorClass(...)` if `SomeErrorClass` extends `Schema.TaggedErrorClass` (i.e., it implements Effect’s Yieldable interface). For plain `Error` subclasses (like `BlockedLoopError` / `LoopStopError`) or inline `new Error(...)` values, they are not yieldable and must be wrapped as `yield* Effect.fail(new PlainError(...))`. Do not recommend changing `yield* Effect.fail(new SomePlainError(...))` to `yield* new SomePlainError(...)` unless the error class extends `Schema.TaggedErrorClass`.
Applied to files:
packages/opencode/src/tool/bash.ts
📚 Learning: 2026-04-28T12:49:09.792Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/src/session/subagent-run.ts:445-445
Timestamp: 2026-04-28T12:49:09.792Z
Learning: In this repo’s `packages/opencode/src` module files, the canonical module-as-namespace pattern is load-bearing: at the bottom of each module file, add a self-reexport in the form `export * as <ModuleName> from "./<module-file>"` (e.g., in `subagent-run.ts`: `export * as SubagentRun from "./subagent-run"`). Consumers must import the namespace via a named binding (e.g., `import { SubagentRun } from ".../subagent-run"`) and then access members as `SubagentRun.Service`, `SubagentRun.defaultLayer`, `SubagentRun.AgentListFilter`, etc. During code review, do NOT flag these self-reexports as dead code or redundant; removing or altering them will break the consumer named binding.
Applied to files:
packages/opencode/src/tool/bash.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Prefer Effect-aware helpers from `fixture/fixture.ts` over building manual runtimes in tests: use `tmpdirScoped()` for scoped temp directories, `provideInstance(dir)(effect)` for low-level binding without directory creation, `provideTmpdirInstance(...)` for single temp instance binding, or `provideTmpdirServer(...)` for tests that also need the test LLM server.
Applied to files:
packages/opencode/test/tool/bash-env-source.test.tspackages/desktop-electron/scripts/prepare-officecli.test.ts
📚 Learning: 2026-04-23T08:51:00.819Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 186
File: packages/opencode/test/plugin/workspace-adaptor.test.ts:139-144
Timestamp: 2026-04-23T08:51:00.819Z
Learning: For pawwork tests under packages/opencode/test/**, auth.json teardown may intentionally combine `Filesystem.write` (from `packages/opencode/src/util/filesystem.ts`) with `node:fs/promises` `unlink` for cleanup. Do not flag this as inconsistent style; it is the established/intentional pattern because `Filesystem` does not provide a `remove`/`unlink` helper.
Applied to files:
packages/opencode/test/tool/bash-env-source.test.tspackages/opencode/test/github/desktop-smoke-workflow.test.ts
📚 Learning: 2026-04-28T05:36:18.200Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:311-328
Timestamp: 2026-04-28T05:36:18.200Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the ripgrep binary download flow (`HttpClientRequest.get` → `fs.writeWithDirs` → `extract`) does NOT verify a pinned SHA-256 checksum. Adding per-platform hash verification (15.1.0 × 6 platforms) requires a dedicated manifest + `crypto.subtle.digest` step and is intentionally deferred to a follow-up PR. Existing mitigations: (1) `VERSION` is pinned to `"15.1.0"`, (2) downloads are over HTTPS from GitHub Releases, (3) PawWork production builds ship a bundled `rg` binary so this download path is a last-resort fallback (system PATH → cached → download). Do NOT re-flag the missing checksum verification in this file until the dedicated follow-up PR lands.
Applied to files:
packages/desktop-electron/scripts/prepare-officecli.test.ts.github/workflows/desktop-smoke.ymlpackages/desktop-electron/scripts/release-workflow-contract.test.tspackages/desktop-electron/scripts/prepare-officecli.ts
📚 Learning: 2026-04-28T11:24:35.312Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 287
File: packages/opencode/test/session/subagent-lifecycle-integration.test.ts:41-47
Timestamp: 2026-04-28T11:24:35.312Z
Learning: In `packages/opencode/test/session/subagent-lifecycle-integration.test.ts` (Astro-Han/pawwork, PR `#287`), the test suite intentionally uses a manual `run` helper (`Effect.runPromise(program.pipe(Effect.provide(Layer.mergeAll(SubagentRun.defaultLayer, Session.defaultLayer)), Effect.orDie))`) and raw `bun:test` test cases rather than the `testEffect(...)` harness. All 27 lifecycle tests pass with this pattern. Migration to `testEffect` + `it.live(...)` is deferred to a dedicated test-harness sweep PR to avoid fixture drift before merge. Do NOT re-flag the manual `Effect.runPromise` runner in this file as needing harness migration until that sweep PR lands.
Applied to files:
packages/desktop-electron/scripts/prepare-officecli.test.tspackages/desktop-electron/scripts/release-workflow-contract.test.tspackages/opencode/test/github/desktop-smoke-workflow.test.ts
📚 Learning: 2026-04-28T08:29:02.858Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 270
File: packages/opencode/src/file/ripgrep.ts:469-470
Timestamp: 2026-04-28T08:29:02.858Z
Learning: In `packages/opencode/src/file/ripgrep.ts` (Astro-Han/pawwork, PR `#270`), the `tree()` function filters files with `if (file.includes(".opencode")) continue`, which is a raw substring match that would also drop unrelated paths whose names merely contain `.opencode` (e.g., `.opencode-backup`). The correct fix is `if (file.split(path.sep).includes(".opencode")) continue` to match only the exact path segment. However, upstream `dev:packages/opencode/src/file/ripgrep.ts` carries the identical false-positive filter. PR `#270` is an upstream-sync graft; fixing it here would diverge from the baseline. The fix is deferred to a PawWork ripgrep filter cleanup PR or an upstream-first fix, to land alongside the other ripgrep deferrals (Schema.Class, bytes-union, abort short-circuit, mixed-separator). Do NOT re-flag the `.opencode` substring filter in upstream-sync PRs.
Applied to files:
packages/desktop-electron/electron-builder.config.ts
📚 Learning: 2026-04-27T12:59:49.844Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 264
File: packages/opencode/test/session/prompt-effect.test.ts:0-0
Timestamp: 2026-04-27T12:59:49.844Z
Learning: In `packages/opencode/test/session/prompt-effect.test.ts` and `packages/opencode/src/session/diagnostics.ts` (PR `#264`), the recovery reminder copy differs between signature kinds: the input-repeat variant says "repeated the same tool input 3 times" (uses a literal count), while the target-repeat variant says "failed against the same target multiple times" (uses "multiple times" with no count). Assertions that check for injected reminder text in LLM inputs must accept both phrasings when a scenario produces both `input:` and `target:` signatures (e.g., `read` tool with a `filePath` parameter). Do NOT narrow the assertion to only the input-variant phrasing.
Applied to files:
packages/desktop-electron/scripts/release-workflow-contract.test.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : When validating routing, assert against canonical or resolved workspace slugs using shared helpers from `../actions` to account for Windows canonicalization
Applied to files:
packages/desktop-electron/scripts/release-workflow-contract.test.ts
📚 Learning: 2026-04-24T00:02:53.315Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 203
File: packages/app/e2e/sidebar/sidebar-session-links.spec.ts:34-55
Timestamp: 2026-04-24T00:02:53.315Z
Learning: In Astro-Han/pawwork E2E tests (`packages/app/e2e/**/*.spec.ts`), `project.trackDirectory()` and `project.trackSession()` cannot be called before `project.open()` — the `project` fixture throws until `open()` initializes its internal state. The correct pattern is: call `project.trackSession(sessionID)` from inside the `beforeGoto` callback (where state already exists), call `project.trackDirectory(directory)` and cross-workspace `project.trackSession(id, directory)` immediately after `project.open()` returns, and rely on explicit `finally` cleanup (e.g. `cleanupSession` / `cleanupTestProject`) for any resources created before `open()` that cannot yet be tracked via the fixture.
Applied to files:
packages/desktop-electron/scripts/release-workflow-contract.test.ts
📚 Learning: 2026-04-20T14:36:31.032Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/test/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:31.032Z
Learning: Applies to packages/opencode/test/**/*.test.{ts,tsx} : Define `const it = testEffect(...)` near the top of the test file and keep the test body inside `Effect.gen(function* () { ... })`. Yield services directly with `yield* MyService.Service` or `yield* MyTool`.
Applied to files:
packages/desktop-electron/scripts/release-workflow-contract.test.ts
📚 Learning: 2026-04-25T09:51:18.951Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 231
File: packages/desktop-electron/src/main/menu-template.ts:178-189
Timestamp: 2026-04-25T09:51:18.951Z
Learning: In Astro-Han/pawwork (`packages/desktop-electron/src/main/menu-template.ts`), the `buildMacosMenuTemplate` and `buildWindowsMenuTemplate` functions are intentionally kept as separate, literal per-platform templates rather than sharing extracted submenu helpers. The split exists to allow future per-platform divergence; the templates already differ structurally (File menu: macOS omits `quit` because it lives in the App menu, Windows places `quit` next to `close`; Window submenu differs; macOS has an App menu with no Windows equivalent; View differs on terminal accelerator `Ctrl+\`` vs `CmdOrCtrl+\``). Do NOT suggest extracting shared submenu builder helpers (e.g., `buildEditSubmenu`, `buildViewSubmenu`) as YAGNI — the literal duplication is intentional and easy to scan.
Applied to files:
packages/desktop-electron/scripts/prepare-officecli.ts
🪛 LanguageTool
THIRD_PARTY_NOTICES.md
[style] ~174-~174: ‘any and all’ might be wordy. Consider a shorter alternative.
Context: ...ge, computer failure or malfunction, or any and all other commercial damages or losse...
(EN_WORDINESS_PREMIUM_ANY_AND_ALL)
🔇 Additional comments (15)
README.md (1)
89-90: Clear third-party attribution update.Line 89 adds explicit OfficeCLI credit and license context in a user-facing place; this matches the release/compliance intent.
THIRD_PARTY_NOTICES.md (1)
1-216: Third-party notice content is complete and appropriately scoped.The file provides the expected OfficeCLI attribution plus full Apache-2.0 text, which is appropriate for packaged distribution notices.
README_CN.md (1)
89-90: Localized attribution looks good.Line 89 mirrors the English README credit in Chinese and keeps the licensing acknowledgment explicit.
packages/opencode/src/tool/bash.ts (1)
410-410: Good guardrail for deterministic bundled-tool behavior.Line 410 enforces
OFFICECLI_SKIP_UPDATE=1for shell-executed commands, which prevents in-place OfficeCLI self-updates during runs.packages/desktop-electron/electron-builder.config.ts (1)
67-70: Packaging rule is correct.Lines 67-70 correctly include
THIRD_PARTY_NOTICES.mdin app resources as a top-level bundled artifact.packages/desktop-electron/electron-builder-app-update.test.ts (1)
65-75: Useful regression test for packaging contract.This test directly protects the new
extraResourcesrequirement for third-party notices.packages/desktop-electron/bundled-tools.json (1)
1-12: Pinned bundled-tools manifest is well-structured.The OfficeCLI entry makes version intent explicit and provides target-specific asset mapping needed for reproducible release prep.
.github/workflows/build.yml (1)
302-316: Preparation step wiring is correct.Lines 302-316 add the right pre-packaging OfficeCLI preparation gate and target mapping for non-finalize builds.
packages/opencode/test/tool/bash-env-source.test.ts (1)
5-9: Good contract coverage for update-skip env wiring.This test correctly guards the source contract that disables OfficeCLI self-update.
packages/desktop-electron/scripts/release-workflow-contract.test.ts (2)
7-13:expectBeforehelper is clear and fit-for-purpose.The helper keeps ordering checks readable and reusable across workflow assertions.
44-53: OfficeCLI release-step ordering checks look solid.These assertions correctly lock the prepare step before both macOS and Windows packaging paths.
packages/opencode/test/github/desktop-smoke-workflow.test.ts (1)
55-57: Strong workflow contract assertions for OfficeCLI and notices.The new checks cover step existence, ordering, and runtime bundle expectations well.
Also applies to: 80-82, 92-95
.github/workflows/desktop-smoke.yml (1)
131-133: Workflow hardening looks correct.Preparing OfficeCLI before build and asserting both executable presence and third-party notices in the app bundle are good release-safety checks.
Also applies to: 196-207
packages/desktop-electron/scripts/prepare-officecli.test.ts (1)
13-56: Great helper-level test coverage for the OfficeCLI prep contract.These tests lock in the critical mapping, URL pinning, and checksum parsing behavior.
packages/desktop-electron/scripts/prepare-officecli.ts (1)
74-100: Checksum-first preparation flow is well implemented.The sequence (asset selection → SHA256SUMS lookup → hash verification → install) is robust and release-safe.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request integrates OfficeCLI into the PawWork project to handle Word, Excel, and PowerPoint files locally. Key changes include the addition of a third-party license notice, a new manifest for bundled tools, and a robust script for downloading and verifying OfficeCLI binaries with checksum and version checks. The CI workflows and build configurations have been updated to include these new steps, and the Bash tool environment now explicitly disables OfficeCLI self-updates. Feedback is provided regarding a potential crash on Linux platforms due to a missing platform check in the tool preparation script.
11 tasks
This was referenced Jun 1, 2026
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.
Summary
v1.0.63in a desktop bundled-tools manifest.SHA256SUMS, and remove the tracked macOS arm64 binary from git.OFFICECLI_SKIP_UPDATE=1for agent shell commands, and add OfficeCLI Apache-2.0 attribution throughTHIRD_PARTY_NOTICES.mdplus README credit.Why
Issue #106 found that PawWork tracked only a macOS arm64 OfficeCLI binary under
resources/tools, while release packaging copied that directory wholesale for every target. That made Windows and macOS x64 packages vulnerable to bundling the wrong binary. This PR makes OfficeCLI versioning explicit, target-specific, checksum verified, and credited as a bundled third-party tool.Residual risk: local
package,package:mac, andpackage:winscripts still callelectron-builderdirectly. The production release workflow and desktop smoke workflow now prepare OfficeCLI before packaging; local package-script tightening can be a follow-up if needed.Related Issue
Fixes #106. Follow-up #330 covers automatic OfficeCLI version bump PRs.
How To Verify
Screenshots or Recordings
Not applicable. No visible UI changes.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
New Features
Documentation
Chores