Skip to content

Commit ef7845a

Browse files
baozhoutaoclaude
andauthored
fix(runtime): publish-drafts 中途失败时,已落盘的可见性翻转不再从响应消失 (#5242) (#5392)
`POST /packages/:id/publish-drafts` 的 ADR-0045 可见性翻转是一个循环:每个 app 一次独立的 `saveMetaItem`,每次成功各自落盘。但 `unhidden` 声明在 try 之内、 `result.unhiddenApps` 只在整个循环跑完后才赋值 —— N 个 app 里第 k 个抛异常时, 前 k-1 个确实已翻转并持久化,却随栈一起被丢弃:响应里 `unhiddenApps` 不存在, 紧随其后读这个字段的 `metadata:reloaded` 广播也漏播这些 app,boot-cached 的 消费者(automation engine)要等下一次重启才同步。 改为增量累积:`unhidden` 与它的赋值一并提到 try 之外,名字只在对应 `saveMetaItem` 兑现之后才 push,因此该列表在任意时刻恰好等于"已经落盘的那些"; 赋值移到 try/catch 之后,成功与中途失败两条路径都执行,且仍在 announce 段之前。 部分失败时 `unhiddenApps` 与 `unhideError` 并存 —— 前者说什么翻成功了,后者说 还有没翻完的。`unhidden` 是每请求局部量,不引入共享可变状态(#5385 姿态)。 同时修正那条 error 日志的措辞:它原先断言"其 app 全部仍以 hidden: true 存着", 一旦有翻转已落盘这句话就是假的;现在按两半如实点名。 响应契约不变:仍然 200,字段仍是原来那两个,只是部分失败时可以同时出现; 重跑依旧幂等。 Claude-Session: https://claude.ai/code/session_01VkPSGsX9o17MsGv3Lbxu2w Co-authored-by: Claude <noreply@anthropic.com>
1 parent 3d94141 commit ef7845a

3 files changed

Lines changed: 130 additions & 3 deletions

File tree

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
fix(runtime): 可见性翻转中途失败时,已落盘的 app 不再从响应里整批消失 (#5242)
6+
7+
`POST /packages/:id/publish-drafts` 的 ADR-0045 可见性翻转是一个**循环**:每个 app 一次
8+
独立的 `saveMetaItem`,每次成功各自落盘。但 `unhidden` 数组声明在 `try` 之内、
9+
`result.unhiddenApps` 又只在整个循环跑完之后才赋值 —— 5 个 app 里第 3 个抛异常时,前 2 个
10+
**确实已经翻转并持久化**,却随栈一起被丢弃:响应里 `unhiddenApps` 压根不存在。
11+
12+
后果有两层,都指向同一个「机器可读面在撒谎」:
13+
14+
1. **响应少报了真实发生的事。** 调用方看到的是「翻转失败」,看不到「其中 2 个已经生效」。
15+
2. **`metadata:reloaded` 对这 2 个 app 漏播。** 紧随其后的重绑定段读的正是 `unhiddenApps`,
16+
字段缺失 → 这 2 个已经变可见的 app 不进 `changed` → boot-cached 的消费者(首当其冲是
17+
automation engine)不重新同步它们,要等下一次重启。
18+
19+
修法按 PM 裁定取**增量累积**而非预校验:`unhidden` 与它的赋值一并提到 `try` 之外,名字只在
20+
对应的 `saveMetaItem` **兑现之后**才 push,因此这个列表在任意时刻恰好等于「已经落盘的那些」。
21+
赋值移到 `try/catch` 之后,成功与中途失败两条路径都会执行,并且仍在 announce 段之前 ——
22+
部分失败时 `unhiddenApps``unhideError` **并存**:前者说什么翻成功了,后者说还有没翻完的。
23+
`unhidden` 是每请求的局部量,不引入任何共享可变状态,符合 #5385 确立的显式传参姿态。
24+
25+
同时修掉那条 `error` 日志的措辞:它原先断言「其 app **全部**仍以 `hidden: true` 存着」,
26+
一旦有翻转已落盘这句话就是假的。现在按两半如实点名 —— 哪些确实翻了(列出名字)、哪些仍然
27+
是隐藏的,以及一如既往的后果与修复动作。
28+
29+
响应契约不变:仍然 200,字段还是原来那两个,只是部分失败时它们可以同时出现;重跑依旧幂等
30+
(已翻转的 app `hidden !== true`,循环会跳过)。

packages/runtime/src/domains/packages.ts

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,19 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
200200
// caller never needs to know how the package was built).
201201
// Best-effort: a custom protocol without the meta
202202
// primitives keeps plain draft-publish semantics.
203+
//
204+
// #5242 — `unhidden` and its result assignment live OUTSIDE
205+
// this try. A name is pushed only AFTER its `saveMetaItem`
206+
// resolved, so at any moment the list is exactly "what is
207+
// already flipped on disk". When app k of N throws, the k-1
208+
// that DID persist are a fact the caller must be told about:
209+
// accumulating inside the try and assigning after the loop
210+
// discarded them with the stack, so the response claimed
211+
// nothing happened for apps that had already changed state,
212+
// and the 'metadata:reloaded' announce below — which reads
213+
// `unhiddenApps` — skipped them too, leaving boot-cached
214+
// consumers stale until the next restart.
215+
const unhidden: string[] = [];
203216
try {
204217
if (
205218
typeof (protocol as any).getMetaItems === 'function' &&
@@ -213,7 +226,6 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
213226
const apps: any[] = Array.isArray(appsRes)
214227
? appsRes
215228
: Array.isArray((appsRes as any)?.items) ? (appsRes as any).items : [];
216-
const unhidden: string[] = [];
217229
for (const app of apps) {
218230
if (app && typeof app === 'object' && app.hidden === true && typeof app.name === 'string') {
219231
await (protocol as any).saveMetaItem({
@@ -227,7 +239,6 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
227239
unhidden.push(app.name);
228240
}
229241
}
230-
if (unhidden.length > 0) (result as any).unhiddenApps = unhidden;
231242
}
232243
} catch (e: any) {
233244
// #4754 — ADR-0045's visibility flip is a metadata WRITE
@@ -240,16 +251,33 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
240251
// there". So it is reported at `error` (AGENTS.md →
241252
// "Degradation log levels"), not swallowed.
242253
const logger = deps.logger ?? console;
254+
// #5242 — a mid-loop failure leaves the package SPLIT: the
255+
// apps already saved are visible, the rest are not. Name
256+
// BOTH halves. The old wording asserted "every hidden app
257+
// is still stored hidden", which is plainly false once any
258+
// flip persisted, and it left the operator to infer
259+
// "nothing changed" from a bare failure line.
260+
const stillHidden = unhidden.length > 0
261+
? `the flip stopped PARTWAY — ${unhidden.length} app(s) DID flip and are stored visible ` +
262+
`(${unhidden.join(', ')}; they are reported under \`unhiddenApps\` and were announced for ` +
263+
`re-sync), while every REMAINING hidden app bound to it`
264+
: `every hidden app bound to it`;
243265
logger.error(
244266
`[Packages] publish-drafts: the ADR-0045 visibility flip FAILED for package '${id}' — its drafts ARE ` +
245-
`published and live, but every hidden app bound to it is still STORED with \`hidden: true\`, so those ` +
267+
`published and live, but ${stillHidden} is still STORED with \`hidden: true\`, so those ` +
246268
`apps stay invisible in the launcher while the publish reports success. Nothing retries this flip. ` +
247269
`Re-run POST /packages/${id}/publish-drafts once the cause below is resolved (it is idempotent), or ` +
248270
`unhide one app directly via PUT /meta/app/<name> with \`{"hidden": false}\`. Cause: ` +
249271
`${e?.message ?? String(e)}`,
250272
);
251273
(result as any).unhideError = e?.message ?? 'visibility flip failed';
252274
}
275+
// Assigned on BOTH paths — clean completion and mid-loop
276+
// failure alike. On the failure path it rides ALONGSIDE
277+
// `unhideError`: together they say what did flip and that
278+
// something did not, which is the honest report. It must
279+
// stay ABOVE the announce block, which reads this field.
280+
if (unhidden.length > 0) (result as any).unhiddenApps = unhidden;
253281
// A publish promoted drafts to active (or unhid an additive
254282
// app) at RUNTIME — but boot-cached consumers still hold the
255283
// pre-publish view. The load-bearing one is the automation

packages/runtime/src/http-dispatcher.test.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1921,6 +1921,75 @@ describe('HttpDispatcher', () => {
19211921
errorSpy.mockRestore();
19221922
}
19231923
});
1924+
1925+
// #5242 — the flip is a LOOP of independent writes, and each one that
1926+
// resolves is durable on its own. When app k of N throws, the k-1 that
1927+
// already persisted ARE visible on disk; a response that omits them
1928+
// tells the caller nothing happened for apps whose state DID change,
1929+
// and the 'metadata:reloaded' announce (which reads `unhiddenApps`)
1930+
// then skips exactly those apps, leaving boot-cached consumers stale.
1931+
it('POST /packages/:id/publish-drafts reports the apps already unhidden when the flip fails MID-LOOP', async () => {
1932+
const publishPackageDrafts = vi.fn().mockResolvedValue({
1933+
success: true, publishedCount: 0, failedCount: 0, published: [], failed: [], seedApplied: { success: true },
1934+
});
1935+
// 4 hidden apps; the write for the 3rd rejects. So `alpha` and
1936+
// `beta` are persisted visible, `gamma` and `delta` are not.
1937+
const getMetaItems = vi.fn().mockResolvedValue([
1938+
{ name: 'alpha', hidden: true, navigation: [] },
1939+
{ name: 'beta', hidden: true, navigation: [] },
1940+
{ name: 'gamma', hidden: true, navigation: [] },
1941+
{ name: 'delta', hidden: true, navigation: [] },
1942+
]);
1943+
const saveMetaItem = vi.fn().mockImplementation(async ({ name }: { name: string }) => {
1944+
if (name === 'gamma') throw new Error('sys_metadata write rejected');
1945+
return { ok: true };
1946+
});
1947+
(kernel as any).getService = vi.fn().mockImplementation((name: string) => {
1948+
if (name === 'protocol') return Promise.resolve({ publishPackageDrafts, getMetaItems, saveMetaItem });
1949+
if (name === 'objectql') return Promise.resolve({ registry: { getAllPackages: vi.fn().mockReturnValue([]) } });
1950+
return null;
1951+
});
1952+
const trigger = vi.fn().mockResolvedValue(undefined);
1953+
(kernel as any).context.trigger = trigger;
1954+
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
1955+
1956+
try {
1957+
const result = await dispatcher.handlePackages('/app.partial/publish-drafts', 'POST', {}, {}, { request: {} });
1958+
1959+
// The loop stopped at `gamma` — `delta` was never attempted.
1960+
expect(result.response?.status).toBe(200);
1961+
expect(saveMetaItem).toHaveBeenCalledTimes(3);
1962+
expect(saveMetaItem).not.toHaveBeenCalledWith(expect.objectContaining({ name: 'delta' }));
1963+
1964+
const data = (result.response as any)?.body?.data;
1965+
// The two flips that DID persist are reported, not discarded
1966+
// with the stack — and the failure is reported alongside them,
1967+
// so the body names what flipped AND that something did not.
1968+
expect(data?.unhiddenApps).toEqual(['alpha', 'beta']);
1969+
expect(data?.unhideError).toBe('sys_metadata write rejected');
1970+
1971+
// ...and the same two reach the re-sync broadcast, so a
1972+
// boot-cached consumer picks up the apps that really changed
1973+
// instead of waiting for a restart.
1974+
expect(trigger).toHaveBeenCalledWith(
1975+
'metadata:reloaded',
1976+
expect.objectContaining({ changed: ['app/alpha', 'app/beta'] }),
1977+
);
1978+
1979+
// The operator-facing line names BOTH halves: what flipped and
1980+
// what is still stored hidden. The old wording claimed "every
1981+
// hidden app is still stored hidden", which is false here.
1982+
const line = errorSpy.mock.calls
1983+
.map((c) => String(c?.[0] ?? ''))
1984+
.find((l) => l.includes('[Packages] publish-drafts')) ?? '';
1985+
expect(line).toContain('alpha, beta');
1986+
expect(line).toMatch(/PARTWAY/);
1987+
expect(line).toMatch(/REMAINING hidden app/);
1988+
expect(line).toContain('sys_metadata write rejected');
1989+
} finally {
1990+
errorSpy.mockRestore();
1991+
}
1992+
});
19241993
});
19251994

19261995
// ═══════════════════════════════════════════════════════════════

0 commit comments

Comments
 (0)