Public: anyone can read this; the crew can change it.

Cmd-K: typing a repository's name finds it, and a repository and its app read as one thing #7775

Merged opened this in answer to

Check · Review · Land · Live done — Landed and live in production.

Merged

Typing `forge` in Cmd-K now puts trevato/forge first, and a repository and its app show as one row. `tsc` and the palette, search, repo, explore, `web` and `web-ui` tests pass. I couldn't run the palette's browser tests: no usable chromium here, so the keyboard path and the new row look are untested in a browser.

shipped production is serving it deploy 119f4d2

opened this

Typing forge in Cmd-K now puts trevato/forge first, and a repository and its app show as one row. tsc and the palette, search, repo, explore, web and web-ui tests pass. I couldn't run the palette's browser tests: no usable chromium here, so the keyboard path and the new row look are untested in a browser.

What changed

  • Ranking: an exact name match outranks everything, then a prefix match. The viewer's own repositories lead each tier. The rule is in rankPalette (src/web/palette.ts).
  • Why it was missing: the palette index held only the 30 newest repositories, and the server's search took the 5 newest matches. An older repository lost to newer forks. The index now lists the viewer's own repositories first. searchLocalRepos ranks a wider page by name before it cuts. The new test uses 60 newer lookalike repositories.
  • One row per repository: the "Apps" group is gone, so everything sits under "Repositories". A row with an app shows "App · running" (or "App · not serving"). Enter goes to the repository, and Cmd/Ctrl+Enter opens the app. The footer shows the new key. Repositories that come from the server's search rather than the index show a plain "App" label, because the search result doesn't carry the app's state.
  • No empty headings: an app-less repository has no app label, and a heading only exists when it has rows.
  • Search result shape: local repository hits in the search results now carry app: boolean. This is an addition, so existing callers are unaffected.
  • listReposByOwner: it now honours withoutArchived. It silently ignored that option before, and the new index needs it.
  • Speed: palette ranking is still done in the browser on the index, so results are instant. The server query is the existing debounced one. I haven't measured the 50 ms target.

Where else the split shows (listed, not changed)

  • The top-nav "Apps" entry and the /apps shelf.
  • The palette's "Apps" and "New app" rows.
  • The terminal palette's repository list.

I decided to leave these alone, because /apps is a real view (live state, previews) rather than a duplicate list.

Left out: the index's per-keystroke cost for repositories beyond the first 30 still depends on the server round-trip. I didn't change #7660's open-instantly work.

Try it: press Cmd-K, type forge, and Cmd/Ctrl+Enter on a row with an app. The tests are in test/palette-repos.test.ts, and the notes are in docs/features/cmd-k-repositories.md.

Closes #7661

commented

Typing forge in Cmd-K now puts trevato/forge first, and a repository and its app show as one row. tsc and the palette, search, repo, explore, web and web-ui tests pass. I couldn't run the palette's browser tests: no usable chromium here, so the keyboard path and the new row look are untested in a browser.

What changed

  • Ranking: an exact name match outranks everything, then a prefix match. The viewer's own repositories lead each tier. The rule is in rankPalette (src/web/palette.ts).
  • Why it was missing: the palette index held only the 30 newest repositories, and the server's search took the 5 newest matches. An older repository lost to newer forks. The index now lists the viewer's own repositories first. searchLocalRepos ranks a wider page by name before it cuts. The new test uses 60 newer lookalike repositories.
  • One row per repository: the "Apps" group is gone, so everything sits under "Repositories". A row with an app shows "App · running" (or "App · not serving"). Enter goes to the repository, and Cmd/Ctrl+Enter opens the app. The footer shows the new key. Repositories that come from the server's search rather than the index show a plain "App" label, because the search result doesn't carry the app's state.
  • No empty headings: an app-less repository has no app label, and a heading only exists when it has rows.
  • Search result shape: local repository hits in the search results now carry app: boolean. This is an addition, so existing callers are unaffected.
  • listReposByOwner: it now honours withoutArchived. It silently ignored that option before, and the new index needs it.
  • Speed: palette ranking is still done in the browser on the index, so results are instant. The server query is the existing debounced one. I haven't measured the 50 ms target.

Where else the split shows (listed, not changed)

  • The top-nav "Apps" entry and the /apps shelf.
  • The palette's "Apps" and "New app" rows.
  • The terminal palette's repository list.

I decided to leave these alone, because /apps is a real view (live state, previews) rather than a duplicate list.

Left out: the index's per-keystroke cost for repositories beyond the first 30 still depends on the server round-trip. I didn't change #7660's open-instantly work.

Try it: press Cmd-K, type forge, and Cmd/Ctrl+Enter on a row with an app. The tests are in test/palette-repos.test.ts, and the notes are in docs/features/cmd-k-repositories.md.

commented

Parked by the crew: the check failed ("the studio, continued > /apps in the studio is the shelf, not dev's table: an app the viewer can land, never one they cannot, and no dev table markup; ?mode=dev is unchanged") and the decider saw no point in another try try 1: docs/features/cmd-k-repositories.md, src/actions/search.ts, src/core/repos.ts, src/core/search.ts (+4 more)

commented

Cmd-K now finds a repository by its name first, and a repository and its app show as one row. The change that failed the check last time is now fixed.

The failure: the shelf test ("/apps in the studio is the shelf, not dev's table") checks that /apps never contains the phrase "not serving", which is dev-table wording. My palette row for an app that isn't up used that exact phrase as its fallback label. It sits in the palette index JSON embedded in every page, so the studio shelf picked it up. The test was right, so I changed the label to "not running" in src/web/layout.ts and left the test alone. "not running" is also what the shelf row itself already shows.

What I checked: test/studio-changes.test.ts and test/palette-repos.test.ts pass (29 tests), and bun x tsc --noEmit is clean. scripts/fast.ts said the diff is too broad to name tests (it listed 1438 of 1691 files), so I ran only the two files that touch this change. The forge's full check runs on the push.

What the earlier commit does:

  • A repository's name ranks first in the palette.
  • A repository and its app collapse into one row, which carries the app's state.
  • ⌘↵ on that row opens the app.
  • test/palette-repos.test.ts covers this, and docs/features/cmd-k-repositories.md describes it.

To try it: press ⌘K or Ctrl-K on any page and type a repository's name.

commented

Parked by the crew: the check failed ("an owner's page lists an app once, not once in each vocabulary (#221) > a private app on a public repository shows its state only to its writers: a visitor gets a plain row") and the decider saw no point in another try try 1: docs/features/cmd-k-repositories.md, src/actions/search.ts, src/core/repos.ts, src/core/search.ts (+4 more)

commented

Cmd-K now finds a repository by its name first, with a repository and its app shown as one row. This push fixes the check failure in test/studio-door.test.ts. It touches two files, src/web/layout.ts and test/studio-door.test.ts.

What was wrong: the palette index gave every viewer an "App · <state>" label and an open-app link for any repository with an app. For a private app, the state and the link should reach only people who may enter it. The failing test was the one that checks a visitor gets a plain row.

The fix: the palette index in src/web/layout.ts now includes a repository's app label and open link only when the viewer may enter that app (canEnterApp). Everyone else gets the plain repository row.

Test changes:

  • The old assertion ("not running" absent) searched the whole page, including the palette index. That index legitimately lists other public apps that are not running, so I narrowed the check to the page body. The test was too broad, not the code.
  • I added an assertion that a visitor's page never contains the private app's open link (/hal/ledger/app/production/open).

Checks: bun x tsc --noEmit is clean. test/studio-door.test.ts passes, and test/palette-repos.test.ts passed alongside it before I added the new assertion. I did not run the full suite; the forge's check runs it on this push.

commented

This push fixes the test/studio.test.ts failure ("the palette lists apps first, then the other repositories (#906)"). The change is in the test, not the code. I haven't re-run the full suite here; the forge's check does that on this push.

The test still expected a repository's app to be a separate palette row in the "Apps" category. Cmd-K now shows a repository and its app as one row in "Repositories". The row carries the app as a label ("App · <state>") and a link that opens it.

I rewrote the assertions to match. The alice/notes row is in "Repositories" and carries the "App · " label and the /alice/notes/app/production/open link. The alice/palette-later row carries no app label. The check that the app's row comes before the plain repository's row is unchanged.

bun x tsc --noEmit is clean and test/studio.test.ts passes (22 tests).

commented

Parked by the crew: the check stayed red through 2 automatic tries

commented

This pull makes Cmd-K find a repository by its name, first in the results, and shows a repository and its app as one row. The branch was behind main, so I merged main in and pushed (no code change of my own). I haven't seen the forge's check verdict on that push.

  • The earlier failure: the check that failed was the one in test/studio-door.test.ts about a private app on a public repository showing its state only to its writers. It passes on this branch, before and after the merge. The branch already had the fix commits for it (2cf50df1, 1de0dd40), so the red was most likely from the older tip.
  • What I ran after the merge: the invariants gate, tsc --noEmit, and the seven test files around the palette, audience and private-app behaviour (119 tests). All passed. I did not run the full suite; the forge's check does that on the push.
  • Left out: nothing new. No dependencies were added.

To try it, open the preview for this pull, press Cmd-K, and type a repository's name. The repository should be the first row, with its app shown as a label on that row.

commented

This push fixes the two N+1 failures from the check. I re-ran test/surfaces-adversarial.test.ts and the Cmd-K tests, and they pass. I did not run the full suite.

What was wrong: the palette index (paletteIndex in src/web/layout.ts) checked app access and read app state separately for each repository. That runs on every page, so every page cost more queries as apps grew. That was the failing "/apps" and dashboard N+1 tests (35 vs 85 and 53 vs 103 statements).

The fix: the index now makes the access check once for the whole page of repositories and reads app state in one batch for the apps the viewer may enter. Private apps still show as plain rows to anyone who can't write to them, so the rows are unchanged.

Checks: bun x tsc --noEmit, bun scripts/gate.ts and bun scripts/design-lint.ts are clean. scripts/fast.ts found the diff too broad to name tests (exit 2), so I ran the affected files by hand. Those were surfaces-adversarial, palette-repos, studio and studio-door: 68 pass, 0 fail. The forge's check runs the rest.

What the pull does overall:

  • Cmd-K finds a repository by its name, with an exact name ranked above a prefix match and a prefix match above the rest.
  • Your own repositories lead within each rank.
  • A repository that runs an app shows its state and opens the app on ⌘↵.
  • Archived repositories stay out of the palette; typing the full name into search still finds them.

Left out: nothing new beyond what was already in the pull.

To try it: open the preview and press Cmd/Ctrl-K, then type a repository's name.

Changes requested — before the latest push

Name-first ranking and the one-row-per-repo index are sound, and the preview index renders. Changes needed, all in one round:

  1. FIRST: the server search leaks private apps. searchLocalRepos sets app from appsForRepos with no enterableApps check, so a visitor gets app:true (row label 'App' and the Cmd+Enter door) for a private app. The index already gates this; the search path must use the same gate (and a test for a stranger).
  2. The app door is keyboard-and-modifier only (Cmd/Ctrl+Enter). On a phone or with a screen reader it is unreachable from the palette, and the 'App · running' span is not a link. Make the app label an <a> to the app inside the row (or a second option), keeping Enter for the repository.
  3. Both doors: the TUI palette is unchanged, so a terminal user gets neither name-first ranking nor the app on the row. Do it in the terminal or state in the pull why not; README does not mention the new key.
  4. Tests: expect(shellJs).toContain("shown[at].o") is a string check that would pass with broken behaviour; test the key path in a real test or drop it. The tier-2/word-start claim in the ranking test is not asserted. In the test, the viewer lookup duplicates getUserByName via a dynamic import and a SQL query; use one static import.
  5. Cost: paletteIndex now calls appsRuntime(...).statusMany on every page render; confirm it stays well under 50 ms or serve the state lazily. Search now fetches 200 rows per keystroke.
  6. Direction: this is a feature while the pipeline items come first; keep it small. Remove the app = false default on matchOneRepo (one caller, pass it) and wrap the long lines in layout.ts/search.ts to the file's style.
commented

Pushed a06d2885 on the same branch. It covers all of @juniper's points except the terminal palette (point 3), which I left out and explain below. After the changes, tsc, the gate and design-lint are clean. These tests pass: palette-repos, surfaces-adversarial, studio and studio-door. I did not run the full suite.

  • Private-app leak: the server search (searchLocalRepos and the scoped matchOneRepo path) now sets app only when the viewer may enter the app, using the same enterableApps check as the palette index. matchOneRepo no longer defaults app; its one caller passes it. A new test shows the owner gets app: true and a signed-in stranger and an anonymous visitor get false.
  • App door: the app is now its own option under its repository row, "Open the app of owner/name". Arrow keys, touch and screen readers reach it, and Enter on the repository still goes to the repository. Cmd/Ctrl+Enter stays as a shortcut. I used a second option rather than a link inside the row because the row is already an <a> and nested links are invalid.
  • Terminal palette: unchanged. It has its own screen-and-repository ranking, and doing name-first ranking and an app option there is a separate, larger change. This is stated in docs/features/cmd-k-repositories.md. The README now mentions the new Cmd-K behaviour.
  • Tests:

- I dropped the shellJs string check. - The ranking test now asserts that a name which only contains the word ranks after every prefix match. - The viewer lookup uses one static getUserByName import. - I did not add a test of the Cmd/Ctrl+Enter key path or the new app option, because no test here runs the palette's browser script. Neither is covered by a test.

  • Cost: search now reads up to 100 repositories per query instead of 200. I did not measure the palette index's time. It adds one batched statusMany read (one query) per page render, and the earlier N+1 fix keeps the statement count flat. Serving app state lazily was not done.
  • Style: the long lines in layout.ts and search.ts are wrapped.

To try it: press Cmd/Ctrl-K on the preview and type a repository's name.

Approved

Earlier asks: private-app leak in search and index is met (enterableApps on both paths, with an owner/stranger/anonymous test). The app as its own arrow-reachable option under its repository is met, so it works by keyboard, touch and screen reader and is a plain <a> with JS-free fallback to the repository row. The README line and the feature doc are met. Name-first ranking is met and tested with 60 newer lookalikes. Not met: the terminal palette still lists repositories by its own ranking, with no app option. It is documented as a separate change, so I am not blocking on it. Two things I would do next: (1) Non-blocking: give src/tui the same name-first ranking and an open-app action, so both doors match. (2) Non-blocking: add a browser test of the Cmd/Ctrl+Enter path and the Open the app option in client.ts, which has none today.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs