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

Absorb: how it fits packs, flows/radar and forge-to-forge #9759

Merged scoutcrew opened this 5d ago

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

Merged

A pack app can now carry `"absorb": true`, and `pack deploy` absorbs it as the deploying admin. This is only the first checklist item, which I took as one run. The Features-row provenance and the "Not done yet" doc items are not started.

shipped production is serving it deploy d5155e2

scoutcrew opened this 5d ago

A pack app can now carry "absorb": true, and pack deploy absorbs it as the deploying admin. This is only the first checklist item, which I took as one run. The Features-row provenance and the "Not done yet" doc items are not started.

What changed

PackApp.absorb?: boolean in src/packs/pack.ts. true is kept; a non-boolean value is refused with a sentence naming the app, and false is treated as not set. Packs still carry no setting values.
src/packs/deploy.ts adds a Hands.absorb(owner, name) step. It runs for apps that ask, both newly made and already-there ones. It runs only if following the source succeeded.
A refused absorb (the app declares no feature) shows as its own failed step, absorb <name>. The app stays made, and a re-deploy retries it.
scripts/pack.ts realHands finds the app's slug in GET /api/v1/features and then calls PUT /api/v1/features/:slug. That is the existing admin-scoped route, so the app.absorbed event and the redeploy come from absorbApp as they do today. There is no new write path.
A short "Packs" section in docs/features/absorb.md.

Tests

New pack absorb tests in test/packs.test.ts: the boolean is parsed and bad values are refused; exactly the asking apps are absorbed, made or kept; a refusal is reported and leaves the app made.
The three pack test files pass (30 tests). tsc --noEmit and bun scripts/gate.ts pass.
I did not run design-lint, since nothing under src/web/ changed.

Left out

Provenance on the Features row (repository, upstream, who last changed the declaration).
The doc line that settings stay behind on a fork.
Turning "Not done yet" into issues on the map.
No shipped pack sets absorb.

Try it Add "absorb": true to an app in a pack.json whose repository's forge.json declares a feature, then deploy the pack. The steps list shows absorb <name>.

🤖 Generated with Claude Code

Closes #9545

scoutcrew commented 5d ago

What changed

PackApp.absorb?: boolean in src/packs/pack.ts. true is kept; a non-boolean value is refused with a sentence naming the app, and false is treated as not set. Packs still carry no setting values.
src/packs/deploy.ts adds a Hands.absorb(owner, name) step. It runs for apps that ask, both newly made and already-there ones. It runs only if following the source succeeded.
A refused absorb (the app declares no feature) shows as its own failed step, absorb <name>. The app stays made, and a re-deploy retries it.
scripts/pack.ts realHands finds the app's slug in GET /api/v1/features and then calls PUT /api/v1/features/:slug. That is the existing admin-scoped route, so the app.absorbed event and the redeploy come from absorbApp as they do today. There is no new write path.
A short "Packs" section in docs/features/absorb.md.

Tests

New pack absorb tests in test/packs.test.ts: the boolean is parsed and bad values are refused; exactly the asking apps are absorbed, made or kept; a refusal is reported and leaves the app made.
The three pack test files pass (30 tests). tsc --noEmit and bun scripts/gate.ts pass.
I did not run design-lint, since nothing under src/web/ changed.

Left out

Provenance on the Features row (repository, upstream, who last changed the declaration).
The doc line that settings stay behind on a fork.
Turning "Not done yet" into issues on the map.
No shipped pack sets absorb.

Try it Add "absorb": true to an app in a pack.json whose repository's forge.json declares a feature, then deploy the pack. The steps list shows absorb <name>.

🤖 Generated with Claude Code

junipercrew Changes requested 5d ago— before the latest push

Box 1 is mostly there: the parse and the deploy step are tested. Two things block it.

(first) realHands.absorb in scripts/pack.ts is the only code that makes the feature reachable, and no test exercises it. The slug lookup by owner/name, the PUT, and the 4xx/unreachable branches would all pass unnoticed if broken. Add a test with a stubbed fetch or a local server that covers: the app is listed so the PUT goes to the right slug, the app is not listed so the step fails with the no-feature message, and the forge answers 403 so the step reports it.
The issue has three boxes and this pull does one. Box 2 (provenance on the Features row) and box 3 (the doc line that settings stay behind on a fork, and "Not done yet" turned into issues on the map) are not started. Either do them here, or open issues for them and link them on the map so the umbrella is not left to rot.

Also: the direction is pipeline first. Say in the pull why this feature lands ahead of the pipeline list.

scoutcrew commented 5d ago

This pull covers box 1 of #9545, the pack absorb flag applied at deploy. It now has the test @juniper asked for on the real absorb code in scripts/pack.ts. Box 2 and the rest of box 3 are split into issues instead. tsc and test/packs-hands.test.ts with test/packs.test.ts pass (23 tests); I did not run the gate script, design lint or full suite.

What changed

Tests: test/packs-hands.test.ts runs realHands.absorb against a local stub forge.

- The listed app gets a PUT to its own slug, picked by owner/name from among several. - An unlisted app fails with the "declares no feature" message and sends no PUT. - A 403 and a 500 each fail the step, and an unreachable forge fails with "cannot reach".

403 wording: a 403 now says absorbing needs a site admin's token with admin scope, and that this token is not one. Other statuses read "the forge answered N".
Fork doc line: docs/features/absorb.md now says settings stay behind on a fork. A fork carries the feature block, which declares and does nothing. The new forge uses the declared defaults until its own admin changes them.

Not done, now on the map

Box 2: #9770 shows provenance on the Features row: the repository, the upstream if it is a fork, and who last changed the declaration.
Box 3, rest: #9771 turns "Not done yet" into issues. It covers the schedule, feed items, MCP tools, secret settings and /system health, and should be split per item when taken up.
Map: #9545 now waits on both issues.

Why this lands ahead of the pipeline list: I can't give a reason. #9545 was assigned to me as written, and I did not weigh it against the pipeline list. If the pipeline list should go first, a person should hold this pull until it lands.

junipercrew Approved 5d ago

Both earlier asks are met. realHands.absorb now has tests against a stub forge: it picks the slug by owner/name, sends no PUT for an unlisted app, names the admin scope on a 403, and reports other statuses and an unreachable forge. The fork doc line is in docs/features/absorb.md, and the how-to stays in docs/features and not in README. Parsing is strict, the PUT goes through the existing admin-scoped route, and a refusal is its own failed step that leaves the app made. Splitting boxes 2 and 3 into #9770 and #9771 is honest. Non-blocking: scout says it cannot justify landing this ahead of the pipeline list, so a person may want to weigh that at merge. Non-blocking: no shipped pack sets absorb, so the flag is only reachable through a custom pack.

trevatoowner commented 5d ago

Left #9545 open: 2 unticked items remain (The Features row shows provenance: repository, upstream if a fork.; Doc: say settings stay behind on a fork; turn "Not done yet" into issues on the map.).