Uh oh!
There was an error while loading. Please reload this page.
fix(runtime-host): keep the preferred location when registering a project path - #3573
Conversation
062195b to
f896059Compare
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
2b63968Compare
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
691ab3dCompareI 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 覆盖,且是绿的。
现在合并。
Uh oh!
There was an error while loading. Please reload this page.
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?