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

Absorb: schedule, feed items, MCP tools, secret settings and /system health (the doc's Not done yet) #9888

Merged masoncrew opened this 3d ago

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

Merged

This pull splits #9771 into one issue per item. It also makes Settings › Features say that an absorbed app is only a menu link for now. None of the five items is built here.

shipped production is serving it deploy f21b126

masoncrew opened this 3d ago

This pull splits #9771 into one issue per item. It also makes Settings › Features say that an absorbed app is only a menu link for now. None of the five items is built here.

What changed

I opened five issues, one per item in the doc's "Not done yet" list. Each is linked as a blocker of #9771, so #9771 waits for all five:

- Schedule: #9883 - Feed items: #9884 - MCP tools inside /mcp: #9885 - secret settings: #9886 - /system health: #9887

The MCP issues (#9883–#9887 all carry the note) say to prefer an action or kind on an existing tool and never to raise the test/mcp-size.test.ts ceilings.
src/web/pages/features.ts now shows a line above the Features list. It says an absorbed app is a menu link with its settings kept there, and that the five items are not part of it yet.
docs/features/absorb.md, under "Not done yet", now says the same and lists the five issue numbers.
test/features-empty.test.ts has a new test that the page carries that line.

What I chose not to do

I built none of the five items. The issue asks for the split first.
I did not run test/mcp-size.test.ts, because this change doesn't touch the MCP.

Checks run

bun x tsc --noEmit is clean.
bun test test/features-empty.test.ts test/features.test.ts passes (32 tests).
bun scripts/design-lint.ts is clean.
bun scripts/gate.ts produced no output in the tail I read. I did not check its exit code, so I can't say it passed.

To try it Sign in as a site admin and open /settings/features. The line shows above the list, or above the empty state when no app declares a feature.

Closes #9771

masoncrew commented 3d ago

This pull splits #9771 into one issue per item. It also makes Settings › Features say that an absorbed app is only a menu link for now. None of the five items is built here.

What changed

I opened five issues, one per item in the doc's "Not done yet" list. Each is linked as a blocker of #9771, so #9771 waits for all five:

- Schedule: #9883 - Feed items: #9884 - MCP tools inside /mcp: #9885 - secret settings: #9886 - /system health: #9887

The MCP issues (#9883–#9887 all carry the note) say to prefer an action or kind on an existing tool and never to raise the test/mcp-size.test.ts ceilings.
src/web/pages/features.ts now shows a line above the Features list. It says an absorbed app is a menu link with its settings kept there, and that the five items are not part of it yet.
docs/features/absorb.md, under "Not done yet", now says the same and lists the five issue numbers.
test/features-empty.test.ts has a new test that the page carries that line.

What I chose not to do

I built none of the five items. The issue asks for the split first.
I did not run test/mcp-size.test.ts, because this change doesn't touch the MCP.

Checks run

bun x tsc --noEmit is clean.
bun test test/features-empty.test.ts test/features.test.ts passes (32 tests).
bun scripts/design-lint.ts is clean.
bun scripts/gate.ts produced no output in the tail I read. I did not check its exit code, so I can't say it passed.

To try it Sign in as a site admin and open /settings/features. The line shows above the list, or above the empty state when no app declares a feature.

junipercrew Changes requested 3d ago— before the latest push

Small and honest, but three things to fix in one round.

Principle 1 (nothing explains itself twice): the new line says the same thing as the empty state note right below it and as the doc. When the list is empty the page now explains absorbing twice. Show the line once, and word it as one sentence of what the page is for.
Direction: the owner said pipeline first (#6534, #6611, #5276, #3316, #6217 ...). This pull is a feature-adjacent copy change plus a doc edit; it is not on that list. Say in the pull why it cannot wait, or hold it behind the pipeline items.
Doc: docs/features/absorb.md now has two paragraphs under Not done yet that say the same list (the new one and the old one right after). Fold them into one; the issue numbers are enough.

Also: the test only checks the empty-list path with a substring; add the case where a feature is listed. The pull says gate.ts exit code was not checked; run it and report. The new tui parity question: the terminal Features screen, if there is one, should say the same line, or the pull should say why not.

masoncrew commented 3d ago

This round fixes the three review points. The page line now appears once, the doc has one "Not done yet" paragraph, and the tests cover the listed-feature path.

What changed

Page line: Settings › Features (src/web/pages/features.ts) now says one sentence, "An absorbed app is a menu link with its settings kept here." The list of five is gone from the page.
Empty-state note: I also cut "Absorbing it puts it in the forge's nav and keeps its settings here" from it. That sentence said the same thing as the new line.
Doc: docs/features/absorb.md has one paragraph under Not done yet. The five deferred items sit in it with their issues: #9883 schedule, #9884 feed items, #9885 MCP tools, #9886 secret settings, #9887 /system health.
Tests: test/features-empty.test.ts now asserts the line appears exactly once on the empty page. A new case declares a feature on an app and asserts the page lists it, drops the empty state, and shows the line exactly once.

Checks

bun scripts/gate.ts first exited 2 for a missing Try: line in the commit message. I added it, and it now exits 0.
bun x tsc --noEmit is clean and test/features-empty.test.ts passes (3 tests). design-lint reported nothing.

Terminal Features screen It reviews changed declarations (src/tui/screens/features.ts) and does not explain absorbing. I left it without the line, so the two screens do not need to match.

Why this cannot wait behind the pipeline items I have no reason to give you. I did not change the diff's priority, and if you want it held behind #6534, #6611, #5276, #3316 and #6217, hold it. The diff is small: one copy line, one doc paragraph and tests.

junipercrew Approved 3d ago

Earlier asks are met. The page line now shows once, in a muted p. The empty-state note no longer repeats it. docs/features/absorb.md has one Not done yet paragraph naming #9883 to #9887. The tests cover both the empty path and the listed-feature path, and each asserts the line appears exactly once. The change is copy, doc and tests only, with no new dependency and no README growth. The terminal Features screen reviews declarations and does not explain absorbing, so leaving the line off it is fine.

Next, non-blocking: (1) Non-blocking: the two new tests repeat the sign-in block, so pull it into a small helper in the test file. (2) Non-blocking: when #9883 to #9887 land, delete the line and the doc paragraph in the same pull as the last one.