-
Notifications
You must be signed in to change notification settings - Fork 517
refactor(runtime-host)!: make the Runtime Host State Root self-contained #5429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
a7f0110
9a9624f
916b7a7
658b575
53d8fc1
c31c2b1
904e54c
aaba615
4b26939
041386e
1952cc6
34b01ad
6ae20f7
97f51f8
9cd21e3
a80f609
727ae6f
5649cd9
0fd7d28
5f4d27d
a728e9c
61809f8
128b5ab
f6dae5f
62d24ba
ad252ea
b4d0a29
9355aa6
62d0995
b3ba58b
d660de8
6c25fa8
9f43263
fa76b81
0799593
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,9 +66,20 @@ on: | |
| # `scripts/windows-package-source-closure.test.mjs` recomputes this set | ||
| # from the dist tests the steps run and fails on any difference in either | ||
| # direction. | ||
| - 'packages/cli/src/__tests__/runtime-host-root-upgrade.test.ts' | ||
| - 'packages/cli/src/runtime-host-cli-installation.ts' | ||
| - 'packages/cli/src/runtime-host-lifecycle-transaction.ts' | ||
| - 'packages/cli/src/runtime-host-managed-deployment.ts' | ||
| - 'packages/cli/src/runtime-host-package-deployment.ts' | ||
| - 'packages/cli/src/runtime-host-peer-artifact.ts' | ||
| - 'packages/cli/src/runtime-host-service-management-command.ts' | ||
| - 'packages/cli/src/runtime-host-setup-command.ts' | ||
| - 'packages/cli/src/runtime-host-update-command.ts' | ||
| - 'packages/cli/src/runtime-host-windows-task-launcher-artifact.ts' | ||
| - 'packages/core/src/diagnostic-log.ts' | ||
| - 'packages/runtime-host/src/__tests__/control-endpoint.test.ts' | ||
| - 'packages/runtime-host/src/__tests__/fixtures/endpoint-hygiene.ts' | ||
| - 'packages/runtime-host/src/__tests__/root-upgrade.test.ts' | ||
| - 'packages/runtime-host/src/__tests__/skill-catalog-protocol.test.ts' | ||
| - 'packages/runtime-host/src/__tests__/skill-catalog-repository.test.ts' | ||
| - 'packages/runtime-host/src/__tests__/skill-catalog-transaction.test.ts' | ||
|
|
@@ -90,6 +101,7 @@ on: | |
| - 'packages/runtime-host/src/peer-reachability/publisher.ts' | ||
| - 'packages/runtime-host/src/protocol/host-status.ts' | ||
| - 'packages/runtime-host/src/protocol/skill-catalog.ts' | ||
| - 'packages/runtime-host/src/root-upgrade.ts' | ||
| - 'packages/runtime-host/src/server/access-credential-store.ts' | ||
| - 'packages/runtime-host/src/server/host-resource-probe.ts' | ||
| - 'packages/runtime-host/src/server/skill-catalog-repository.ts' | ||
|
|
@@ -129,6 +141,7 @@ on: | |
| - 'packages/storage/src/session-bundle-file-service.ts' | ||
| - 'packages/storage/src/sqlite-long-term-memory-store.ts' | ||
| - 'packages/storage/src/stable-storage.ts' | ||
| - 'packages/storage/src/workspace-root.ts' | ||
| - '.github/workflows/windows-recovery.yml' | ||
| # Unfiltered on purpose: required_status_checks is `strict: false`, so a pull | ||
| # request goes green against a stale base and only the merged result proves | ||
|
|
@@ -248,6 +261,14 @@ jobs: | |
| packages/runtime/dist/__tests__/runtime-resume-crash.test.js ` | ||
| packages/runtime/dist/__tests__/runtime-continuation-crash.test.js | ||
|
|
||
| - name: Verify Runtime Host root format upgrade | ||
| shell: pwsh | ||
| run: | | ||
| node.exe --test --test-concurrency=1 ` | ||
| packages/storage/dist/__tests__/root-migration.test.js ` | ||
| packages/runtime-host/dist/__tests__/root-upgrade.test.js ` | ||
| packages/cli/dist/__tests__/runtime-host-root-upgrade.test.js | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] This step adds
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved by amending the policy in |
||
|
|
||
| - name: Verify Runtime Host owner-death recovery | ||
| shell: pwsh | ||
| run: | | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] These entries put the recovery lane back on ordinary merges, which
ci-workflow-policy.test.mjspins against on purpose.scripts/ci-workflow-policy.test.mjs:704derives the lane's executed workspaces from thepackages/*/dist/paths its steps run and asserts the set exactly:Line 269 of this workflow now runs
packages/cli/dist/__tests__/runtime-host-root-upgrade.test.js, so the set becomes['cli', 'runtime', 'runtime-host', 'storage']andtestfails. The filter additions here are the other half of the same move.The test states why it is pinned, and the reason is a cost argument rather than bookkeeping:
So this is a decision about which lane owns the upgrade suites, not a line to patch. Two coherent answers: keep the
cliupgrade suites intestwhere they already run on Linux and leave this lane to the three workspaces it was narrowed to; or argue that these particular suites need real Windows and change the policy — in which case the assertion and the comment above it have to move with it, carrying the new rationale. What cannot hold is the current state, where the workflow and the policy that guards it disagree.I have no view on which answer is right; it is a CI-cost tradeoff for whoever owns that lane.
简体中文
这些条目会让 recovery 通道重新在大多数合并上被触发,而
ci-workflow-policy.test.mjs正是为此把它钉死的。scripts/ci-workflow-policy.test.mjs:704从该通道步骤实际运行的packages/*/dist/路径推导出它执行的 workspace,并精确断言其集合为['runtime','runtime-host','storage']。本工作流第 269 行现在会运行packages/cli/dist/__tests__/runtime-host-root-upgrade.test.js,集合因此变成['cli','runtime','runtime-host','storage'],test随之失败;这里新增的路径过滤条目是同一动作的另一半。测试把钉死的理由写明了,而且是成本论证不是记账:这些套件是普通 TypeScript,
test每个 PR 都会在 Linux 上先跑先红;把它们的源码列进这条通道只买到"第二次、更慢的红",而且会让这条通道回到大多数合并上。所以这是"升级套件归哪条通道"的决定,不是补一行能了的事。两种自洽的答案:要么把 cli 的升级套件留在
test(它们本来就在那儿跑),这条通道维持被收窄后的三个 workspace;要么论证这些套件确实需要真实 Windows,那就连同断言与其上方的注释一起改,把新理由写进去。不能成立的是当前状态:工作流和守护它的策略互相矛盾。哪个答案对我没有立场 —— 这是那条通道的持有者要做的 CI 成本权衡。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved by moving the policy side in
62d24ba83: the assertion now admitscliand the comment above it carries the new rationale. These suites execute win32-branching production code — the task-launcher artifact gate and in-root lock semantics — that Linuxtestcannot reach; the 3/3 deterministic failure on real Windows reported below is the evidence they belong here. The lane's criterion is 'needs real Windows', not workspace bookkeeping.