diff --git a/.changeset/publish-drafts-partial-unhide.md b/.changeset/publish-drafts-partial-unhide.md new file mode 100644 index 0000000000..bf852fd72d --- /dev/null +++ b/.changeset/publish-drafts-partial-unhide.md @@ -0,0 +1,30 @@ +--- +"@objectstack/runtime": patch +--- + +fix(runtime): 可见性翻转中途失败时,已落盘的 app 不再从响应里整批消失 (#5242) + +`POST /packages/:id/publish-drafts` 的 ADR-0045 可见性翻转是一个**循环**:每个 app 一次 +独立的 `saveMetaItem`,每次成功各自落盘。但 `unhidden` 数组声明在 `try` 之内、 +`result.unhiddenApps` 又只在整个循环跑完之后才赋值 —— 5 个 app 里第 3 个抛异常时,前 2 个 +**确实已经翻转并持久化**,却随栈一起被丢弃:响应里 `unhiddenApps` 压根不存在。 + +后果有两层,都指向同一个「机器可读面在撒谎」: + +1. **响应少报了真实发生的事。** 调用方看到的是「翻转失败」,看不到「其中 2 个已经生效」。 +2. **`metadata:reloaded` 对这 2 个 app 漏播。** 紧随其后的重绑定段读的正是 `unhiddenApps`, + 字段缺失 → 这 2 个已经变可见的 app 不进 `changed` → boot-cached 的消费者(首当其冲是 + automation engine)不重新同步它们,要等下一次重启。 + +修法按 PM 裁定取**增量累积**而非预校验:`unhidden` 与它的赋值一并提到 `try` 之外,名字只在 +对应的 `saveMetaItem` **兑现之后**才 push,因此这个列表在任意时刻恰好等于「已经落盘的那些」。 +赋值移到 `try/catch` 之后,成功与中途失败两条路径都会执行,并且仍在 announce 段之前 —— +部分失败时 `unhiddenApps` 与 `unhideError` **并存**:前者说什么翻成功了,后者说还有没翻完的。 +`unhidden` 是每请求的局部量,不引入任何共享可变状态,符合 #5385 确立的显式传参姿态。 + +同时修掉那条 `error` 日志的措辞:它原先断言「其 app **全部**仍以 `hidden: true` 存着」, +一旦有翻转已落盘这句话就是假的。现在按两半如实点名 —— 哪些确实翻了(列出名字)、哪些仍然 +是隐藏的,以及一如既往的后果与修复动作。 + +响应契约不变:仍然 200,字段还是原来那两个,只是部分失败时它们可以同时出现;重跑依旧幂等 +(已翻转的 app `hidden !== true`,循环会跳过)。 diff --git a/packages/runtime/src/domains/packages.ts b/packages/runtime/src/domains/packages.ts index 4e607d69ac..33f98e5961 100644 --- a/packages/runtime/src/domains/packages.ts +++ b/packages/runtime/src/domains/packages.ts @@ -200,6 +200,19 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin // caller never needs to know how the package was built). // Best-effort: a custom protocol without the meta // primitives keeps plain draft-publish semantics. + // + // #5242 — `unhidden` and its result assignment live OUTSIDE + // this try. A name is pushed only AFTER its `saveMetaItem` + // resolved, so at any moment the list is exactly "what is + // already flipped on disk". When app k of N throws, the k-1 + // that DID persist are a fact the caller must be told about: + // accumulating inside the try and assigning after the loop + // discarded them with the stack, so the response claimed + // nothing happened for apps that had already changed state, + // and the 'metadata:reloaded' announce below — which reads + // `unhiddenApps` — skipped them too, leaving boot-cached + // consumers stale until the next restart. + const unhidden: string[] = []; try { if ( typeof (protocol as any).getMetaItems === 'function' && @@ -213,7 +226,6 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin const apps: any[] = Array.isArray(appsRes) ? appsRes : Array.isArray((appsRes as any)?.items) ? (appsRes as any).items : []; - const unhidden: string[] = []; for (const app of apps) { if (app && typeof app === 'object' && app.hidden === true && typeof app.name === 'string') { await (protocol as any).saveMetaItem({ @@ -227,7 +239,6 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin unhidden.push(app.name); } } - if (unhidden.length > 0) (result as any).unhiddenApps = unhidden; } } catch (e: any) { // #4754 — ADR-0045's visibility flip is a metadata WRITE @@ -240,9 +251,20 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin // there". So it is reported at `error` (AGENTS.md → // "Degradation log levels"), not swallowed. const logger = deps.logger ?? console; + // #5242 — a mid-loop failure leaves the package SPLIT: the + // apps already saved are visible, the rest are not. Name + // BOTH halves. The old wording asserted "every hidden app + // is still stored hidden", which is plainly false once any + // flip persisted, and it left the operator to infer + // "nothing changed" from a bare failure line. + const stillHidden = unhidden.length > 0 + ? `the flip stopped PARTWAY — ${unhidden.length} app(s) DID flip and are stored visible ` + + `(${unhidden.join(', ')}; they are reported under \`unhiddenApps\` and were announced for ` + + `re-sync), while every REMAINING hidden app bound to it` + : `every hidden app bound to it`; logger.error( `[Packages] publish-drafts: the ADR-0045 visibility flip FAILED for package '${id}' — its drafts ARE ` + - `published and live, but every hidden app bound to it is still STORED with \`hidden: true\`, so those ` + + `published and live, but ${stillHidden} is still STORED with \`hidden: true\`, so those ` + `apps stay invisible in the launcher while the publish reports success. Nothing retries this flip. ` + `Re-run POST /packages/${id}/publish-drafts once the cause below is resolved (it is idempotent), or ` + `unhide one app directly via PUT /meta/app/ with \`{"hidden": false}\`. Cause: ` + @@ -250,6 +272,12 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin ); (result as any).unhideError = e?.message ?? 'visibility flip failed'; } + // Assigned on BOTH paths — clean completion and mid-loop + // failure alike. On the failure path it rides ALONGSIDE + // `unhideError`: together they say what did flip and that + // something did not, which is the honest report. It must + // stay ABOVE the announce block, which reads this field. + if (unhidden.length > 0) (result as any).unhiddenApps = unhidden; // A publish promoted drafts to active (or unhid an additive // app) at RUNTIME — but boot-cached consumers still hold the // pre-publish view. The load-bearing one is the automation diff --git a/packages/runtime/src/http-dispatcher.test.ts b/packages/runtime/src/http-dispatcher.test.ts index 3ed46db2c9..3b82a0f5f2 100644 --- a/packages/runtime/src/http-dispatcher.test.ts +++ b/packages/runtime/src/http-dispatcher.test.ts @@ -1921,6 +1921,75 @@ describe('HttpDispatcher', () => { errorSpy.mockRestore(); } }); + + // #5242 — the flip is a LOOP of independent writes, and each one that + // resolves is durable on its own. When app k of N throws, the k-1 that + // already persisted ARE visible on disk; a response that omits them + // tells the caller nothing happened for apps whose state DID change, + // and the 'metadata:reloaded' announce (which reads `unhiddenApps`) + // then skips exactly those apps, leaving boot-cached consumers stale. + it('POST /packages/:id/publish-drafts reports the apps already unhidden when the flip fails MID-LOOP', async () => { + const publishPackageDrafts = vi.fn().mockResolvedValue({ + success: true, publishedCount: 0, failedCount: 0, published: [], failed: [], seedApplied: { success: true }, + }); + // 4 hidden apps; the write for the 3rd rejects. So `alpha` and + // `beta` are persisted visible, `gamma` and `delta` are not. + const getMetaItems = vi.fn().mockResolvedValue([ + { name: 'alpha', hidden: true, navigation: [] }, + { name: 'beta', hidden: true, navigation: [] }, + { name: 'gamma', hidden: true, navigation: [] }, + { name: 'delta', hidden: true, navigation: [] }, + ]); + const saveMetaItem = vi.fn().mockImplementation(async ({ name }: { name: string }) => { + if (name === 'gamma') throw new Error('sys_metadata write rejected'); + return { ok: true }; + }); + (kernel as any).getService = vi.fn().mockImplementation((name: string) => { + if (name === 'protocol') return Promise.resolve({ publishPackageDrafts, getMetaItems, saveMetaItem }); + if (name === 'objectql') return Promise.resolve({ registry: { getAllPackages: vi.fn().mockReturnValue([]) } }); + return null; + }); + const trigger = vi.fn().mockResolvedValue(undefined); + (kernel as any).context.trigger = trigger; + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + try { + const result = await dispatcher.handlePackages('/app.partial/publish-drafts', 'POST', {}, {}, { request: {} }); + + // The loop stopped at `gamma` — `delta` was never attempted. + expect(result.response?.status).toBe(200); + expect(saveMetaItem).toHaveBeenCalledTimes(3); + expect(saveMetaItem).not.toHaveBeenCalledWith(expect.objectContaining({ name: 'delta' })); + + const data = (result.response as any)?.body?.data; + // The two flips that DID persist are reported, not discarded + // with the stack — and the failure is reported alongside them, + // so the body names what flipped AND that something did not. + expect(data?.unhiddenApps).toEqual(['alpha', 'beta']); + expect(data?.unhideError).toBe('sys_metadata write rejected'); + + // ...and the same two reach the re-sync broadcast, so a + // boot-cached consumer picks up the apps that really changed + // instead of waiting for a restart. + expect(trigger).toHaveBeenCalledWith( + 'metadata:reloaded', + expect.objectContaining({ changed: ['app/alpha', 'app/beta'] }), + ); + + // The operator-facing line names BOTH halves: what flipped and + // what is still stored hidden. The old wording claimed "every + // hidden app is still stored hidden", which is false here. + const line = errorSpy.mock.calls + .map((c) => String(c?.[0] ?? '')) + .find((l) => l.includes('[Packages] publish-drafts')) ?? ''; + expect(line).toContain('alpha, beta'); + expect(line).toMatch(/PARTWAY/); + expect(line).toMatch(/REMAINING hidden app/); + expect(line).toContain('sys_metadata write rejected'); + } finally { + errorSpy.mockRestore(); + } + }); }); // ═══════════════════════════════════════════════════════════════