fix(config): gate Instance dispose on actual config change by Astro-Han · Pull Request #722 · Astro-Han/pawwork

Astro-Han · 2026-05-18T04:40:16Z

Summary

Port upstream PR anomalyco/opencode#25114's changed byte-comparison gate to both Config.update and Config.updateGlobal so that a no-op config write does not tear down running instances.

Why

Without the gate, both Config.update (project config) and Config.updateGlobal (global config) call Instance.dispose() / Instance.disposeAll() on every write. A write of identical bytes — e.g., a UI mount that re-emits the existing setting — still disposes every running instance, which closes each SessionRunState scope mid-turn. The user sees the active assistant turn end with MessageAbortedError and an Tool execution aborted tool result, then has to resend.

Upstream already fixed this same class of bug in opencode commit a9d399699 ("Prevent Model response Interruption when opening settings dialog"), but the fix is not in PawWork's dev history (git branch -a --contains a9d399699 returns only remotes/upstream/dev). PawWork's Config.update and Config.updateGlobal are still in the pre-fix shape.

This PR ports just the changed gate, not the broader #25475 instance-lifecycle decoupling refactor. The minimal change is enough to remove the no-op-write trigger.

Related Issue

Candidate mitigation for #721. Deliberately not phrased with a closing keyword because we have not yet confirmed via post-#710 abort diagnostics that the silent writer is what's tearing down scopes in the reported cases. This PR removes one known trigger of that class. The #721 diagnostic comments lay out what evidence would confirm or refute that this was the root cause.

Human Review Status

Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.

Review Focus

The !fileExisted clause in updateGlobal. PawWork's updateGlobal has a seed-migration path: on a fresh install with no global config file, it seeds from upstream config sources. If we gated purely on byte equality, the seeded content would never materialize because serialized === before on first run. The !fileExisted guard makes sure we always write on first run even when the merged result equals the seed text. Worth confirming this is correct for the existing seed test cases.

Risk Notes

Low behavior risk. The change only suppresses dispose when the write would have produced identical bytes. Any write that actually changes config still disposes as before. No public API change. No platform-specific concern.

How To Verify

Typecheck: bun --cwd packages/opencode run typecheck — pass
Targeted tests: bun --cwd packages/opencode test test/config/pawwork-global-config.test.ts test/config/config.test.ts — 153 pass, 0 fail
Diff check: git diff --check — clean

Screenshots or Recordings

Not applicable. No visible UI change.

Checklist

coderabbitai · 2026-05-18T04:40:22Z

Warning

Rate limit exceeded

@Astro-Han has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 59 minutes and 16 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b66abb3f-2952-410d-b96b-97c9e4b73213

📥 Commits

Reviewing files that changed from the base of the PR and between d5e6186 and ca2e368.

📒 Files selected for processing (2)

packages/opencode/src/config/config.ts
packages/opencode/test/config/pawwork-global-config.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch claude/config-changed-gate

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions

Suggested priority: P2 (includes non-doc, non-test paths outside the low-risk bucket).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

gemini-code-assist

Code Review

This pull request optimizes configuration updates in packages/opencode/src/config/config.ts by ensuring that disk writes and instance teardowns only occur when the configuration content has actually changed. By comparing the new configuration against the existing file content, the code avoids unnecessary interruptions to active processes caused by no-op writes. I have no feedback to provide as there were no review comments.

Port upstream PR #25114's `changed` byte-comparison gate to both `Config.update` and `Config.updateGlobal`. Without it, a no-op global or project config write tears down all running instances via `Instance.disposeAll()`, which closes each SessionRunState scope mid- turn and surfaces to the user as `MessageAbortedError` with a tool showing `Tool execution aborted`. PawWork's existing `update` path always called `Instance.dispose()` unconditionally, and `updateGlobal` always called `invalidate()` (which calls `Instance.disposeAll()`) unconditionally. Upstream already fixed the same class of bug in opencode commit `a9d399699` ("Prevent Model response Interruption when opening settings dialog"), but the fix was not in PawWork's `dev` history. This is a candidate fix for #721. It does not change behavior when the write genuinely changes bytes on disk. The first global write on a fresh install still materializes the seeded file because the gate also keys on `!fileExisted`. Verification: bun --cwd packages/opencode run typecheck passes, bun --cwd packages/opencode test test/config/pawwork-global-config.test.ts test/config/config.test.ts passes 153/153.

Sentinel-mtime regression for the changed gate added in the previous commit. If the gate ever regresses, the second saveGlobal would rewrite the file and bump mtime past the sentinel, failing this test. Reviewer-requested guard so we explicitly assert the behavior rather than rely on the existing 153 config tests covering only functional correctness.

Astro-Han · 2026-05-18T05:17:35Z

@coderabbitai review

coderabbitai · 2026-05-18T05:17:40Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Astro-Han · 2026-05-18T05:32:59Z

On the P2 follow-up around cache potentially lagging disk after a no-op Config.update / Config.updateGlobal: I'd push back and not open a follow-up, for three reasons.

1. The premise does not hold in PawWork. PawWork is a single-instance Electron app (packages/desktop-electron/src/main/index.ts:334 calls app.requestSingleInstanceLock()), so a second process writing pawwork.json concurrently does not happen in the desktop product. The only remaining "external write" path is a user manually editing pawwork.json, and the race only triggers if the UI then writes an input whose serialized bytes happen to be identical to what the user just wrote. Three coincident conditions, with a recovery path of restarting PawWork.

2. Upstream evaluated the same trade-off and did not split. The fix we are porting (opencode commit a9d399699, "Prevent Model response Interruption when opening settings dialog") ends Config.updateGlobal with if (changed) yield* invalidate() — no separate "refresh cachedGlobal but skip Instance dispose" branch. The larger upstream PR #25475 (fix(instance): run bootstrap from instance store, 41 files, Fixes #25450) addresses a different bug (plugin-registered agents lost on reload) and leaves cachedGlobal's Duration.infinity TTL behavior unchanged, so this same race persists after #25475.

3. Splitting cache refresh from instance dispose is not a small change. cachedGlobal is built via Effect.cachedInvalidateWithTTL(loadGlobal(), Duration.infinity) and invalidate() currently shoulders both "drop the in-memory copy" and "tear down running instances." Decoupling them — refreshing the cache without aborting in-flight turns — would require reshaping the cache write path, approaching the scope of #25475-style refactors, beyond this PR's deliberate "remove one known trigger" boundary.

Plan: keep #722 at its current minimal scope and merge. No follow-up issue. If PawWork later gains a multi-process write path, or if production logs show this race actually firing, we revisit then.

Astro-Han added bug Something isn't working harness Model harness, prompts, tool descriptions, and session mechanics P2 Medium priority labels May 18, 2026

github-actions Bot reviewed May 18, 2026

View reviewed changes

gemini-code-assist Bot reviewed May 18, 2026

View reviewed changes

Astro-Han added 2 commits May 18, 2026 13:17

Astro-Han force-pushed the claude/config-changed-gate branch from 1a476b6 to ca2e368 Compare May 18, 2026 05:18

Astro-Han merged commit 0e64c23 into dev May 18, 2026
27 checks passed

Astro-Han deleted the claude/config-changed-gate branch May 18, 2026 05:34

This was referenced May 18, 2026

[Bug] First-turn tool execution aborted without recorded source — model stops, user must resend #721

Closed

chore(release): bump desktop version to v2026.5.19 #742

Merged

[Task] Add LLM stream failure diagnostics v2 to session exports #760

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(config): gate Instance dispose on actual config change#722

fix(config): gate Instance dispose on actual config change#722
Astro-Han merged 2 commits into
devfrom
claude/config-changed-gate

Astro-Han commented May 18, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented May 18, 2026 •

edited

Loading

Rate limit exceeded

Uh oh!

github-actions Bot left a comment

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Astro-Han commented May 18, 2026

Uh oh!

coderabbitai Bot commented May 18, 2026

Uh oh!

Astro-Han commented May 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Astro-Han commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Related Issue

Human Review Status

Review Focus

Risk Notes

How To Verify

Screenshots or Recordings

Checklist

Uh oh!

coderabbitai Bot commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Astro-Han commented May 18, 2026

Uh oh!

coderabbitai Bot commented May 18, 2026

Uh oh!

Astro-Han commented May 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Astro-Han commented May 18, 2026 •

edited

Loading

coderabbitai Bot commented May 18, 2026 •

edited

Loading