fix(runtime): publish-drafts 中途失败时,已落盘的可见性翻转不再从响应消失 (#5242)

.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`,循环会跳过)。

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,16 +251,33 @@ 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/<name> with \`{"hidden": false}\`. Cause: ` +
                             `${e?.message ?? String(e)}`,
                         );
                         (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

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();
+            }
+        });
     });
 
     // ═══════════════════════════════════════════════════════════════