|
42 | 42 | * the same listing. |
43 | 43 | */ |
44 | 44 |
|
45 | | -import { describe, it, expect, beforeAll, afterAll } from 'vitest'; |
| 45 | +import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'; |
46 | 46 | import { mkdtempSync, rmSync } from 'node:fs'; |
47 | 47 | import { tmpdir } from 'node:os'; |
48 | 48 | import { join } from 'node:path'; |
@@ -95,14 +95,38 @@ function make() { |
95 | 95 | // Present so a DELETE that is ALLOWED reaches its persisted half too — |
96 | 96 | // the allow-path must be exercised end to end, not just to the gate. |
97 | 97 | deletePackage: async () => ({ deletedCount: 1 }), |
| 98 | + // [#14451] This double MINTS THE TARGET PACKAGE RECORD, because the real |
| 99 | + // `duplicatePackage` does: its `registry.installPackage` call runs ahead |
| 100 | + // of the copy loop, which is why the reported defect left a real, empty |
| 101 | + // `com.acme.dupbase` in `GET /packages`. |
| 102 | + // |
| 103 | + // ⚠️ Not decoration — it is what makes the refusal cases falsifiable. A |
| 104 | + // double that only returned a value would leave "the target is not in |
| 105 | + // the listing" true whether or not the gate exists, i.e. an assertion |
| 106 | + // that can never go red. With the mint in place, deleting the gate makes |
| 107 | + // those cases fail on the listing, not merely on the status. |
| 108 | + duplicatePackage: vi.fn(async (req: any) => { |
| 109 | + registry.installPackage(manifest(req.targetPackageId)); |
| 110 | + // The exact empty-success body the card measured on a running code |
| 111 | + // package: HTTP 200, `success: false`, nothing copied, nothing named |
| 112 | + // as failed. |
| 113 | + return { |
| 114 | + success: false, |
| 115 | + copiedCount: 0, |
| 116 | + failedCount: 0, |
| 117 | + targetPackageId: req.targetPackageId, |
| 118 | + copied: [], |
| 119 | + failed: [], |
| 120 | + }; |
| 121 | + }), |
98 | 122 | }; |
99 | 123 | const kernel: any = { |
100 | 124 | context: { |
101 | 125 | getService: (name: string) => |
102 | 126 | name === 'objectql' ? objectql : name === 'protocol' ? protocol : null, |
103 | 127 | }, |
104 | 128 | }; |
105 | | - return { dispatcher: new HttpDispatcher(kernel), registry }; |
| 129 | + return { dispatcher: new HttpDispatcher(kernel), registry, protocol }; |
106 | 130 | } |
107 | 131 |
|
108 | 132 | /** Authorized under #7033 — holds the write capability on every call below. */ |
@@ -292,3 +316,179 @@ describe('/packages lifecycle — an unknown package id keeps its 404 (#7560)', |
292 | 316 | expect(r.response?.status).toBe(404); |
293 | 317 | }); |
294 | 318 | }); |
| 319 | + |
| 320 | +// ══════════════════════════════════════════════════════════════════════════════ |
| 321 | +// 6. [#14451] POST /:id/duplicate — the SOURCE must be a base |
| 322 | +// |
| 323 | +// ## The defect this section pins |
| 324 | +// |
| 325 | +// `POST /api/v1/packages/com.example.todo/duplicate` against a RUNNING code |
| 326 | +// package answered **HTTP 200** with |
| 327 | +// `{"success":false,"copiedCount":0,"failedCount":0,"copied":[],"failed":[]}` |
| 328 | +// and left a real, empty `com.acme.dupbase` behind in `GET /packages`. |
| 329 | +// Reproduced independently twice on two separate `os dev` processes. |
| 330 | +// |
| 331 | +// `copiedCount: 0` there is BY CONSTRUCTION: `duplicatePackage` clones the |
| 332 | +// source's `sys_metadata` rows, and a code package's metadata is delivered as |
| 333 | +// code, so the scan is one that could never have found anything. That is the |
| 334 | +// #11063 ruling one route over — "a read that could not happen must not be |
| 335 | +// reported as a read that found nothing" — and the caller could not tell it |
| 336 | +// from a base that really is empty. |
| 337 | +// |
| 338 | +// ⛔ The fix is NOT to make duplicate clone code items. ADR-0070 D4 is declared |
| 339 | +// and NOT built ("D4–D6 remaining") and its object is a *base*; cloning a code |
| 340 | +// package would EXTEND D4, and the ADR still lists that as an open question |
| 341 | +// ("should customising a code item also fork it into a writable base?"). |
| 342 | +// |
| 343 | +// ## What is asserted, and why it is not the status code |
| 344 | +// |
| 345 | +// The harm was the empty shell, so every refusal case asserts the listing — |
| 346 | +// and the protocol double MINTS that shell exactly as the real implementation |
| 347 | +// does, so these assertions can actually go red. A double that merely returned |
| 348 | +// a value would make "the target is absent" true with or without the gate. |
| 349 | +// ══════════════════════════════════════════════════════════════════════════════ |
| 350 | + |
| 351 | +const DUP_TARGET = 'com.acme.dupbase'; |
| 352 | + |
| 353 | +describe('/packages duplicate — a source that is not a base is refused (#14451)', () => { |
| 354 | + for (const pkg of READ_ONLY_PACKAGES) { |
| 355 | + it(`422s duplicate of the ${pkg.label} package AND mints no target record`, async () => { |
| 356 | + const { dispatcher, registry, protocol } = make(); |
| 357 | + const before = listedIds(registry); |
| 358 | + |
| 359 | + const r = await dispatcher.handlePackages( |
| 360 | + `/${pkg.id}/duplicate`, 'POST', { targetPackageId: DUP_TARGET, targetName: 'Dup Base' }, {}, admin(), |
| 361 | + ); |
| 362 | + |
| 363 | + expect(r.response?.status).toBe(422); |
| 364 | + expect(r.response?.body?.error?.code).toBe('DUPLICATE_SOURCE_NOT_A_BASE'); |
| 365 | + // THE observable the card was written from: the pre-fix answer left |
| 366 | + // a real, listed, `writable: true` package record behind. |
| 367 | + expect(listedIds(registry)).toEqual(before); |
| 368 | + expect(listedIds(registry)).not.toContain(DUP_TARGET); |
| 369 | + expect(registry.getPackage(DUP_TARGET)).toBeUndefined(); |
| 370 | + // The refusal ran BEFORE the protocol call — which is the only |
| 371 | + // placement that can keep the shell from being minted, since |
| 372 | + // `duplicatePackage` installs it ahead of its own copy loop. |
| 373 | + expect(protocol.duplicatePackage).not.toHaveBeenCalled(); |
| 374 | + // …and the source is untouched: this route never wrote to it, and |
| 375 | + // the refusal must not have changed that. |
| 376 | + expect(registry.getPackage(pkg.id)).toBeDefined(); |
| 377 | + }); |
| 378 | + } |
| 379 | + |
| 380 | + it('answers the ADR-0112 envelope with the package id and the ADR pointer', async () => { |
| 381 | + const { dispatcher } = make(); |
| 382 | + const r = await dispatcher.handlePackages( |
| 383 | + `/${CODE_LOADED}/duplicate`, 'POST', { targetPackageId: DUP_TARGET }, {}, admin(), |
| 384 | + ); |
| 385 | + const err = r.response?.body?.error; |
| 386 | + expect(r.response?.status).toBe(422); |
| 387 | + expect(err?.code).toBe('DUPLICATE_SOURCE_NOT_A_BASE'); |
| 388 | + expect(err?.httpStatus).toBe(422); |
| 389 | + expect(err?.details?.packageId).toBe(CODE_LOADED); |
| 390 | + expect(err?.details?.docs).toBe('docs/adr/0070-package-first-authoring.md'); |
| 391 | + // The remedy is the one that EXISTS for a code package (ADR-0005 |
| 392 | + // overlay), and the message says why the copy would be empty rather |
| 393 | + // than only that it is refused. |
| 394 | + expect(err?.message).toContain('ADR-0005'); |
| 395 | + expect(err?.message).toContain('read-only'); |
| 396 | + }); |
| 397 | + |
| 398 | + it('is its OWN code, not WRITABLE_PACKAGE_REQUIRED — the two conditions differ', async () => { |
| 399 | + const { dispatcher } = make(); |
| 400 | + const dup = await dispatcher.handlePackages( |
| 401 | + `/${CODE_LOADED}/duplicate`, 'POST', { targetPackageId: DUP_TARGET }, {}, admin(), |
| 402 | + ); |
| 403 | + const del = await dispatcher.handlePackages(`/${CODE_LOADED}`, 'DELETE', {}, {}, admin()); |
| 404 | + // Same predicate (`isWritablePackage`), same status, different meaning: |
| 405 | + // DELETE is refused because the package may not be WRITTEN to; duplicate |
| 406 | + // is refused because the GESTURE does not apply to this source. A caller |
| 407 | + // told `WRITABLE_PACKAGE_REQUIRED` here would hunt for a way to make a |
| 408 | + // code package writable, which is neither possible nor the remedy. |
| 409 | + expect(del.response?.body?.error?.code).toBe('WRITABLE_PACKAGE_REQUIRED'); |
| 410 | + expect(dup.response?.body?.error?.code).toBe('DUPLICATE_SOURCE_NOT_A_BASE'); |
| 411 | + expect(dup.response?.body?.error?.code).not.toBe(del.response?.body?.error?.code); |
| 412 | + }); |
| 413 | +}); |
| 414 | + |
| 415 | +// ══════════════════════════════════════════════════════════════════════════════ |
| 416 | +// 7. [#14451] The gate is not an outage — the control that must be non-zero |
| 417 | +// ══════════════════════════════════════════════════════════════════════════════ |
| 418 | + |
| 419 | +describe('/packages duplicate — a WRITABLE base still duplicates (#14451)', () => { |
| 420 | + it('reaches the protocol with the source and target it was given, and mints the target', async () => { |
| 421 | + const { dispatcher, registry, protocol } = make(); |
| 422 | + expect(listedIds(registry)).not.toContain(DUP_TARGET); |
| 423 | + |
| 424 | + const r = await dispatcher.handlePackages( |
| 425 | + `/${WRITABLE}/duplicate`, 'POST', { targetPackageId: DUP_TARGET, targetName: 'Dup Base' }, {}, admin(), |
| 426 | + ); |
| 427 | + |
| 428 | + expect(r.response?.status).toBe(200); |
| 429 | + expect(protocol.duplicatePackage).toHaveBeenCalledTimes(1); |
| 430 | + expect(protocol.duplicatePackage.mock.calls[0]?.[0]).toMatchObject({ |
| 431 | + sourcePackageId: WRITABLE, |
| 432 | + targetPackageId: DUP_TARGET, |
| 433 | + targetName: 'Dup Base', |
| 434 | + }); |
| 435 | + // The non-zero control the refusal cases are read against: on the allow |
| 436 | + // path the double really does mint the record, so their `not.toContain` |
| 437 | + // is a measurement rather than a vacuous truth. |
| 438 | + expect(listedIds(registry)).toContain(DUP_TARGET); |
| 439 | + }); |
| 440 | + |
| 441 | + it('an EMPTY writable base still answers 200 / copiedCount 0 — that read HAPPENED', async () => { |
| 442 | + // ⛔ Deliberately unchanged by this card. The axis is whether the gesture |
| 443 | + // APPLIES to the source, never whether it found anything: a base that |
| 444 | + // owns no active rows is a read that ran and came back empty, which is |
| 445 | + // the legitimate arm of the #11063 ruling. Narrowing this to "refuse an |
| 446 | + // empty result" would refuse a legitimate duplicate of a fresh base. |
| 447 | + const { dispatcher } = make(); |
| 448 | + const r = await dispatcher.handlePackages( |
| 449 | + `/${WRITABLE}/duplicate`, 'POST', { targetPackageId: DUP_TARGET }, {}, admin(), |
| 450 | + ); |
| 451 | + expect(r.response?.status).toBe(200); |
| 452 | + expect(r.response?.body?.data?.copiedCount).toBe(0); |
| 453 | + expect(r.response?.body?.data?.success).toBe(false); |
| 454 | + }); |
| 455 | + |
| 456 | + it('an UNKNOWN source id falls through to the protocol — the gate is no existence oracle', async () => { |
| 457 | + // Same rule `requireWritablePackage` follows: an id that resolves to |
| 458 | + // nothing is treated as writable, so it reaches the route's own answer |
| 459 | + // instead of being re-labelled 422 by a gate that would then leak which |
| 460 | + // ids exist. |
| 461 | + const { dispatcher, protocol } = make(); |
| 462 | + const r = await dispatcher.handlePackages( |
| 463 | + '/com.nobody.nothing/duplicate', 'POST', { targetPackageId: DUP_TARGET }, {}, admin(), |
| 464 | + ); |
| 465 | + expect(r.response?.status).not.toBe(422); |
| 466 | + expect(protocol.duplicatePackage).toHaveBeenCalledTimes(1); |
| 467 | + }); |
| 468 | +}); |
| 469 | + |
| 470 | +// ══════════════════════════════════════════════════════════════════════════════ |
| 471 | +// 8. [#14451] The sibling refusal stops prescribing a dead end |
| 472 | +// ══════════════════════════════════════════════════════════════════════════════ |
| 473 | + |
| 474 | +describe('/packages lifecycle — the read-only refusal no longer sends callers at duplicate (#14451)', () => { |
| 475 | + for (const verb of [ |
| 476 | + { label: 'DELETE', path: (id: string) => `/${id}`, method: 'DELETE' }, |
| 477 | + { label: 'disable', path: (id: string) => `/${id}/disable`, method: 'PATCH' }, |
| 478 | + ]) { |
| 479 | + it(`${verb.label} prescribes the ADR-0005 overlay, not POST /:id/duplicate`, async () => { |
| 480 | + const { dispatcher } = make(); |
| 481 | + const r = await dispatcher.handlePackages(verb.path(CODE_LOADED), verb.method, {}, {}, admin()); |
| 482 | + const message = String(r.response?.body?.error?.message ?? ''); |
| 483 | + |
| 484 | + expect(r.response?.body?.error?.code).toBe('WRITABLE_PACKAGE_REQUIRED'); |
| 485 | + // It used to read "…or duplicate this one into a writable base |
| 486 | + // (POST /packages/<id>/duplicate) and change that" — a remedy that, |
| 487 | + // for exactly the packages this refusal fires on, now answers 422 |
| 488 | + // and before that answered an empty 200. A refusal that prescribes a |
| 489 | + // dead end is worse than one that prescribes nothing. |
| 490 | + expect(message).not.toContain('/duplicate'); |
| 491 | + expect(message).toContain('ADR-0005'); |
| 492 | + }); |
| 493 | + } |
| 494 | +}); |
0 commit comments