Report the deployment artifacts a project declares - #147
Conversation
Groundwork for saying an app has no server-side runtime, and specifically for not saying it wrongly. The negative is the dangerous claim. A serverless handler this extractor cannot parse produces no endpoint, and an empty endpoint list is indistinguishable from an app that genuinely has no server. A consumer classifying on that basis would tell the owner of an unparsed Netlify function that there is nothing to protect — and nothing anywhere would raise. Note that the completeness flag does not help: `importsComplete` says the IMPORT INVENTORY is complete, not that entry-point recognition is, so an unrecognised framework leaves every coverage counter clean over a codebase full of server code. So this reports what is PRESENT: a `vercel.json`, a `netlify/functions` directory, a `wrangler.toml`, a Pages `_worker.js`, `supabase/functions`, a root `functions/` or `api/` directory. Each finding names the file or directory that proved it, because a classification a consumer cannot explain is one it should not act on. Two judgements worth stating. A root `functions/` directory is Cloudflare Pages Functions, Firebase, or a Deno layout depending on the platform, and nothing in the repository reliably distinguishes them — so it is reported as `root-functions-directory` rather than attributed to a provider that may not be involved. And a platform directory holding no source file is scaffolding, not a deployment: counting it would make every project that once considered serverless look like it ships it. Cheap by construction — a handful of stats at known paths, no walking — and fail-open: an unreadable project yields an empty list. The map says in its notes what that empty list does and does not mean, since "we recognized none" and "this app has no server" are the two readings that must not be conflated. A definitive claim needs deployment or build attestation, which source analysis cannot supply. Additive, so still version 3. Mutation-checked: dropping the empty-directory guard and removing the one-level descent each fail their own assertions and nothing else. Full suite 1155 passed, typecheck and the capability contract clean.
|
Introduces clear deployment artifact detection with comprehensive evidence reporting. 🎯 Quality: 100% Elite · 📦 Size: Medium 📈 This month: Your 86th PR — above team average · Averaging Excellent |
Three review findings, and one of my own tests turned out to be passing for the wrong reason.
**The caveat was missing from the only case that needs it.** The note was pushed only when a shape was
found, so a map with `deploymentShapes: []` — the one state a consumer could read as "static" — carried no
serialized warning at all. Type documentation does not travel to a JSON reader. The note is now
unconditional, with the empty case spelling out why the absence is unreliable: an unparsed handler produces
no endpoint, and entry-point recognition has no completeness flag (`importsComplete` covers the import
inventory only).
**An artifact that resolves outside the project is no longer evidence.** `statSync` follows symlinks, so a
linked `api/` or `vercel.json` pointing at a sibling workspace became this project's deployment evidence —
the map would describe a runtime belonging to different code. Same boundary rule the source walk applies,
with the same `followOutside` opt-out.
**Findings now carry their strength**, so a classifier cannot read more into one than it says:
config the project DECLARES a deployment (vercel.json, wrangler.toml, _worker.js)
provider-directory a provider-specific function directory holding real source
layout an ordinary application folder that MIGHT hold functions (api/, functions/)
`api/client.ts` is a normal front-end folder and `api/handler.ts` is a platform function; the directory
name is identical. Ranking it in the data rather than in a comment is what stops the next layer concluding
a server runtime from a folder name — it may use `layout` to stay undecided, and no more.
**And the correction.** Mutation-testing the boundary refusal failed two of three symlink tests: the nested
one stayed green. `readdirSync(withFileTypes)` classifies a symlink as neither file nor directory, so a
linked entry can never satisfy the source test and the per-hop boundary checks I had added there were
unreachable. They are gone, the property is now asserted for the reason it actually holds, and the accepted
cost — a legitimate in-project link inside a provider directory not counting — is written down.
17 tests here, full suite 1160 passed, typecheck and capability contract clean. Mutation-checked: making
the note conditional, removing the boundary refusal, and mislabelling a layout folder as `config` each fail
their own assertions and nothing else.
|
All three addressed in P1 — the caveat was missing from the only case that needs it. Exactly right, and the way it was wrong is P2 — escaping artifacts refused. Same boundary rule as Your third point is now encoded in the data, not a comment. Each finding carries its strength: So the step-2 classifier consumes the distinction instead of re-deriving it, and a JSON reader sees it too. The correction. Mutation-testing the boundary refusal failed two of the three symlink tests — the nested 17 tests here, full suite 1160, typecheck and capability contract clean. Mutation-checked: making the note |
`DeploymentShape` existed twice — structurally identical in `sources.ts` and `types.ts` — so the evidence vocabulary had two definitions and nothing comparing them. That is the drift this branch spent its review cycles closing everywhere else, left sitting in the type layer. `types.ts` owns it, since it is the document's contract, and the union is named (`DeploymentEvidence`) rather than inlined so the detector and the schema cannot disagree about which strengths exist. `sources.ts` imports both. `DeploymentScanOptions` stays where it is — it describes an argument to a scan, not a field in the document. Type-only change: full suite 1160 passed, typecheck and capability contract clean.
|
Done in One thing I changed beyond the letter of the note: the evidence union is named (
Type-only change: full suite 1160 passed, typecheck and capability contract clean. |
|
/review |
2 participants
Step 1 of ENG-3630 — the prerequisite before any "this app has no server" classification exists. No
classification here, only the evidence one would need.
Why the evidence layer comes first
The negative is the dangerous claim. A serverless handler this extractor cannot parse produces no
endpoint, and an empty endpoint list is indistinguishable from an app that genuinely has no server. A
consumer classifying on that basis would tell the owner of an unparsed Netlify function that there is
nothing to protect, and nothing anywhere would raise.
The completeness flag does not rescue it either:
importsCompletesays the import inventory iscomplete, not that entry-point recognition is. An unrecognised framework leaves every coverage counter
clean over a codebase full of server code.
What lands
deploymentShapes— artifacts that are present, each with the file or directory that proved it:vercelvercel.jsonnetlifynetlify.tomlcloudflare-workerswrangler.toml/.jsonc/.jsoncloudflare-pages-advanced_worker.js/_worker.tsnetlify-functionsnetlify/functions,netlify/edge-functionssupabase-functionssupabase/functionsroot-functions-directoryfunctions/root-api-directoryapi/Two judgements worth review:
functions/directory is not attributed to a provider. It is Cloudflare Pages Functions,Firebase, or a Deno layout depending on where it deploys, and nothing in the repository reliably says
which. Naming the shape honestly beats naming a provider that may not be involved.
project that once considered serverless looks like it ships it. Directory checks descend one level, which
is what covers the per-function layout (
netlify/functions/hello/index.ts).root-api-directorystays separate fromverceldeliberately: a project can use one without the other,and Next's
pages/api/app/apiare already read by the endpoint walk.What it refuses to claim
An empty list means "no known artifact was recognized" — never "this app has no server-side runtime". The
map says so in its notes, where a consumer reading the document will see it. A definitive claim needs
deployment or build attestation, which source analysis cannot supply, and that wording is reserved for the
classification layer in ENG-3630.
Cheap by construction (a handful of stats at known paths, no walking) and fail-open (an unreadable project
yields an empty list). Additive, so still version 3 — a v3 reader that ignores the field keeps behaving
correctly.
Verification
Full suite 1155 passed, 1 skipped; typecheck and the capability contract clean. Mutation-checked: dropping
the empty-directory guard fails the two refusal tests, and removing the one-level descent fails the
per-function-layout test — each and nothing else.