Repository navigation
Commit bafb58b
fix(cli): warn at boot when an app's branding names a runtime asset the server will not serve (#22089)
Fixes #22071
Clause-②: no
## What changed
`os serve` resolves the runtime assets directory as
`OS_RUNTIME_ASSETS_DIR`, else the `assets/` directory under its working
directory. `createRuntimeAssetsPlugin` mounted `GET
/runtime/assets/:filename` only when that directory existed, and said
nothing when it did not. An artifact booted from any other directory
therefore served a 404 for an app's `branding.logo` / `branding.favicon`
and printed nothing about it.
Now, once the boot has settled (`kernel:bootstrapped`), the plugin reads
the served app list. It prints ONE warning per branding URL under
`/runtime/assets/` that this boot will not serve. The line names:
- every app and key that uses the file;
- the file;
- the directory searched;
- `OS_RUNTIME_ASSETS_DIR`, and whether the directory came from it or
from the working-directory default;
- when the directory does not exist, that fact, and that nothing under
`/runtime/assets/` is mounted for this run.
What the route serves does not change. No route is added or removed
(triage ruling `6038494234`; carrying the files in the artifact stays
out of scope).
Files: `packages/cli/src/utils/console.ts` (the plugin),
`packages/cli/src/commands/serve.ts` (the runtime assets block now
passes which source the directory came from), the two test files below,
and one `@objectstack/cli` `patch` changeset.
## The PM's readings (dispatch H1 to H5), measured
**H1, reproduce first. Held.** Measured on `origin/main` `3d918850` with
the CLI run from source. The artifact had one app with `branding: {
logo: '/runtime/assets/icon.svg', favicon: '/runtime/assets/icon.svg'
}`. Each boot ran from a fresh directory holding only that artifact
(`OS_ARTIFACT_PATH`, the same `serve` that `start --artifact` spawns in
its own working directory).
| boot | `GET /runtime/assets/icon.svg` | boot lines mentioning assets,
branding or `icon.svg` |
| --- | --- | --- |
| before, no `assets/` | 404 | 0 |
| before, `assets/icon.svg` beside the artifact | 200 `image/svg+xml` |
0 |
| before, `OS_RUNTIME_ASSETS_DIR` naming a directory with `icon.svg` |
200 `image/svg+xml` | 0 |
| after, no `assets/` | 404 | **1**, in *Boot diagnostics* |
| after, `assets/` control | 200 `image/svg+xml` | 0 |
| after, `OS_RUNTIME_ASSETS_DIR` control | 200 `image/svg+xml` | 0 |
The line printed on the first after-boot (temp path shortened to
`DEPLOY`):
```text
WARN Branding asset not served: app 'brand_app' (branding.logo, branding.favicon) → /runtime/assets/icon.svg, but the directory searched, DEPLOY/assets (the CWD/assets default, since OS_RUNTIME_ASSETS_DIR is unset), does not exist, so /runtime/assets/ is not mounted this run; the console will draw a broken image. To fix, put icon.svg in that directory and restart, or set OS_RUNTIME_ASSETS_DIR to the directory that holds it.
```
The real line spells `CWD` as `cwd` inside angle brackets. It is written
`CWD` here because GitHub strips angle-bracket fragments from bodies.
**H2, where the loaded apps' branding is read.**
`kernel.getService('protocol').getMetaItems({ type: 'app' })`
(`packages/metadata-protocol/src/protocol.ts`, `getMetaItems`, the
`served` audience). `GET /api/v1/meta/app` answers from the same call
(`packages/rest/src/rest-server.ts`, the `GET /meta/:type` list door,
`p.getMetaItems(listRequest)`). The console's app list and chrome are
drawn from that route: objectui `MetadataProvider` calls
`client.meta.getItems('app')`, read at objectui `9dfaca654`, not at the
`.objectui-sha` pin. Config boots and artifact boots both register their
apps with that protocol. The artifact boot above is the measurement: the
warning names `brand_app`, which only that read could have supplied.
Both `{ type, items }` and a bare array are accepted, as the REST door's
own comment says `getMetaItems` can answer either.
**H3, one resolution. Took the PM's lean.** A private helper in
`utils/console.ts`, `resolveRuntimeAssetPath(assetsDir, filename)`,
holds the route's own steps: strip separators, `path.join`, then the
traversal guard (`null` maps to 403). The route now serves through it
and the check judges through it. The route's behaviour is byte-for-byte
what it was. "Servable" adds only what the route's `readFileSync` needs:
a readable regular file. A directory and a missing file both answer no,
exactly as the route 404s them. The helper is not exported: this
module's export set is pinned by
`test/published-subpath-console.pin.test.ts`, and a 14th export would
need that pin edited.
**H4, what the matcher accepts.** A `branding.logo` / `branding.favicon`
string that:
- after trimming, is a root path (one leading `/`);
- resolved the way a browser resolves an `img` `src` on this origin,
stays on this origin and has a pathname that starts with
`/runtime/assets/` and names something after it.
Query, fragment and dot segments are removed by that resolution. The
segment is percent-decoded the way the route's `:filename` parameter is.
A path below a subdirectory is reported as a subdirectory, because the
route's single segment never matches it.
These print nothing: absolute URLs, protocol-relative URLs (including
the backslash spelling a browser reads the same way), data URIs,
relative paths, and any other root path (`/assets/…`,
`/runtime/assetsx/…`). The unit test pins every one of them, with a
positive control in the same test case.
**H5, the channel. One, named.** The line goes out through the plugin's
own `ctx.logger.warn`, at `kernel:bootstrapped`, inside
`runtime.start()`:
- under `serve`'s boot-quiet window, `BootLogCapture` captures it and
*Boot diagnostics* (`printBootDiagnostics`) replays it once;
- at `--log-level debug` or `info` it streams live;
- at `error` or `silent` it is hidden, like every boot warning.
It is never printed from the banner list. The sibling #22073
de-duplicates the banner list against *Boot diagnostics*, and that work
sees this line in exactly one of the two.
Failure handling: the hook must never fail the boot it reports on. A
missing protocol, or a read that throws, is logged at `debug` and the
hook returns.
## Tests
Final head is `c397a0f3`. Every run below is from that head;
`origin/main` had not moved from `3d918850`.
- `packages/cli/test/runtime-assets.test.ts`, unit tier, per PR. The
existing three cases were kept and their two-argument call updated. The
old "silently skips" case now asserts that no route is mounted AND the
check is registered. New cases:
- the absent-directory warning appears ONCE for a file both keys name;
- the file-present control: a 200 from the route and no line, with a
positive control that the app list WAS read;
- a present directory with the file missing: no claim that the directory
is absent;
- one line per file across apps;
- the H4 matcher table;
- a URL by URL check that the warning and the captured route handler
agree (query, fragment, dot segment, whitespace, percent-encoding, a
directory, a subdirectory, a missing file);
- the subdirectory reason;
- a protocol that is absent or throws never fails the boot.
- Result: `Tests 12 passed (12)`.
- `packages/cli/test/serve-runtime-assets-branding-warning.e2e.test.ts`,
integration project, `e2e` tier. The triage pins as three real boots of
one artifact from a fresh directory: no `assets/`; the
`OS_RUNTIME_ASSETS_DIR` control; the `assets/` control. It asserts
exactly one boot line naming the URL (with the app, both keys, the
directory, `OS_RUNTIME_ASSETS_DIR` and the absent directory), and a 404
that does not change. In both controls, no line and a 200 with the exact
bytes. By its name it runs on the nightly tier
(`scripts/nightly-tiers.mjs`); the per-PR half is the unit file. Local
run with `OS_TEST_TIERS=nightly`: `Tests 3 passed (3)`.
- Ablations, committed first, mutated and restored by
`scripts/ablation-replace.mjs` (anchor hit 1 to 0, blob changed,
restored blob equal to HEAD, `git diff HEAD` empty). Both subjects are
imported from `src`: the unit test imports `../src/utils/console.js` and
the e2e child runs `bin/run-dev.js` from source through tsx. No `dist/`
is in the path, so no rebuild leg applies.
- `kernel:bootstrapped` renamed so the check never fires: unit `9 failed
| 3 passed`. The e2e no-`assets/` boot went red; both controls stayed
green.
- The parameter decode dropped, so the matcher diverges from the route:
unit `1 failed | 11 passed`, the route-agreement case.
- The first unit leg of the first ablation was run under
`OS_TEST_TIERS=nightly` and selected nothing ("FILTER SELECTED
NOTHING"). It was a no-op and is not counted; it was re-run without the
switch.
- `pnpm --filter @objectstack/cli test` (unit and integration projects,
`e2e` tier excluded by default): `Test Files 352 passed (352)`, `Tests
4692 passed | 2 skipped (4694)`, exit 0. `pnpm --filter @objectstack/cli
typecheck`: exit 0. It is `tsc --noEmit` plus `check:test-typecheck:
OK`, with 3 files, 28 errors and 6 pinned signatures held in
`test-typecheck-debt.json`, unchanged.
## Gates
All of these ran at `c397a0f3`, and each exit code was captured before
any pipe.
- The 67 families `node scripts/pm/dispatch-gates.mjs --repo
objectstack-ai/objectstack --commands` derives over this diff all exited
0. That list is identical to the dispatch's.
- `check:dual-build-cjs-loads` and `check:i18n-coverage` first answered
exit 3, PREREQUISITE NOT MET: some packages had no `dist/`, so nothing
was measured.
- Both were re-run after `turbo run build --filter='!@objectstack/docs'`
(72 tasks, 71 cached) and both exited 0: `106 published require entry
point(s) across 66 package(s) load` and `OK (13 config(s), 621 baselined
untranslated string(s), none new)`.
- `--ran` reconcile: `67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN`.
- `pnpm lint`, the full `eslint . --no-inline-config` the dispatch adds:
exit 0, no findings.
- `pnpm check:startup-registry-verdict` exited 0, with `43
startup/open-registry seam(s) … none recording a verdict the boot can
contradict`. It is not derived for this diff; it was run because the
change adds a boot-time registry read.
## Acceptance notes
- `examples/app-todo/src/apps/todo.app.ts` sets `branding.logo:
'/assets/todo-logo.png'` and `favicon: '/assets/todo-favicon.ico'`.
Neither file exists in the example, and nothing serves `/assets/` at the
root. This is a read-only inference: the example was not booted or
measured. By H4 it is outside this check, which reads only
`/runtime/assets/`. No carrier.
- The boot check reads the environment-wide app list (no organization),
the same list an unscoped `GET /api/v1/meta/app` answers. An app that
exists only as one organization's overlay row is not checked at boot.
- When the directory is absent, the route stays unmounted for the life
of the process; the line says "restart". Mounting it lazily would change
what `/runtime/assets/*` serves, which this card rules out.
- Every boot measured here printed `WARN Insert operation failed
{"object":"sys_migration", … UNIQUE constraint failed: sys_migration.id
…}` with a full knex stack in *Boot diagnostics*. That is 6 of 6
artifact boots on a fresh `:memory:` database, 3 of them on
`origin/main` before this change. It is unrelated to this card and is
reported to the seat in the dev report.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01RWZbGvPFcRKvUqASZtunCU)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent b04a529 commit bafb58b
5 files changed
Lines changed: 717 additions & 26 deletions
File tree
- .changeset
- packages/cli
- src
- commands
- utils
- test
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3715 | 3715 | | |
3716 | 3716 | | |
3717 | 3717 | | |
3718 | | - | |
3719 | | - | |
3720 | | - | |
3721 | | - | |
3722 | | - | |
3723 | | - | |
| 3718 | + | |
| 3719 | + | |
| 3720 | + | |
| 3721 | + | |
| 3722 | + | |
| 3723 | + | |
| 3724 | + | |
| 3725 | + | |
| 3726 | + | |
| 3727 | + | |
3724 | 3728 | | |
3725 | 3729 | | |
3726 | 3730 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
591 | 591 | | |
592 | 592 | | |
593 | 593 | | |
| 594 | + | |
| 595 | + | |
| 596 | + | |
| 597 | + | |
| 598 | + | |
| 599 | + | |
| 600 | + | |
| 601 | + | |
| 602 | + | |
| 603 | + | |
| 604 | + | |
| 605 | + | |
| 606 | + | |
| 607 | + | |
| 608 | + | |
| 609 | + | |
| 610 | + | |
| 611 | + | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
| 623 | + | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
| 670 | + | |
| 671 | + | |
| 672 | + | |
| 673 | + | |
| 674 | + | |
| 675 | + | |
| 676 | + | |
| 677 | + | |
| 678 | + | |
| 679 | + | |
| 680 | + | |
| 681 | + | |
| 682 | + | |
| 683 | + | |
| 684 | + | |
| 685 | + | |
| 686 | + | |
| 687 | + | |
| 688 | + | |
| 689 | + | |
| 690 | + | |
| 691 | + | |
| 692 | + | |
| 693 | + | |
| 694 | + | |
| 695 | + | |
| 696 | + | |
| 697 | + | |
| 698 | + | |
| 699 | + | |
| 700 | + | |
| 701 | + | |
| 702 | + | |
| 703 | + | |
| 704 | + | |
| 705 | + | |
| 706 | + | |
| 707 | + | |
| 708 | + | |
| 709 | + | |
| 710 | + | |
| 711 | + | |
| 712 | + | |
| 713 | + | |
| 714 | + | |
| 715 | + | |
| 716 | + | |
| 717 | + | |
| 718 | + | |
| 719 | + | |
| 720 | + | |
| 721 | + | |
| 722 | + | |
| 723 | + | |
| 724 | + | |
| 725 | + | |
| 726 | + | |
| 727 | + | |
| 728 | + | |
| 729 | + | |
| 730 | + | |
| 731 | + | |
| 732 | + | |
| 733 | + | |
| 734 | + | |
| 735 | + | |
| 736 | + | |
| 737 | + | |
| 738 | + | |
| 739 | + | |
594 | 740 | | |
595 | 741 | | |
596 | 742 | | |
597 | 743 | | |
598 | 744 | | |
599 | 745 | | |
600 | 746 | | |
601 | | - | |
| 747 | + | |
| 748 | + | |
| 749 | + | |
| 750 | + | |
| 751 | + | |
| 752 | + | |
| 753 | + | |
| 754 | + | |
| 755 | + | |
| 756 | + | |
| 757 | + | |
602 | 758 | | |
603 | | - | |
| 759 | + | |
604 | 760 | | |
605 | 761 | | |
606 | 762 | | |
| |||
612 | 768 | | |
613 | 769 | | |
614 | 770 | | |
615 | | - | |
| 771 | + | |
| 772 | + | |
| 773 | + | |
| 774 | + | |
| 775 | + | |
| 776 | + | |
| 777 | + | |
| 778 | + | |
| 779 | + | |
| 780 | + | |
| 781 | + | |
| 782 | + | |
| 783 | + | |
| 784 | + | |
| 785 | + | |
| 786 | + | |
| 787 | + | |
| 788 | + | |
| 789 | + | |
616 | 790 | | |
617 | 791 | | |
618 | | - | |
619 | | - | |
620 | | - | |
621 | | - | |
| 792 | + | |
| 793 | + | |
622 | 794 | | |
623 | 795 | | |
624 | 796 | | |
| |||
0 commit comments