fix(runtime-host): keep the preferred location when registering a project path - #3573
Conversation
062195b to
f896059
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed exact head f896059. Coverage: storage registration preference semantics across existing/new linked-worktree locations and later touch; Runtime Host protocol/coordinator defaulting and strict boolean decoding; CLI default and explicit --prefer transport; existing Desktop callers retaining prefer-by-default behavior; catalog ordering, path normalization, and transactional mutation paths. Exclusions: real multi-process CLI/Desktop smoke testing and visual/UI behavior. Local Core→Storage→Runtime Host→CLI builds passed, with 78 focused tests green; hosted exact-head test is green. No P0–P3 findings. The branch currently conflicts with main only in packages/runtime-host/src/protocol/index.ts; resolution should preserve both compatibility additions/comment blocks and advance from current main epoch 43 rather than choosing the PR epoch 42 verbatim.
f896059 to
2b63968
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Incremental re-review at exact head 2b639688040c317eef75b1d96a1e01be07fc3b4c. APPROVE — no P0–P3.
This confirms the earlier approval, which was bound to f896059b. I did not re-review the whole change; I verified that the only difference since that approval is the epoch realignment, and that it is correct.
What changed since f896059b
Comparing each file's patch content against its own merge-base (efddab2f then, 84ed9a31 now), 11 of the 12 touched files have byte-identical patches. The single difference is in packages/runtime-host/src/protocol/index.ts:
RUNTIME_HOST_COMPATIBILITY_EPOCHmoved from42to45- the ledger comment was re-anchored from "Epoch-41 Hosts" to "Epoch-44 hosts", and the intervening
44/43entries frommainwere kept
Why 45 is the right value
Current main (04836d3b8) is at 44. 45 is the next value, so the monotonic-progression rule holds and no Host that speaks 44 will silently accept the new optional location field on the closed registration input.
One merge-order caveat for maintainers, not a defect in this PR: #3467, #3316, #3376, #3651 and #2521 are also currently claiming 45. Whichever merges first advances main; the rest will need to re-bump. That is inherent to a single global counter, not something this author did wrong.
Gates at this exact head
test— terminalsuccessmergeable—MERGEABLE/CLEAN- unresolved review threads — 0
- the check set is the correct triggered set for these paths
中文
在 exact head 2b639688 上做的增量复审,结论 APPROVE,无 P0–P3。原批准绑在 f896059b,本次不重审全量,只核"自那次批准以来变了什么"。
各文件对各自 merge-base 的 patch 逐一比对:12 个文件里 11 个逐字节相同,唯一变化是 protocol/index.ts 的 epoch 42→45 与注释重排。main 现在是 44,45 单调正确。
提醒(非本 PR 缺陷):#3467、#3316、#3376、#3651、#2521 目前都在争 45,谁先合谁把 main 推上去,其余需要重新递增。这是全局单调计数器本身的性质。
门禁:test 终态绿、CLEAN、零未决线程。
…ject path Project preference is derived from each location's last-used timestamp, so plain registration was also selecting that checkout. Add an explicit registration preference while keeping omission prefer-on for Desktop folder opens. The runtime-host project add CLI now registers without usage by default and exposes --prefer. Compatibility epoch 40 fences the added field on the closed request shape. Generated-by: pi (gpt-5.6-sol)
2b63968 to
691ab3d
Compare
|
I rebased this branch onto the latest main for you. The new head is The conflict was the protocol epoch: main has already moved Local verification: runtime-host and storage typecheck cleanly, and all 78 protocol / project-catalog tests pass. Once CI is green on the new head I will re-review and move this along. 简体中文我帮这个分支 rebase 到了最新的 main(新 head 冲突点是协议 epoch:main 上已经把 本地验证:runtime-host 与 storage 类型检查通过,protocol / project-catalog 相关 78 项测试全过。等新 head 的 CI 跑绿后我再重新 review 并推进合并。 |
Astro-Han
left a comment
There was a problem hiding this comment.
Approving at 691ab3de4. CI is terminal green on this exact head (run 32708235015), including the Release contracts step that failed on the previous attempt — that one was flaky, not caused by this branch.
Since I rebased this branch myself, I re-checked the result rather than relying on the earlier approvals, which were bound to f896059b and 2b639688 and no longer describe this code.
What I verified:
- Epoch lands correctly.
mainpublishes 46 and this head publishes 47, so the bump is sequential over the current base. One other open PR also carries 47; whichever merges second will need 48, and that is a coordination matter rather than anything wrong here. - No persisted format change.
ProjectRegistrationOptions.preferis an in-memory option;PersistedProjectis untouched. An existing catalog file loads unchanged, so there is no upgrade path to worry about. - Merged against current
main, not just the PR head. The merge is clean, and on the merged treepackages/storageproject-catalog tests pass 22/22 and theruntime-hostproject-catalog coordinator, project-catalog protocol, and bootstrap protocol tests pass 56/56. - The
maka-agentbuild fails in my local checkout, but byte-identically on a cleanmaincheckout in the same environment (@maka/evalunbuilt, api-tuiversion mismatch). That is stale local workspace state, not attributable to this change; hosted CI covers those tests and is green.
Merging now.
简体中文
在 691ab3de4 上给出 approve。这个 exact head 的 CI 是终态绿(运行记录 32708235015),其中包括上一次失败的 Release contracts 步骤——那次是偶发,不是这个分支造成的。
由于这个分支是我自己 rebase 的,我重新核对了结果,而没有沿用之前的 approve:它们绑在 f896059b 和 2b639688 上,已经不描述当前代码了。
我核对了这些:
- epoch 落点正确。
main发布的是 46,这个 head 发布的是 47,因此这次提升在当前基线上是连续的。另有一个开放 PR 也占用了 47,后合入的那个需要改成 48;这属于协调事项,不是这里有问题。 - 没有持久化格式变更。
ProjectRegistrationOptions.prefer是内存中的选项,PersistedProject未被改动。既有的 catalog 文件可以原样加载,因此不存在需要考虑的升级路径。 - 是对当前
main做了合并验证,而不只是看 PR head。 合并干净,并且在合并后的树上,packages/storage的 project-catalog 测试 22/22 通过,runtime-host的 project-catalog coordinator、project-catalog protocol 和 bootstrap protocol 测试 56/56 通过。 maka-agent在我本地检出上构建失败,但在同一环境下干净main检出上的报错逐字节一致(@maka/eval未构建、pi-tui版本不匹配)。这是本地 workspace 状态过期,与本次改动无关;这部分测试由托管 CI 覆盖,且是绿的。
现在合并。
Summary
Project preference is derived from location usage (newest
lastUsedAtamong available locations), and registration wrote the registration timestamp into the location it touched. Soruntime-host project add <second checkout>also selected that checkout, and every later project-targeted Session started there.This adds an optional
preferfield to theproject.registerrequest. Omitted ortruekeeps today's behavior;falseadds or refreshes a location without recording usage (a new location is stored withlastUsedAt: 0, an existing one keeps its timestamp). A new project still establishes its sole location as the initial preference.The CLI
runtime-host project addnow sendsprefer: falseby default and gains--preferfor callers who mean to select the path. Desktop is unchanged: its registration call sites omit the field, so the folder-open flow (register, then create a project-targeted Session in that folder) behaves as before.Fixes #3572
Review focus
Opened as a draft for a design check before review effort goes in. Two decisions here are yours to make, and I would rather adjust than argue:
preferon the existingproject.registerrequest, soproject addstops selecting by default and--preferopts in. The alternative is to keep registration as it is and add a separate "use this location" operation (aproject use <path>verb), which matches howdocker context useorkubectl config use-contextseparate creating from selecting. Both need the same epoch bump; I picked the smaller diff.Compatibility epoch
Bumped from 41 to 42. The registration request is a closed shape, so an epoch-41 Host rejects the new field and a new CLI always sends it. Precedent:
8346fbb92bumped 36 to 37 when an optionaltextfield was added to another exact catalog request.Verification
New regression tests, all red before the change and green after:
project-catalog.test.ts: a second location registered withprefer: falseleavespreferredPathunchanged; re-registering the non-preferred location withprefer: falseleaves it unchanged; registration with the option omitted still selects the path; a latertouchon the new location makes it preferred; a new project still gets its sole location as preference.project-catalog-protocol.test.tsandproject-catalog-coordinator.test.ts:project.registeracceptsprefer: falseand the returned record keepspreferredPath; a non-booleanpreferis rejected;protocol.test.tspins epoch 42.runtime-host-operator-command.test.ts:project addsendsprefer: falseby default andprefer: truewith--prefer.Suites, on Node 24.19.0 (the version main CI uses):
Typecheck passed in the three workspaces.
biome lintandbiome formatreported no fixes on the 12 changed files;git diff --checkclean.Live, fresh Host roots, worktree named
aaa-worktreeso it sorts beforerepository:Not run: full monorepo
npm test, root-wide typecheck/lint/format, Desktop suites, Windows or Linux.AI use
Select exactly one:
Tool(s) and scope: pi (gpt-5.6-sol) traced the storage, protocol, Desktop and Session-creation paths, wrote the implementation and the regression tests, ran the live and compatibility checks, and drafted this description. Claude Code chose the design, reviewed the diff, re-ran the three suites, and edited this text. The commit carries the trailer.
Generated-by: pi (gpt-5.6-sol)
Checklist
Does this PR entail a change in behavior?