fix(chat): preserve cwd when reloading sessions by atishpatel · Pull Request #376 · block/berd

atishpatel · 2026-10-05T17:59:15Z

Summary

Fix history reloads after compaction when the renderer has no workspace path. acpLoadSession previously substituted literal ~, which Goose rejects because cwd must be absolute.

Treat empty or whitespace-only directories as missing. Reuse the session's prepared directory, or recover its saved directory from the owning backend. Preserve valid paths unchanged.
Resolve the directory inside the existing session mutation queue. Bound metadata recovery to 60 seconds and ignore late responses without limiting history replay.
On timeout, detach only the request's connection before releasing the queue. Old timers cannot disconnect a replacement; stuck cleanup cannot block retry.
Stop abandoned mutations and history replays from changing prepared state, publishing configuration, or making follow-up calls. Ignore late notifications and retire pending permission prompts from detached connections.
Fail without inventing a directory when saved metadata is unavailable. Keep the existing transcript-preservation warning.

Risk: This changes the shared session-load path, permission lifecycle, and timeout handling for local and SSH chats. Transport cleanup no longer blocks timed-out mutations. It does not change compaction prompts, default models, or the Goose backend pin.

Laws: Reviewed LAWS/README.md, LAWS/CHAT.md, and LAWS/AGENTS.md. Session dispatch and configured provider/model requirements remain unchanged; no law changes are needed.

Related issue

Related to #326. This PR fixes omitted-directory reloads; explicit tilde paths and session creation/fork behavior remain outside its scope. No duplicate issue was created.

#371 did not introduce the reload fallback; that code already existed in #2. This is separate from the reasoning-effort rejection reported in aaif-goose/goose#12687.

Testing

No manual desktop retest of the updated code yet. A fresh Apple Silicon internal test app was built and verified from cba4652 for final testing. It is ad-hoc signed locally, not Developer ID signed or notarized.

Automated validation: just check; 8,022 public tests passed (1 skipped); 8,135 internal tests passed (1 skipped). The internal overlay assembly, frozen install, and type checks passed. All normal push hooks, including Rust checks and Clippy, passed under the repository-pinned Hermit toolchain. Regression tests cover stale permission decisions and late history replay after reconnection.

The reported UI warning was:

Couldn't verify the compacted conversation because refreshed history wasn't received. Your previous messages are still shown. Try reloading the session.

The backend diagnostic recorded session/load failing with:

cwd must be an absolute path

The actual requested directory was not recorded in that diagnostic.

Screenshot placeholder: Paste the reported compaction-warning screenshot here before review.

Generated with Codex

morgmart

🤖 Automated code review

REQUEST_CHANGES: two blocking session-recovery defects remain. Blank remote working directories bypass the new recovery path, and a metadata request that never completes can permanently block later work for that session. Supplied GitHub checks for the exact head SHA are complete and passing, but required checks still govern merge readiness.

Deterministic publication result: 2 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.

morgmart

🤖 Automated code review

REQUEST_CHANGES: metadata recovery is now timed out, but the timeout path can still block the per-session queue forever while awaiting connection invalidation. The prior blank-directory issue is fixed. Supplied GitHub checks for the exact head SHA include passing and pending checks; required checks still govern merge readiness.

Deterministic publication result: 1 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.

morgmart

🤖 Automated code review

REQUEST_CHANGES: releasing the queue before timed-out work settles introduces two lifecycle races. A later timeout from another session on the same backend can detach a healthy replacement connection, and late completion of an abandoned mutation can overwrite newer session state. The three prior findings are fixed. Supplied checks for the exact head SHA include successful and cancelled runs; required checks still govern merge readiness.

Deterministic publication result: 2 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.

Detach the ACP client before releasing a timed-out session mutation. Run generation-scoped transport cleanup without awaiting it so a stuck peer cannot hold the queue forever. Cover stalled local and SSH cleanup and late teardown preserving the replacement client. Signed-off-by: Atish Patel <atishpatel2012@gmail.com>

Reject late results and follow-up calls once a mutation times out or its connection generation is retired. Drop detached transport notifications so retries retain their prepared selection and published config.

morgmart

🤖 Automated code review

REQUEST_CHANGES: generation fencing now protects bounded mutations and stale session notifications, but two lifecycle paths remain unfenced. A detached transport can still surface permission requests, and an unbounded history replay can still commit a late response after its connection is replaced. The five prior findings are fixed. Supplied GitHub checks for the exact head SHA are complete and passing; required checks still govern merge readiness.

Deterministic publication result: 2 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.

morgmart

🤖 Automated code review

No new publishable findings. Two current concerns are suppressed because matching automation threads remain unresolved without substantive human replies, so approval fails closed and the recommendation is no publication/retry. Supplied GitHub checks for the exact head SHA include successful and in-progress checks; required checks still govern merge readiness.

Deterministic publication result: 0 blocking and 0 non-blocking inline finding(s) publishable; 2 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.

morgmart

🤖 Automated code review

APPROVE: no publishable findings remain. The prior session recovery, timeout, generation, replay, and permission-lifecycle issues are fixed in the current comparison. Supplied GitHub checks for the exact head SHA include successful and in-progress checks; required checks still govern merge readiness.

Deterministic publication result: 0 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.

Pending checks: 1 check(s) are not complete.

This approval reflects the completed code review only; merge readiness remains governed by the repository's required checks.

## Summary An attached local workspace such as `~/goose artifacts` can override the saved absolute directory after compaction. The agent backend rejects the literal `~`, so Berd keeps the old transcript and warns that refreshed history was not received. Resolve the local home prefix before any history load. Preserve the rest of the path, including filename spaces, and leave remote paths unchanged. Home resolution uses the existing timeout and connection guards; history replay remains unbounded. Add regression coverage for the attached artifacts workspace, local path sources, Windows home prefixes, remote backend ownership, resolution failures, timeouts, and detached connections. Session creation and forks are unchanged. Affected laws: `LAWS/CHAT.md` and `LAWS/AGENTS.md`. The fix preserves serialized session operations and provider/model requirements; no law changes are needed. ### Related issue Follow-up to #376, which recovers missing or blank directories but leaves explicit home-relative paths unchanged. ### Testing No manual testing. No visible UI changes. Generated with Goose

atishpatel marked this pull request as ready for review October 5, 2026 18:15

atishpatel requested a review from a team October 5, 2026 18:15

kalvinnchau previously approved these changes Oct 5, 2026

View reviewed changes

morgmart requested changes Oct 5, 2026

View reviewed changes

Comment thread src/shared/api/acpSessionRegistry.ts Outdated

Comment thread src/shared/api/acpSessionRegistry.ts Outdated

atishpatel dismissed kalvinnchau’s stale review via 595e2da October 5, 2026 19:20

morgmart requested changes Oct 5, 2026

View reviewed changes

Comment thread src/shared/api/acpSessionRegistry.ts

atishpatel enabled auto-merge (squash) October 5, 2026 20:54

atishpatel disabled auto-merge October 5, 2026 20:55

morgmart requested changes Oct 5, 2026

View reviewed changes

Comment thread src/shared/api/acpSessionRegistry.ts Outdated

Comment thread src/shared/api/acpSessionRegistry.ts Outdated

atishpatel added 6 commits October 5, 2026 17:23

fix(chat): preserve cwd when reloading sessions

75fbbae

fix(chat): recover blank session directories

8c7395e

fix(chat): bound session directory recovery

a69aebe

fix(chat): scope timeout cleanup to its connection

b70c349

fix(chat): fence abandoned session mutations

ab8af2c

Reject late results and follow-up calls once a mutation times out or its connection generation is retired. Drop detached transport notifications so retries retain their prepared selection and published config.

morgmart requested changes Oct 5, 2026

View reviewed changes

Comment thread src/shared/api/acpConnection.ts

Comment thread src/shared/api/acpSessionRegistry.ts Outdated

atishpatel force-pushed the atish/compaction-reload-cwd branch from f5ac253 to ab8af2c Compare October 5, 2026 22:48

atishpatel added 2 commits October 5, 2026 17:58

fix(chat): retire detached permission requests

e84d523

fix(chat): fence replay by connection generation

cba4652

morgmart reviewed Oct 5, 2026

View reviewed changes

morgmart approved these changes Oct 5, 2026

View reviewed changes

atishpatel merged commit ed5b8af into main Oct 6, 2026
10 checks passed

atishpatel deleted the atish/compaction-reload-cwd branch October 6, 2026 04:16

atishpatel mentioned this pull request Oct 6, 2026

fix(chat): resolve local home paths before history reload #379

Merged

Navigation Menu

Repository navigation

fix(chat): preserve cwd when reloading sessions - #376

fix(chat): preserve cwd when reloading sessions#376
atishpatel merged 8 commits into
mainfrom
atish/compaction-reload-cwd

atishpatel commented Oct 5, 2026 •

edited

Loading

Uh oh!

morgmart left a comment

Uh oh!

Uh oh!

Uh oh!

morgmart left a comment

Uh oh!

Uh oh!

morgmart left a comment

Uh oh!

Uh oh!

Uh oh!

morgmart left a comment

Uh oh!

Uh oh!

Uh oh!

morgmart left a comment

Uh oh!

morgmart left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

atishpatel commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related issue

Testing

Uh oh!

morgmart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

morgmart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

morgmart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

morgmart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

morgmart left a comment

Choose a reason for hiding this comment

Uh oh!

morgmart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

atishpatel commented Oct 5, 2026 •

edited

Loading