RFC: notte project scaffolding, bundling and declarative deploys - #75
Open
giordano-lucas wants to merge 6 commits into
Open
RFC: notte project scaffolding, bundling and declarative deploys#75giordano-lucas wants to merge 6 commits into
giordano-lucas wants to merge 6 commits into
Conversation
…e deploys We have hand-rolled the same deploy framework twice — anything-api/marketplace (2,404 lines of TypeScript) and managed-auth (996 lines of Python) — and both spent their worst code on the same two problems: faking subcommands in make, and not having a bundler. This RFC proposes folding that framework into the CLI: `notte init` / `notte deploy` over a real Python package, with client-side bundling of local imports, per-environment state in a lockfile, and declarative secrets and schedules. Key findings that constrain the design: - The functions API validates uploads with RestrictedPython (`restricted=True` by default), which rejects every form of local import. Client-side bundling is the only option, not a convenience. - That same validator forbids sys/exec/compile/__import__/os, so the standard Python bundlers (stickytape, pinliner, ComPYner) cannot work — they all rely on a sys.modules prelude. The bundler must be a static flattener. - Dependencies are a fixed allowlist, so there is no dependency resolution to build — only a build-time import check. - Schedules cannot be reconciled today: POST /schedule is a clean upsert but there is no read endpoint, because FunctionResponse drops schedule_cron. Nothing is implemented. Ends with a list of backend asks ordered by how much each unblocks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
| Filename | Overview |
|---|---|
| docs/rfcs/0001-notte-project-scaffolding-and-deploy.md | Adds the complete project/deployment RFC; alias preservation, credential-to-URL coupling, and absent test coverage need clarification. |
Prompt To Fix All With AI
### Issue 1
docs/rfcs/0001-notte-project-scaffolding-and-deploy.md:317
**Aliased imports lose bindings**
Deleting a relative import such as `from .parse import parse_rows as pr` removes the alias without recreating it, while aliases are not listed as unsupported. Specify alias preservation or reject this form explicitly so the generated artifact does not fail with `NameError` at runtime.
### Issue 2
docs/rfcs/0001-notte-project-scaffolding-and-deploy.md:306
**Credential lookup is uncoupled**
The project environment supplies the deployment URL, but the proposed chain does not require the keyring label to be derived from that same URL. Define these as one coupled resolution so `notte deploy --env staging` cannot select a production or global fallback credential for the staging endpoint.
### Issue 3
docs/rfcs/0001-notte-project-scaffolding-and-deploy.md:462
**Test coverage is absent**
This changeset describes testing only as future behavior and adds no unit or integration tests. Add executable coverage for the proposed bundling and project-management contracts when their implementation lands.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "docs(rfc): propose notte project scaffol..." | Re-trigger Greptile
…eview Two spec gaps, both of which would have shipped as silent runtime failures: Aliased relative imports. The algorithm said relative import lines are deleted, so `from .parse import f as g` would drop `g` entirely and the artifact would raise NameError at run time — after passing the bundler and passing upload validation. Import lines are now replaced in place by one assignment per aliased name, and aliases join the collision set so `from .parse import clean as fetch` conflicts with a `fetch` defined elsewhere exactly as a second `def fetch` would. Credential resolution. The chain ended in a bare NOTTE_API_KEY and config.json, neither of which is tied to an endpoint, so `deploy --env staging` with a prod key exported would authenticate to staging as prod — failing closed only when the orgs happen to differ. Key and URL now resolve as one unit derived from the selected env, both endpoint-agnostic fallbacks are removed, and it fails closed with the command to fix it. This is the same class of bug marketplace-catalog.ts documents hitting from the other direction with ambient NOTTE_API_URL. Also adds the bundler's day-one golden-file cases. The alias gap was found by reading this document rather than by a test, which is the argument for listing them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Leo's review on #75 caught a real error, and he was right. The RFC argued that off-the-shelf bundlers are impossible because RestrictedPython forbids sys/exec/compile/__import__/os. That is the wrong mechanism. I verified the upload path and presented it as if it were also the runtime: workflows-lambda/worker.py:891 executes user functions with restricted=False, which takes the branch at worker.py:608 and uses plain compile(). The AST policy is never applied at execution time, so citing FORBIDDEN_CALLS proves nothing about what a deployed function can do. The conclusion survives via a different mechanism, and this is also the answer to "quitte a allow os et sys": the runner keeps __import__ bound to safe_import, which name-checks every import at run time. stickytape dies on `import tempfile` (explicitly discarded at worker.py:520) and then on `import util`, which is the one thing it exists to do. Allowing os and sys touches neither. Making it work means disabling safe_import — arbitrary imports at run time — which is a much larger decision and the only one here with a genuine security dimension. So the document now separates the two gates explicitly (AST policy is upload-only, the import allowlist is a real runtime guard with its own list), concedes the bad framing in place, and leads with the three reasons to flatten that hold regardless of any allowlist: the artifact stops being readable and breaks the diff model, stickytape disclaims itself in its own README, and adopting it reintroduces the Python-runtime dependency the Go-native recommendation exists to avoid. Also cuts the command surface from eighteen to five — init, new, deploy, check, status — with everything else moved to a deferred table carrying a reason each. Half the original list was gated on backend work that does not exist yet, and a large surface is its own cost. Backend ask 5 now requests both allowlists rather than one, since they differ. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ithout it
Deferring `pull` was wrong, and the reason given for it ("only matters for
adopting an existing tree, irrelevant to a new project") had the Notte
workflow backwards.
An org with existing functions is the normal case, not the migration case.
Functions already arrive from `sessions workflow-code`, from the Anything
API build agent, and from the console — author in the browser, then decide
you want it in git. That is `pull`, and `notte init --from-session` is a
single-function `pull` under another name, so the machinery is required
either way.
The sharper problem is that without it `deploy` is actively unsafe.
Create-vs-update reads the lock: no function_id for this env means create.
A fresh `notte init` against an org that already has `amazon_search` gets a
lock that believes nothing exists, so the first deploy creates a *second*
`amazon_search`. functions.name has no unique constraint, so the API accepts
it silently, and two functions now share a name while callers hold the id of
the one that stopped being updated.
So `pull` joins v1, and deploy gains the matching rule: refuse to create a
function whose name exists remotely but is absent from the lock, and point
at `notte pull`. `--force-create` covers the genuine second-copy case.
Also specifies what `pull` may and may not do, since bundling makes it
asymmetric: an unknown function lands as a single-file function because
that is what it is, a function already deployed from this tree is left
alone rather than having its sources overwritten by their own flattened
output, and — following marketplace — a partial run never prunes and remote
extras are reported rather than deleted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Promoting `pull` to v1 without saying how it fetches code left the most
practical question open. The shape is not obvious and is worth writing down.
There is no bulk download and no download command at all:
- GET /functions returns PaginatedResponseFunctionResponse{Items
[]FunctionResponse}, and FunctionResponse has no url field. Only
FunctionWithLinkResponse carries one, and that comes from
GET /functions/{id}.
- So each function's code costs two further requests — one for the signed
URL, one to fetch it. A full pull is ceil(N/100) + 2N, about 4,120
requests for marketplace's 2,049 functions, which it ran at concurrency 48.
- The URL is a Fernet token for Notte-managed functions, decrypted with a key
derived client-side as sha256("api_key:{k}:workflow_id:{id}:dumb")[:64].
- `notte functions download` does not exist. `functions show` already calls
FunctionDownloadUrl, prints the metadata and discards the URL, which is why
marketplace hand-rolls both the fetch and the key derivation and one
secret-derivation rule now lives in two repos.
So: `notte functions download` should exist as a primitive with the key
derived internally, `pull` becomes a loop over it, and the walk needs bounded
concurrency, Retry-After-aware backoff, and a complete page walk before
anything is reported as a remote extra — a listing that stops early is
indistinguishable from one where functions were deleted.
Adds backend ask 3: return the download url from the list endpoint, halving
the request count. Same additive change that added published and
required_secrets. Renumbers the asks below it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit asked the backend to return the download url from the list endpoint, to halve pull's request count from 1+2N to 1+N. That ask is unnecessary and marketplace already proves it. marketplace-catalog.ts runs the full walk at concurrency 48 over 2,049 functions — roughly 4,120 requests — and the comment on its retry classifier records the measurement: "Nothing hit a 429 while this was being measured, but a full pull is several times larger than any sample taken, and a retry is much cheaper than a half-written tree." No rate limiting at the largest scale that exists, and the retry logic is defensive rather than a response to observed throttling. For a realistic project of tens of functions this is a second or two. Trading coordination cost with the backend for an imperceptible win is the wrong call, so the ask is removed and the remaining ones renumbered. What is needed instead is all client-side: notte functions download as a primitive that derives the decryption key internally, a bounded-concurrency loop over it, Retry-After-aware backoff, and a complete page walk before anything is reported as a remote extra. Also notes that check need not pay this cost at all by default. The lock stores artifact_sha256 per env, so the common gate — you changed sources and did not deploy — is a local build and a hash comparison with no network walk. --verify-remote does the full download to catch console edits. marketplace always downloads because it is a mirror with no separate source hash to trust; we have one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
An RFC — discussion only, nothing implemented — proposing that the CLI absorb the deploy framework we have now hand-rolled twice:
anything-api/marketplacemonorepo/apps/back/managed-authscripts/marketplace-catalog.ts(2,404 lines TS)scripts/deploy.py(996 lines Python)make marketplace push prodmake deploy google.com stagingcontract.pyspliced in by one regexBoth converged on the same shape, and both spent their worst code on the same two problems: faking subcommands in
make, and not having a bundler.The proposal:
notte init/notte deployover a real Python package, with client-side bundling of local imports, per-environment state in a lockfile, and declarative secrets and schedules.📄 Read the RFC
The findings that constrain the design
Client-side bundling is the only option.
POST /functionsrunsScriptValidator.parse_script(source, restricted=True)andvisit_ImportFromraises"Relative imports are not allowed"outright.from .util import xdies a second death againstALLOWED_IMPORTS. There is no server-side path to multi-file.Off-the-shelf bundlers don't help — but not for the reason the first draft gave. An earlier version of this RFC said stickytape was impossible because RestrictedPython forbids
sys/exec/os. That was the wrong mechanism, caught in review:workflows-lambda/worker.py:891runs functions withrestricted=False, so the AST policy is never applied at execution time. What actually blocks it issafe_import, which name-checks every import at run time — stickytape dies onimport tempfile(discarded atworker.py:520) and then onimport util, which is the one thing it exists to do. Allowingosandsystouches neither; making it work means disablingsafe_importentirely. The RFC now separates the two gates explicitly and leads with the three reasons to flatten that hold regardless of any allowlist.There is no dependency resolution to build. Dependencies are a fixed allowlist — no
requirements.txt, no PEP-723, just a build-time import check soimport osfails locally in 20 ms rather than after a multipart upload.Schedules cannot be reconciled today.
POST /scheduleis a clean upsert with revision CAS, but there is no read endpoint:functions.schedule_cronexists on the row and is dropped by theFunctionResponsemodel. Additive ~2-line fix, same pattern as whenpublishedandrequired_secretswere added.Command surface
Six commands in v1 —
init,new,pull,deploy,check,status. The first draft proposed eighteen; roughly half were gated on backend work that doesn't exist yet. Everything else is in a deferred table with a reason each.pullis in v1 because without itdeployis unsafe in a non-empty org: create-vs-update reads the lock, so a freshinitagainst an org that already has the function creates a duplicate rather than updating it, andfunctions.namehas no unique constraint to stop it.Open decisions for the team
notte.toml(Python audience, comments matter, shallow nesting). Notably YAML is out partly becauseproxy_country = "no"parses asfalse.functions/at the repo root rather thannotte/functions/, because a top-levelnotte/directory shadows the realnottepackage the moment the repo root lands onsys.path.safe_importstay? It's now the only import guard oncerestricted=False. Worth deciding on its own merits — the RFC doesn't need it either way.FunctionResponseis the cheapest and the most blocking.Not in v1
Managed-auth templates are already the best declarative surface in the API (dry-run returns a real field-level diff, apply is digest-guarded) and could join later. Managed-auth connections probably never should — creating one runs a real browser login, spends money, and provisions a vault and a profile as side effects.
🤖 Generated with Claude Code