Skip to content

Show the app fit on the app page only, with Install disabled - #626

Merged
onel merged 5 commits into
devfrom
feat/fit-notice-placement
Oct 13, 2026
Merged

onel merged 5 commits into
devfrom
feat/fit-notice-placement

Conversation

@onel

@onel onel commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Follows #625 (#599), after trying it on the dev box.

What changes

  • No fit badge on store cards. A too-big or tight app is listed as usual.
  • A yellow alert above the price panel on the App page (Tailwind UI alert with description, WarningAlert.vue). Too big: "This app can't run on this box as it is." / "It requires X of memory, and this box only has Y." The brain's install refusal uses the same words. Placement and wording are the maintainer's choice after testing.
  • Install stays, disabled, for a too-big app on the App page and on pack pages (no sentence on the pack row). needs-remap is unchanged.
  • Disk warning on the App page in the same alert ("This might not fit."), with the install pages' 90% rule. Warning only.
  • Refactor: Catalog.WithFit is removed. Fit moved from Entry to Detail, the detail handler computes it from Detail.Recommended(), and list routes no longer read the box. useStartInstall returns tooBig.

Specs, architecture.md and docs/progress/fit-notice-placement.md updated.

Tested

  • Go: catalog, api, lifecycle, fit suites. Web typecheck and build.
  • Tried on the dev box by the maintainer (too-big alert, disabled Install).

After trying #599 on a running box, the maintainer wants no fit
warning in the store, the notice in the right column under
Information, and a too-big app's Install kept but disabled. Store cards
lose the badge, and the list routes stop carrying fit, so a list
request no longer reads the box. The notice moves under the Memory and
CPU rows. Install stays disabled for too-big on the App page and the
pack page, which says nothing about why.
@greptile-apps

greptile-apps Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; the remaining test-coverage suggestion is non-blocking.

Findings

  1. P2 Box reads go unchecked ▶

Reviews (3) · Last reviewed commit: "Fix the fit alert's position comment and..." · Reviewed by Greptile

Comment thread internal/api/catalog_fit_test.go Outdated
Comment on lines 73 to 78
listed := map[string]bool{}
for _, e := range decodeJSON[appsBody](t, h.getOK("/api/v1/catalog")).Apps {
if e.Fit == nil {
t.Fatalf("%s carries no fit", e.ID)
listed[e.ID] = true
if e.Fit != nil {
t.Errorf("list: %s carries a fit; the store lists show none", e.ID)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Box reads go unchecked

TestCatalogCarriesFit checks that /catalog omits fit, but not that browsing skips SystemResources. A list request could still read the box and discard the result, and this test would pass.

Count those calls and assert zero for the list routes before checking detail and install-plan responses. Also check that entries inside home, category, search, and pack responses omit fit.

onel added 2 commits October 14, 2026 00:21
From testing on the dev box. The fit notice is a yellow Tailwind alert
with a title and a description, as its own block above the price panel.
The too-big text reads "This app can't run on this box as it is. It
requires X of memory, and this box only has Y.", and the brain's install
refusal says the same. The App page also warns when the disk may not
fit the app, with the 90% rule the install pages use. WarningAlert holds
the alert markup both use.
The store lists stopped showing a fit, so Catalog.WithFit walked every
list shape for one caller. The detail handler now computes the fit
from Detail.Recommended, and Fit moves from Entry to Detail, where it
is set. useStartInstall returns tooBig, which the App page and the pack
row each worked out on their own.
Comment thread web-ui/src/views/AppDetailView.vue
onel added 2 commits October 14, 2026 00:33
The specs, the architecture row and the progress entry still described
the notice under Information and Catalog.WithFit. They now say what was
built after testing: the yellow alert above the price panel, its text,
the disk warning on the App page, and Fit as a Detail field.
@onel

onel commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

Greptile P2s: the alert position above the price panel is the maintainer's choice after testing (the old PR description said otherwise; updated). fit is no longer an Entry field, so list routes cannot carry it; a no-host-read test for lists is extra coverage, not a fix, and is left out.

@onel
onel merged commit 5ff76db into dev Oct 13, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant