Skip to content

feat(mcp,skills): sapiom_dev_app_publish + teach App Links in the preview skill (SAP-2922) - #722

Merged
gwitwer merged 3 commits into
mainfrom
feat/SAP-2922
Aug 27, 2026
Merged

feat(mcp,skills): sapiom_dev_app_publish + teach App Links in the preview skill (SAP-2922)#722
gwitwer merged 3 commits into
mainfrom
feat/SAP-2922

Conversation

@gwitwer

@gwitwer gwitwer commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Closes SAP-2922 — https://linear.app/sapiom/issue/SAP-2922

Why

Studio's coding agent could only reach a sandbox preview URL, which dies with the sandbox's ttl — and nothing on that surface told it a durable alternative exists. Asked for "a link I can share with my team", it handed over a URL that would be dead by the time anyone clicked it. The backend App Links REST API has been live on prod and dev; this closes the gap between it and the surface the agent is actually taught to use.

What

sapiom_dev_app_publish (packages/mcp/src/tools/app-publish.ts) — the sibling of sapiom_dev_sandbox_preview. Reads the same sapiom.json type: "sandbox" resource (source dir, start, port, optional build/env), collects the source as a text-only file map, and publishes it as an App Link via the backend in contract order:

  1. POST /v1/app-links (upsert on slug)
  2. PUT /v1/app-links/{id}/bundle
  3. POST /v1/app-links/{id}/publish

Auth is the cached sapiom_authenticate credential as x-api-key. Returns { summary, url, appLinkId, bundleSha256, manifest }. Nothing is provisioned here — no gateway call, no sandbox, no Blaxel; the wake happens later, on first visit, via app-host.

Notable choices:

  • Binary rejection is local. A non-UTF-8 file is named and refused before the first HTTP call (strict TextDecoder, not the lossy utf8 read collectDirFiles uses), so a binary in the source dir costs no round trip and creates no half-published link.
  • sapiom.json is never bundled — it is project config, and its env block holds the app's own secrets, which a static start command would otherwise serve to every visitor of a public link. The resource's env travels separately, on the upsert; its tier/ttl deliberately do not.
  • POST /publish is sent with an empty body, matching the controller (it activates whatever bundle was last uploaded). The response's bundleSha256 is preferred over the uploaded one, so a racing publisher on the same slug is visible rather than reported as ours.
  • Wire codes → actionable errors, each stating that nothing was published: BUNDLE_BINARY_FILE (names the file), BUNDLE_TOO_LARGE (quotes both sizes), PUBLIC_CONFIRM_REQUIRED, PUBLIC_SPEND_CAP_REQUIRED, APP_LINK_MANAGEMENT_PERMISSION_REQUIRED (drop the management fields — not a retry), 401, 403 (names org.app_links.publish), plus NETWORK and an HTTP_<status> fallback.

Teaching, not just capability. Both copies of the sapiom-sandbox-preview skill (byte-identical, guarded by the sync test) now say plainly that the preview URL expires with the ttl, and add a "Make it durable / share it" section routing "share this" / "send this to my team" / "a permanent link" / "keep it alive" / "my link died" to the new tool — carrying the five facts an agent otherwise gets wrong: wake-on-demand cold start, org-scoped by default (public needs confirmPublic + dailySpendCapUsd, so ask first), republish in place on the same slug, text-only bundles, ~10 MiB cap. The frontmatter description picks up the durability triggers.

Plus the nice-to-have: a successful sapiom_dev_sandbox_preview result now carries the hint on the result, not only in the tool description — the agent reads it at the moment it is about to hand the URL over.

Tests

  • packages/mcp/src/tools/app-publish.test.ts (24 tests, backend mocked at fetch): the three-call order and x-api-key, the durability wording the routing decision hangs on, every wire-code mapping, binary rejection with no HTTP call, missing/empty source dir, republish keeping the URL with a new sha, base-path preservation on a custom apiURL.
  • skill-sync.test.ts gains content guards — byte-identity cannot stop two identically wrong copies.
  • analytics.e2e.test.ts tool roster updated.

Verification

pnpm --filter '@sapiom/mcp...' build   ✓
pnpm --filter @sapiom/mcp typecheck    ✓
pnpm --filter @sapiom/mcp lint         ✓  (agent-core lint ✓)
packages/mcp        vitest run         ✓  15 files, 127 tests
agent-core          skill-sync.test.ts ✓  15 tests
pnpm terminology:check                 ✓
pnpm provider-copy:check               ✓

Not verified here: a live publish against api.sapiom.dev (needs a funded org credential) — the acceptance criteria that need real HTTP are left for manual/dev-target checking.

🤖 Generated with Claude Code

https://claude.ai/code/session_014AysBPbfr9uLL7inXTr9Np

…view skill (SAP-2922)

Studio's coding agent could only reach a sandbox preview URL, which dies with
the sandbox's ttl — and nothing on that surface told it a durable alternative
exists. Asked for "a link I can share with my team", it handed over a URL that
would be dead by the time anyone clicked it.

Add `sapiom_dev_app_publish` alongside the `sapiom_dev_sandbox_*` tools. It
reads the SAME `sapiom.json` sandbox resource (source dir, start, port,
optional build/env) and publishes it as an App Link — a durable
https://apps.sapiom.ai/{org}/{slug} address — via the backend publish API in
contract order: POST /v1/app-links (upsert on slug) → PUT /{id}/bundle →
POST /{id}/publish, authed with the cached sapiom_authenticate credential as
x-api-key. Nothing is provisioned here: no gateway call, no sandbox, no Blaxel.
The wake happens later, on first visit, via app-host.

Bundles are UTF-8 text only, so a binary is rejected by name locally, before
any HTTP call — an agent that has to fix its input should not pay for an upload
to learn that. Backend wire codes become errors that say what to change:
BUNDLE_BINARY_FILE (names the file), BUNDLE_TOO_LARGE (quotes both sizes),
PUBLIC_CONFIRM_REQUIRED, PUBLIC_SPEND_CAP_REQUIRED,
APP_LINK_MANAGEMENT_PERMISSION_REQUIRED (drop the management fields, not a
retry), 401, and 403 (names org.app_links.publish). Every branch states that
nothing was published, so the next move is never a blind retry.

Teaching, not just capability: both copies of the sapiom-sandbox-preview skill
(byte-identical, guarded by the sync test) now say plainly that the preview URL
expires with the ttl, and route "share this" / "permanent link" / "keep it
alive" / "my link died" to the new tool, carrying the five facts an agent
otherwise gets wrong — cold start, org-scoped by default, republish in place,
text-only, ~10 MiB. The frontmatter triggers pick up durability asks, and a
successful preview result now carries the hint at the moment the agent is about
to hand the URL over.

Refs: SAP-2922
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014AysBPbfr9uLL7inXTr9Np
@github-actions

Copy link
Copy Markdown
Contributor

Review — PR #722 (sapiom_dev_app_publish + App Links in the preview skill)

🔒 CONFIDENTIALITY — internal backend detail in a published module header

packages/mcp/src/tools/app-publish.ts:14-21. Three things in the header comment of a file
in a public repo (and whose JSDoc survives into dist/*.d.ts):

  • the `org.app_links.publish` delegation gap that affects workflow per-run `sat_` tokens (SAP-2882)
    — an anonymous reader learns that Sapiom's per-run workflow tokens are missing a
    permission delegation, plus the internal token prefix. That is an internal
    authorization limitation, not something a consumer of this tool needs.
  • The three REST calls are `plans/app-links/interfaces.md` §3 — cited three times here
    and again in app-publish.test.ts:14 / the wire error mapping (interfaces §3) describe
    block, as the normative contract. plans/ does not exist in this repo; it is a pointer
    into a private planning doc a reader cannot resolve.
  • lets app-host wake it on the first visit — internal service name; nothing else in
    packages/ mentions it.

Rewrite to the observable contract: "the App Links REST API, in order: POST /v1/app-links →
PUT …/bundle → POST …/publish", drop the SAP-2882 sentence entirely (say only "auth is the
user's cached credential as x-api-key"), and say "the hosting layer" instead of app-host.
Blaxel in the same block is fine — it is pinned public vocabulary.

🐛 "Nothing was published" is false for every failure after step 1 — and untested

packages/mcp/src/tools/app-publish.ts:1262-1341. POST /v1/app-links creates the app
link. If PUT …/bundle or POST …/publish then fails, the link exists — empty, with
whatever visibility was requested — but every mapped error asserts Nothing was published.
and offers no cleanup path.

Concrete: publish a 12 MiB source dir with visibility: "public", confirmPublic: true.
Step 1 creates a public app link. Step 2 returns BUNDLE_TOO_LARGE. The agent is told
"Bundle rejected: … Nothing was published." and reports that to the user — while a public,
bundle-less app link now sits on that slug. Same for a backend-side BUNDLE_BINARY_FILE,
and for a 401/403 on step 2 or 3.

The tests mask this: mockBackendError uses mockResolvedValue (all calls), so every
wire-mapping case fires on call #1 and the created-then-failed path is never exercised. Fix
the copy to be step-aware (step is already threaded in as where — use it: "The app link
was created but no bundle is active; republish to finish it"), and add a test where call #1
succeeds and call #2 fails.

Same correction is needed in .changeset/mcp-app-publish-tool.md ("Every one says nothing
was published") — that line ships into @sapiom/mcp's CHANGELOG permanently.

🐛 A raced publish returns another publisher's sha next to our manifest

app-publish.ts:1099-1107. The comment claims a mismatch "is visible rather than silently
reported as ours", but nothing compares published.bundleSha256 to bundle.bundleSha256.
When they differ, the result is bundleSha256 from the other publisher paired with
manifest (fileCount, bytes, envKeys) from our upload, and summary says
Published "Dash" … (N files) — describing a bundle that is not live. The test at
app-publish.test.ts:577 locks this in as correct. Either compare and surface a warning
field, or drop the comment's claim and the "racing publisher" rationale in the PR body.

🐛 Symlinks are followed out of the source directory

app-publish.ts:1157-1169. statSync follows links, so report.txt -> ../../secrets.txt
inside the source dir is read and uploaded — to a URL that may be visibility: "public". A
self-referential link (a -> .) recurses until the stack blows. collectDirFiles has the
same shape, but its destination is a private sandbox; here the blast radius is a public page.
Use lstatSync and skip symlinks (or resolve and reject anything outside root).

Nits

  • app-publish.ts:1233-1236: a 200 with a non-JSON body makes safeParse return the raw
    string, so link.id is undefined and step 2 hits /v1/app-links/undefined/bundle.
    Validate that step 1 returned an object with a string id.
  • The description promises a 10 MiB cap but only the binary check runs locally; a size
    check is the same one-line saving and avoids the dangling-link case above.
  • ALWAYS_SKIP.has(CONFIG_FILE) is applied at every depth, so a nested sapiom.json that is
    legitimately app content is silently dropped.

Verdict: request changes — the confidentiality leak in the module header and the
inaccurate "nothing was published" copy (which also ships in the changeset) should be fixed
before merge.

…ublish (SAP-2922)

Review of #722. Five real problems, all in the same area: what the tool tells
the agent about the world after something goes wrong.

Confidentiality. The module header of a file in a public, npm-published package
named an internal authorization gap and its ticket, the per-run token prefix, a
private planning-doc section as the normative contract, and an internal service
name. Rewritten to the observable contract; the `interfaces §3` citations in the
test file go too.

"Nothing was published" was false for every failure after the first call. The
upsert CREATES the link, so a BUNDLE_TOO_LARGE on step 2 left a real link —
possibly public — on that slug while telling the agent nothing had happened, and
offering no way to finish or clean up. Error copy is now step-aware: only a
step-1 failure may claim nothing was created; later ones say the link exists
with no active bundle and to publish the same slug again. The tests masked this
— `mockResolvedValue` fired every case on call #1 — so the mock now plays the
happy responses up to the failing step, and the created-then-failed path is
covered at both the bundle and activate steps.

Symlinks were followed out of the source directory. `statSync` follows links, so
`report.txt -> ../../secrets` would have been read and published, and this
destination can be a public URL, not a private sandbox. Now `lstatSync` and skip
— which also removes the `a -> .` infinite recursion.

The raced-sha comment claimed a mismatch was "visible" while nothing compared
the two shas: the result paired another publisher's live sha with OUR manifest
and file count. Now compared, with a `warning` field and a summary that stops
claiming to describe the live bundle.

Also: the 10 MiB cap is checked locally, measured exactly as the server measures
it (canonical sorted-key JSON byte length), so the most likely cause of a
dangling link never reaches the network; a 2xx body that is not an app link is
refused instead of addressing `/v1/app-links/undefined/bundle`; and
`sapiom.json` is skipped only at the bundle root, since a nested one is ordinary
app content.

The changeset copy carried the same false "nothing was published" claim into
@sapiom/mcp's permanent CHANGELOG — corrected there as well.

Refs: SAP-2922

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014AysBPbfr9uLL7inXTr9Np
@gwitwer

gwitwer commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Worked through all of these. Five fixes pushed in bd87a83; one finding I'm partly pushing back on.

🔒 Confidentiality — fixed. Confirmed the repo is public (isPrivate: false), so this is real. Header rewritten to the observable contract: the SAP-2882 sentence and the sat_ prefix are gone entirely (now just "auth is the cached sapiom_authenticate credential as x-api-key"), plans/app-links/interfaces.md §3 is now "the App Links REST API, in order: …", and app-host is "the hosting layer". Also removed the §3 citations in app-publish.test.ts (header, the order-test label, and the describe block) and a leftover // ─── Wire shapes (interfaces §1, §3) banner. Blaxel left as-is per your note.

🐛 "Nothing was published" after step 1 — fixed, and you were right that the tests masked it. The copy is now step-aware via a PublishStep (create | bundle | publish) threaded through the transport, and a single aftermath(step, slug) that every branch appends:

  • create"Nothing was created or published."
  • bundle / publishThe "dash" app link EXISTS but has no new bundle active. Fix the above and publish the same slug again to finish it (or delete the link if you no longer want it).

mockBackendError is replaced by mockBackendErrorAt(step, body, status), which plays the happy responses up to the failing step. Each wire-mapping case now declares its realistic step (BUNDLE_* at bundle, the rest at create) and asserts the aftermath sentence, plus three new tests: a 409 at activate (asserts all three calls fired, the "EXISTS" copy, and not.toContain("Nothing was")), a mid-flow NETWORK failure, and the step-1 NETWORK case keeping the "nothing was created" claim. .changeset/mcp-app-publish-tool.md is rewritten — good catch that it ships permanently.

🐛 Raced sha — fixed. You're right, the comment described a comparison that wasn't there, and I wrote that comment. Now: raced = activeSha !== bundle.bundleSha256, and when it's true the result carries a warning naming both shas and stating that manifest describes the uploaded bundle, not the live one; the summary swaps the file count for "a bundle published by something else — see warning". Two tests cover both branches.

🐛 Symlinks — fixed. statSynclstatSync, symlinks skipped, and non-file entries ignored. The blast-radius argument is the deciding one: collectDirFiles writes into a private sandbox, this can end up behind a public URL. The a -> . recursion goes away with it. Test creates both a link out of the tree (to a real file with SECRET in it) and a self-referential one, and asserts only index.html is uploaded.

Nits — two fixed, one pushed back on:

  • Non-JSON 2xx → fixed. asAppLink() requires a string id and url, else UNEXPECTED_RESPONSE ("the publish may or may not have taken effect"). Test asserts exactly one fetch call, so /v1/app-links/undefined/bundle is unreachable.
  • Local size check → fixed, and worth it for the reason you give. Measured exactly as the backend does — Buffer.byteLength(JSON.stringify({ files: sorted }), "utf8") against the canonical sorted-key JSON — so the local check can't disagree with the server it stands in for. Test publishes a 10 MiB + 1 file and asserts zero fetch calls.
  • Nested sapiom.json → fixed. Root-only now (depth === 0); node_modules/.git stay every-depth. Test keeps fixtures/sapiom.json and drops the project's own.

The one I'm not doing: adding a cleanup/rollback path (deleting the link when step 2 or 3 fails). The failure is now accurately reported and the recovery is named in the copy — republish the same slug, since POST /v1/app-links is an upsert, so finishing a half-published link is the same call the agent already knows. Auto-deleting on failure would be worse: on a republish of an existing app it would tear down a link that was serving fine before this call. Out of scope for the ticket, and I don't think it's the right behavior anyway.

Verification: packages/mcp full vitest suite 134/134 (31 in app-publish.test.ts), agent-core skill-sync.test.ts 15/15, both skill copies still byte-identical, build + typecheck + lint clean, terminology:check and provider-copy:check pass.

@github-actions

Copy link
Copy Markdown
Contributor

Follow-up review — PR #722 (delta since 6508d6b)

All four earlier findings are fixed, and correctly: the module header and test file no
longer cite plans/…interfaces.md, SAP-2882, sat_ or app-host; error copy is
step-aware with mockBackendErrorAt exercising the created-then-failed path; the race is
surfaced as warning + summary; symlinks are skipped and covered by a test. The changeset
rewrite is accurate. Three new items, all introduced by this push.

🐛 A symlinked source directory now fails as "not found"

packages/mcp/src/tools/app-publish.ts:357 — the root check moved from statSync to
lstatSync. Only entries inside the tree needed that; the root is the path the user
explicitly pointed at. With source.path: "web" where web is a symlink to a real
directory (common in workspaces), lstatSync(root).isDirectory() is false and the tool
throws NO_SOURCE_DIR: Source directory not found: …/web, telling the user to fix a
source.path that is correct. Keep statSync for the root, lstatSync in walk.

🐛 The root-only sapiom.json skip misfires when source.path isn't .

app-publish.ts:369depth === 0 && entry === CONFIG_FILE skips the bundle root's
sapiom.json, which is the project config only when source.path is .. With
source.path: "web", a legitimate web/sapiom.json app asset is silently dropped, while
the actual project config was never in the tree. Compare childAbs to
path.resolve(projectDir, CONFIG_FILE) instead of using depth.

🐛 asAppLink on step 3 can fail a publish that succeeded

app-publish.ts:248 — the nit was about step 1, where id addresses the next call. Step 3
has no such need: if the activate route answers 204/{ok:true} rather than echoing the
link, the tool now returns UNEXPECTED_RESPONSE ("may or may not have taken effect") for a
publish that fully succeeded, and no test covers a non-link publish body. Fall back to the
step-1 link for url/id when the publish body isn't an app link.

Verdict: approve with comments — the symlinked-root regression is worth fixing before
merge; the other two are small. (Tests not run here: the sandbox declined the vitest
invocation.)

…(SAP-2922)

Follow-up review of #722. Three regressions from the previous push, each a case
of a check being stricter than what it was protecting.

The source root. Moving to `lstatSync` was right for entries inside the tree and
wrong for the root: a `source.path` pointing at a symlinked directory — ordinary
in a workspace — reported `NO_SOURCE_DIR` and told the user to fix a path that
was correct. The root is the path the user explicitly named, so it is resolved
with `statSync`; only the walk refuses links.

The project-config skip. `depth === 0 && entry === CONFIG_FILE` skips the BUNDLE
root's sapiom.json, but that file is config only at `<projectDir>/sapiom.json`.
Under `source.path: "web"` the real config was never in the tree, while a
legitimate `web/sapiom.json` app asset was silently dropped. Now matched by
absolute path against the project config — which is also what the rule always
meant, and needs no depth counter.

The publish response. `asAppLink` was written for step 1, where the `id`
addresses the next two calls; applying it to step 3 turned a 204 or a bare
`{ok:true}` — a publish that fully succeeded — into UNEXPECTED_RESPONSE "may or
may not have taken effect". Split into `requireAppLink` (step 1, must be a link,
attributed to the create step) and `asAppLink` returning null, with step 3
falling back to the step-1 link for url/id. A fallback also means no active sha
to compare, so the race warning correctly stays silent.

Refs: SAP-2922

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014AysBPbfr9uLL7inXTr9Np
@gwitwer

gwitwer commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

All three are correct, and all three were regressions from my previous push. Fixed in d1e3bf1 — no pushback this round.

🐛 Symlinked source root — fixed. Right diagnosis: the move to lstatSync was correct for entries inside the tree and wrong for the root. The root is the path the user explicitly named, so it's back to statSync; only walk refuses links. Comment says why, so it doesn't get "tidied" back:

// `statSync`, not `lstatSync`: the root is the path the user named, and a
// symlinked source directory must not be reported as a missing one.
if (!existsSync(root) || !statSync(root).isDirectory()) {

Test covers source.path: "web" where web is a symlink to a real directory, and asserts the file inside it is bundled.

🐛 Root-only skip misfires under a non-. source.path — fixed, and your version is what the rule always meant. Replaced the depth counter with an absolute-path match against path.resolve(projectDir, CONFIG_FILE), so the depth parameter is gone from walk entirely. Under source.path: "web" the project config was never in the tree and web/sapiom.json now survives; with source.path: "." the root config is still skipped, since root === projectDir makes the paths identical. Test asserts web/sapiom.json arrives as sapiom.json in the file map.

🐛 asAppLink on step 3 — fixed. You're right that the strictness was inherited from a requirement step 3 doesn't have. Split in two:

  • requireAppLink(data) — step 1 only, throws UNEXPECTED_RESPONSE (now attributed to POST /v1/app-links and carrying "Nothing was created or published.", which it wasn't before).
  • asAppLink(data) — returns null, and step 3 does asAppLink(activated) ?? link.

So a 204 or bare {ok:true} is a success that reports the step-1 url/id. One consequence worth noting: falling back means there's no server-reported active sha to compare, so raced is false and the warning correctly stays silent rather than firing on a missing field. New test covers the 204 path (asserts isError undefined, correct url/id/bundleSha256, no warning), and the existing step-1 guard test now also asserts the step and the aftermath sentence.

Verification: packages/mcp 137/137 (34 in app-publish.test.ts), build + typecheck + lint + prettier clean. Re: your sandbox note — npx vitest run from packages/mcp is what I use locally if it becomes runnable there.

@gwitwer
gwitwer merged commit 054f749 into main Aug 27, 2026
10 checks passed
@gwitwer
gwitwer deleted the feat/SAP-2922 branch August 27, 2026 10:54
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