[justinwlin]

Closes #384

flash deploy gave every endpoint the deployer's RUNPOD_API_KEY. _check_makes_remote_calls searched ./flash_manifest.json and /flash_manifest.json, but deploy keeps the manifest at .flash/flash_manifest.json, so the lookup missed and fell back to "needs the key".

Changes

• Deploy follows the manifest. create_resource_from_manifest stores makes_remote_calls on the resource; _check_makes_remote_calls uses it before any file lookup. flash run has no manifest and keeps the old fallback (fix(run): inject API token for LB cross-endpoint calls #347).

• Build detection errs toward giving the key. Deploy now trusts the build, so a missed detection is a runtime 401. Names are resolved against the module's globals and any imports inside the function, including relative imports and sibling project modules. A function needs the key when it, directly or through a helper or class in a scanned project file (package __init__.py included):

  • references another @Endpoint function or class, including through a module or package path (tasks.square(x), pkg.tasks.square(x)), an aliased import, or as a value (map(square, xs)). Only direct by-name calls were detected before.
  • uses a client-mode Endpoint(id=...) / Endpoint(image=...), at module level, inline, or under an aliased import
  • uses the runpod SDK or reads RUNPOD_API_KEY
  • references a project class or instance whose methods or instance attributes do any of the above (client.generate(x), Helpers.call(x), holder.ep.runsync(x)), or a bound method, functools.partial, container, or lru_cache-style wrapper around any of the above

  A function, helper, or class the scanner cannot analyze, and a project import it cannot resolve, is treated as needing the key.

• Stale keys are removed. On update, a live RUNPOD_API_KEY is no longer carried forward when the endpoint does not make remote calls. Unchanged endpoints skip the update path, so deploy also removes the key from each unchanged local-only endpoint's template. This costs one myself.endpoints read plus one template read per such endpoint, run concurrently under a 120s bound. Failures are logged and never fail the deploy. A template shared with another endpoint keeps the key in both paths, since that endpoint may call others. A key the user set in env is never touched.

• Users can see and fix it. The deploy summary lists the endpoints Flash gave the key. On a worker without the key, a 401 from a client Endpoint call explains why and what to change. docs/Cross_Endpoint_Routing.md documents the rules.

Side effects on existing deployments

• The first deploy after upgrading removes RUNPOD_API_KEY from local-only endpoints. The template env changes, so those endpoints roll their workers once.
• Flash cannot tell a key it injected from one added in the console. A console-added key on a local-only endpoint is removed. A key set in the Flash env is kept.

Known limits

• A client Endpoint passed in as an argument, or a hand-built HTTP request that never names RUNPOD_API_KEY, is not detected. The 401 message points to the fix.
• A client Endpoint(image=...) that Flash deploys runs an image Flash does not scan, so it no longer gets the key. If that image calls other endpoints, set RUNPOD_API_KEY in its env (documented).
• An update whose user env is unchanged still does not inject runtime vars (the fix: resolve .env from CWD and stop _do_deploy env mutation #236 design). On the deploy path this is covered: an endpoint that starts making remote calls gets RUNPOD_API_KEY in its env, which counts as an env change.

Testing

• New unit tests across the scanner (local, relative, and package imports, project classes, __init__ helpers, unparseable helpers), provisioner, update paths, unchanged-endpoint cleanup (shared templates, timeout, partial failure), 401 hint, and deploy summary.
• Scanner run on sample projects covering each case above: all classified correctly. main's scanner marks the module-attribute, alias, and pass-as-value cases false, which the first commit alone would have broken.
• Full suite: 2809 passed. 2 failures in tests/unit/test_load_balancer_sls_stub.py are order-dependent and also fail on main.
• mypy: no new errors in touched files.
• Reviewed by Codex. Its findings on unchanged-redeploy cleanup, function-local imports, cyclic helpers, and the summary wording are fixed here.
• The myself.endpoints { id templateId } query was checked read-only against the live API.
• Not yet verified with a live deploy.

[OneAsterisk]

Claude review, round 1

Verdict: Approved. The deploy fix is correct and the stale-key cleanup is safe; four scanner and duplication issues are worth fixing before or soon after merge.
Approval gate: 0 Critical, 0 High, 7 Medium+Low → approved.

Existing feedback

• No prior reviews or comments. CI: all Quality Gates (3.10 to 3.14), lint, Guard, and Nix flake pass. Build Package skips on PRs.

PR description vs code

Claim	Status	Where
Deploy pins makes_remote_calls on the resource; flash run keeps the fallback	✅	runtime/resource_provisioner.py:206, core/resources/serverless.py:895
Pinned value survives the deploy path (create and update)	✅	Every _check_makes_remote_calls call runs on the manifest-built object (serverless.py:1005,1062,1230)
Names resolve against "any imports inside the function"	⚠️	Only modules still in sys.modules; project modules are popped after scan (see Medium 1)
Module references like tasks.square(x) are detected	⚠️	One level only; pkg.tasks.square(x) is missed (Medium 2)
"A function the scanner fails to analyze is treated as needing the key"	⚠️	True for the endpoint function; a helper that fails to parse returns [] (Medium 3)
Stale-key cleanup costs "one extra GraphQL read per such endpoint"	❌	Two reads (get_endpoint_template_id, get_template), plus a write when a key is found (cli/utils/deployment.py:46,49)
Cleanup failures are logged and never fail the deploy	✅	deployment.py:81-88, :94-95
A key the user set in env is never touched	✅	deployment.py:243-245, serverless.py:1065, :1231
401 hint on a worker without the key	✅	endpoint.py:45-61, same request/response so callers' except still matches
39 new test cases	✅	Counted, including parametrize cases

Findings

Medium

• Function-local imports of project modules do not resolve (scanner.py:494, :504). _import_module_from_file pops each project module from sys.modules after import (scanner.py:264-272), so from tasks import square as sq; sq(x) or import tasks; tasks.square(x) inside a function finds nothing unless another file imported tasks at top level. Relative imports are skipped entirely. Before this PR the miss was masked by the broken fallback; now deploy trusts it.
• Nested module attributes are missed (scanner.py:560). The branch requires child.value to be an ast.Name, so pkg.tasks.square(x) and config.clients.vllm.runsync() are never flagged. _resolve already recurses; drop the guard and label with ast.unparse(child).
• Helper analysis fails open (scanner.py:581-582). A helper whose source cannot be fetched or parsed returns [], which contradicts the "errs toward giving the key" design. Return ["<unanalyzed>"] instead.
• Second copy of the saveTemplate payload builder (cli/utils/deployment.py:54-64). Same allowed fields, volumeInGb, id, and comment as ServerlessResource._build_template_update_payload (serverless.py:853-883). If one gains a field, the other silently drops it on every unchanged redeploy.

Low

• Cleanup runs serially before the timed provisioning block (deployment.py:263). Two GraphQL calls per unchanged local-only endpoint on every deploy, one after another, outside the 600s wait_for. Fine at today's endpoint counts; a gather would remove the latency. Fix the PR body's "one extra read".
• Side effects not in the PR body. The first deploy after upgrade sends saveTemplate to every unchanged endpoint that still holds the key, which starts a rolling release. A RUNPOD_API_KEY added by hand in the Runpod console (not in Flash env) is removed on every deploy. Client-mode image= endpoints are always makes_remote_calls: False (manifest.py:362), so a user image that relied on the injected key loses it. Worth a "Things to know" section.
• Test gaps on the risky paths. test_function_local_imports only uses runpod_flash, runpod, and json, which are always loaded, so it cannot catch Medium 1. No test pins a user-set RUNPOD_API_KEY with makes_remote_calls=False on either update branch in serverless.py. test_errors_never_fail_the_deploy covers only the outer except, not a per-endpoint failure followed by the next endpoint.

Nits

• cli/commands/deploy.py:183 says "added", but the list includes unchanged endpoints that got the key in an earlier deploy. "Endpoints with RUNPOD_API_KEY" is accurate.
• docs/Cross_Endpoint_Routing.md:194 implies every miss surfaces the 401 hint. A missed @Endpoint-to-@Endpoint call goes through ServiceRegistry, which skips the State Manager lookup when makes_remote_calls=False (runtime/service_registry.py:184) and never hits _raise_for_status.
• Commit subjects are fix(...); squash so intermediate ones do not land in release notes.

Simplification opportunities

1. Reuse _build_template_update_payload in _remove_stale_api_key (about 10 lines removed, can be in this PR).
2. One predicate for "Flash owns this key and the endpoint does not need it", used at serverless.py:1062, :1229, deployment.py:243, deploy.py:177. Four slightly different forms today. Follow-up is fine.

Verified clean

• _makes_remote_calls is set after construction, so CPU/LB validators do not clear it; pickled copies drop it but are only used as self in update.
• Manifest aggregation per resource (manifest.py:595) covers class endpoints and LB routes.
• @Endpoint(image=...) as a decorator raises (endpoint.py:685), so walking decorator_list gives no false positive.
• runpod_flash does not match _is_runpod_module_name.
• Cleanup payload sends the same fields the existing update path sends; deleted endpoints return None.
• Tests stub RUNPOD_ENDPOINT_ID and get_api_key explicitly; no ambient env dependency.

Questions for the author

• Are function-local cross-endpoint imports common in real Flash projects? That decides whether Medium 1 should block.
• Is the rolling release on the first post-upgrade deploy acceptable, or should cleanup be opt-in for one release?

---

_{Reviewed by Claude (claude-opus-5-5) via the pr-review skill. Tokens: 166 in / 13,517 out / 5,797,746 cache read / 284,779 cache write = 6,096,208. Estimated cost: $3.71 at $4 in / $20 out / $0.20 cache read / $8 cache write per MTok (1-hour cache TTL). Round 1 of this PR. This is automated assistance; a human owns the final call.}

[Copilot]

🟡 Changes recommended

Scanner false negatives can withhold required credentials, and shared-template cleanup can remove credentials from other endpoints.

3 open findings

• Shared templates can incorrectly remove required credentials · New
• Local project imports remain unresolved · New
• Nested attribute references miss required API keys · New

What changed in this PR

Addresses #384 by making deployment follow build-time API-key requirements while preserving the flash run fallback.

Changes:

• Expands remote-call detection and carries its result into provisioning.
• Removes stale keys from local-only endpoints.
• Adds deployment visibility, authentication guidance, documentation, and tests.

File	Description
tests/​unit/​test_endpoint_client.py	Tests missing-key hints.
tests/​unit/​runtime/​test_resource_provisioner.py	Tests manifest-driven injection.
tests/​unit/​resources/​test_serverless.py	Tests stale-key removal during updates.
tests/​unit/​core/​api/​test_runpod_graphql_extended.py	Tests template-ID lookup.
tests/​unit/​cli/​utils/​test_deployment.py	Tests unchanged-endpoint cleanup.
tests/​unit/​cli/​test_deploy.py	Tests key-recipient summaries.
tests/​unit/​cli/​commands/​build_utils/​test_runtime_scanner.py	Expands detection tests.
src/​runpod_flash/​runtime/​resource_provisioner.py	Retains the manifest decision.
src/​runpod_flash/​endpoint.py	Explains missing worker credentials.
src/​runpod_flash/​core/​resources/​serverless.py	Honors detection and removes stale keys.
src/​runpod_flash/​core/​api/​runpod.py	Adds endpoint template lookup.
src/​runpod_flash/​cli/​utils/​deployment.py	Cleans unchanged endpoints.
src/​runpod_flash/​cli/​commands/​deploy.py	Lists automatic key recipients.
src/​runpod_flash/​cli/​commands/​build_utils/​scanner.py	Expands remote-call analysis.
docs/​Deployment_Architecture.md	Documents injection and cleanup.
docs/​Cross_Endpoint_Routing.md	Explains detection and remediation.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[OneAsterisk]

Claude review, round 2

Verdict: Approved. Every round 1 finding and every Copilot and @deanq thread is fixed at 3c1265b9; what remains is Low (docs, one test, PR body).
Approval gate: 0 Critical, 0 High, 4 Medium+Low → approved.

Reviewed 551d7557...3c1265b9 (3 commits: 75045515, 69754e0f, 3c1265b9) in full, plus every file a prior finding pointed at. The watcher triggered on 69754e0f; the head has since moved to 3c1265b9, which is what this round covers.

Status of prior findings

#	Finding	Severity	Status	Evidence
R1-1	Function-local project imports do not resolve	Medium	Fixed	scanner.py:535-596, _CallScan.lookup_module reads project_modules (:807-823); unresolved project imports add <unanalyzed> (:644). Tests use real sibling modules. 75045515
R1-2	Nested module attributes missed	Medium	Fixed	scanner.py:669-672 handles every Attribute; _resolve falls back to scanned submodules (:454-456). 75045515
R1-3	Helper analysis fails open	Medium	Fixed	scanner.py:684-687 returns [_UNANALYZED]; test_unparseable_helper_assumes_key_needed. 75045515
R1-4	Second copy of the saveTemplate payload builder	Medium	Fixed	deployment.py:58-60 calls ServerlessResource._template_update_payload (serverless.py:877). 69754e0f
R1-5	Cleanup serial and outside any timeout	Low	Fixed	One myself.endpoints read, asyncio.gather, 120s wait_for (deployment.py:95-106). PR body updated. 69754e0f
R1-6	Side effects not in the PR body	Low	Not addressed	Body still omits the rolling release on first deploy, removal of a console-added key, and image= endpoints losing the key.
R1-7	Test gaps on risky paths	Low	Partially fixed	Local-import and per-endpoint-failure tests added. Still no test pins a user-set RUNPOD_API_KEY with makes_remote_calls=False on either update branch (test_serverless.py _update only passes LOG_LEVEL).
R1-N1	deploy.py:183 says "added"	Nit	Not addressed
R1-N2	Cross_Endpoint_Routing.md:194 implies every miss shows the 401 hint	Nit	Not addressed
R1-N3	fix: commit subjects leak into release notes	Nit	Not addressed	8 fix(...) commits now.

Existing feedback

All threads are resolved. Verified each at head:

• Copilot on deployment.py shared templates: fixed at 69754e0f. template_shared guards both paths (deployment.py:79, serverless.py:925); lookup failure keeps the key (serverless.py:918-924). #389 (comment)
• Copilot on local project imports: fixed at 75045515 (see R1-1). #389 (comment)
• Copilot on nested attributes: fixed at 75045515 (see R1-2). #389 (comment)
• @deanq on __init__.py helpers: fixed. project_files now built from all_files (scanner.py:835); test_helper_in_package_init. #389 (comment)
• @deanq on project classes and instances: fixed at 75045515 and 3c1265b9. _project_class, _class_targets, and _needs_key follow methods, static methods, and nested instance attributes; test_project_classes_and_instances includes a pure-class negative case. #389 (comment)
• @deanq on "not found" string matching: fixed by removal. The account-wide endpoints map treats a deleted endpoint as absent (deployment.py:76-78). #389 (comment)

No CI checks are reported on the branch (gh pr checks: none).

PR description vs code

Claim	Status	Where
Local, relative, and package imports resolve, incl. sibling modules	✅	scanner.py:535-596
pkg.tasks.square(x) detected	✅	scanner.py:669-672, test qualified
Project classes, instances, holder.ep.runsync(x)	✅	scanner.py:603-628, :691-713
Unanalyzable helper/class/project import needs the key	✅	scanner.py:644, :686, :708
One myself.endpoints read plus one template read per endpoint, concurrent, 120s bound	✅	deployment.py:95-106
Shared template keeps the key in both paths	✅	deployment.py:79, serverless.py:925
docs/Cross_Endpoint_Routing.md documents the rules	⚠️	Docs predate this round (Low 1)

Findings

Low

• Docs lag the new rules (docs/Cross_Endpoint_Routing.md:185, docs/Deployment_Architecture.md:191). Routing docs still say "through a helper function" and omit classes, package paths, local imports, and the unresolved-import rule. Architecture docs say a stale key "is removed on the next flash deploy" with no shared-template exception.
• test_slow_api_is_bounded cannot fail (tests/unit/cli/utils/test_deployment.py:1017). Without the bound, hang returns after 10s, template_ids.get raises inside clean, the outer except logs it, and get_template is still not awaited. Assert elapsed time or the timeout warning.
• R1-6 carried: PR body side effects (see table).
• R1-7 carried: user-set key test on the update branches (see table).

Nits

• _should_drop_api_key (serverless.py:906) runs one account-wide endpoints query per updated endpoint that still holds the key. Fine after the first deploy; worth knowing for large accounts.
• deployment.py:58 calls the private ServerlessResource._template_update_payload from another module. A public name would make the shared use explicit.

Simplification opportunities

1. The R1 predicate note still stands: "Flash owns this key and the endpoint does not need it" appears at serverless.py:1110, :1277, deployment.py:256, deploy.py:177. Follow-up is fine.

Verified clean

• _CallScan cycle handling: cache seeded with [] per class and helper, seen per _object_needs_key call, cache fresh per endpoint (scanner.py:755).
• Relative imports resolve through the module's __package__, which spec_from_file_location sets; synthetic parent packages are registered before analysis (scanner.py:800).
• A failed project import maps to None and yields <unanalyzed>, not a silent miss.
• _template_update_payload adds value is not None; model_dump(exclude_none=True) already dropped None, so the existing update path is unchanged.
• Cleanup: deleted endpoint and missing template skip; per-endpoint errors are isolated inside clean; timeout and lookup errors never fail the deploy.

Questions for the author

• Has this been run against a live deploy yet? The body still says "Not yet verified with a live deploy", and the myself.endpoints change is new this round.

---

_{Reviewed by Claude (claude-opus-5-5) via the pr-review skill. Tokens: 30 in / 13,758 out / 985,372 cache read / 83,903 cache write = 1,083,063. Estimated cost: $2.61 at the Opus 5.5 rate card in the skill's sum_usage.py (cache writes priced at 1-hour TTL). Round 2 of this PR. This is automated assistance; a human owns the final call.}

[OneAsterisk]

Claude review, round 3

Verdict: Approved. The new wrapper and container handling is correct for client Endpoints; one Medium gap remains for wrappers around project helpers.
Approval gate: 0 Critical, 0 High, 6 Medium+Low → approved.

Reviewed 3c1265b9...c0c14f0a (1 commit, 3 files, +88/-2) in full, plus every file an open finding points at.

Status of prior findings

#	Finding	Severity	Status	Evidence
R1-1 to R1-5	Scanner and cleanup fixes	Medium/Low	Fixed (round 2)	Unchanged at c0c14f0a
R1-6	Side effects not in the PR body	Low	Partially fixed	image= endpoints losing the key is now in the body and Cross_Endpoint_Routing.md:194. The rolling release on first deploy is still unmentioned.
R1-7	No test pins a user-set RUNPOD_API_KEY with makes_remote_calls=False on the update branches	Low	Not addressed	tests/unit/resources/test_serverless.py unchanged since round 2
R2-1	Docs lag the new rules	Low	Partially fixed	image= paragraph added. Cross_Endpoint_Routing.md:185 still says "through a helper function" only; Deployment_Architecture.md:191 still has no shared-template exception.
R2-2	test_slow_api_is_bounded cannot fail	Low	Not addressed	test_deployment.py unchanged
R1-N1	deploy.py:183 says "added"	Nit	Not addressed
R1-N2	401 hint implies every miss shows it	Nit	Not addressed	Cross_Endpoint_Routing.md:196
R1-N3	fix: commit subjects leak into release notes	Nit	Not addressed	9 fix(...) commits

Existing feedback

• All Copilot and @deanq threads were resolved and verified in round 2. Nothing in this commit reopens them.
• Two round 2 threads stay open (R2-1, R2-2); replies posted.
• No CI checks are reported on the branch.

PR description vs code

Claim	Status	Where
Bound method around a client Endpoint (run = vllm.runsync)	✅	scanner.py:630-631, test bound
functools.partial around a client Endpoint	✅	scanner.py:632-634, test partial
Container holding a client Endpoint	✅	scanner.py:635-640, test container
lru_cache-style factory	✅	_project_function unwraps (scanner.py:502), test cached
Partial, container, or wrapper "around any of the above", including a helper	⚠️	Not for project helper functions (Medium 1)
image= endpoints do not receive the key, documented	✅	Body "Known limits", Cross_Endpoint_Routing.md:194

Findings

Medium

• A partial or container around a project helper is missed (scanner.py:644). _needs_key handles a plain function only through _project_class, which returns None for any function. So call = functools.partial(call_model, timeout=5) or HANDLERS = {"gen": call_model}, where call_model calls vllm.runsync, marks the endpoint local-only. The Call path does not catch it either: _unwrap does not see through a partial, and a Subscript callee does not resolve. Deploy then removes the key and the worker gets a 401. Fix inline.

Low

• R1-6, R1-7, R2-1, R2-2 carried (see table).

Nits

• scanner.py:659 _unwrap sits next to _unwrap_to_original (scanner.py:152). Both call inspect.unwrap. One helper with the class and exception guard would cover both call sites.
• _MAX_CONTAINER_ITEMS caps each container, not the walk, and seen resets for each AST reference. A nested 1000×1000 constant table referenced N times costs about N million calls. Unlikely in practice.
• The plain_dict negative case builds a dict literal, which _resolve never follows. A module-level CONFIG = {"a": 1} referenced by name would exercise the new container branch's negative path.

Simplification opportunities

1. The round 1 predicate note still stands ("Flash owns this key and the endpoint does not need it" in four places). Follow-up is fine.

Verified clean

• Each new test case resolves the way the assertion says. run_vllm goes through MethodType.__self__, CLIENTS through the dict branch, call through partial.func.__self__, and client through the Call path after unwrap.
• _project_function checks _is_remote before and after unwrap, so an @Endpoint function is never analyzed as a helper.
• inspect.unwrap raises ValueError on cycles and on proxies whose __getattr__ fabricates __wrapped__. _unwrap catches it. Classes skip unwrap, so class-level __wrapped__ cannot redirect.
• Moving the seen check ahead of _project_class adds non-project objects to seen. That only skips repeat visits, which return the same answer.
• str and bytes are excluded from the container branch, so iterating a string cannot recurse.

Questions for the author

• Has this run against a live deploy yet? The body still says "Not yet verified with a live deploy".

---

_{Reviewed by Claude (claude-opus-5-5) via the pr-review skill. Tokens: 22 in / 8,857 out / 589,704 cache read / 65,079 cache write = 663,662 (main session, no sub-agents; diff under 300 lines). Estimated cost: $1.89 at the Opus 5.5 rate card in the skill's sum_usage.py (cache writes priced at 1-hour TTL), as of posting. Round 3 of this PR. This is automated assistance; a human owns the final call.}

[OneAsterisk]

Claude review, round 4

Verdict: Approved. This commit closes the docs, test, and message items from earlier rounds; the round 3 Medium (helpers behind a partial or container) is still open.
Approval gate: 0 Critical, 0 High, 2 Medium+Low → approved.

Reviewed c0c14f0a...eeac18c7 (1 commit, 8 files, +44/-14) in full, plus scanner.py for R3-1.

Status of prior findings

#	Finding	Severity	Status	Evidence
R1-1 to R1-5	Scanner and cleanup fixes	Medium/Low	Fixed (round 2)	Unchanged
R1-6	Side effects not in the PR body	Low	Fixed	Body now says the first deploy rolls local-only endpoints once
R1-7	No test pins a user-set key with makes_remote_calls=False on the update branches	Low	Partially fixed	New test covers the env-changed branch only. Both cases change env, so serverless.py:1277 stays untested. See inline.
R2-1	Docs lag the new rules	Low	Fixed	Cross_Endpoint_Routing.md:186-192, Deployment_Architecture.md:191
R2-2	test_slow_api_is_bounded cannot fail	Low	Fixed	test_deployment.py:1038-1039 asserts elapsed time and the new timeout log
R3-1	Partial or container around a project helper is missed	Medium	Not addressed	scanner.py unchanged since c0c14f0a
R1-N1	deploy.py:183 says "added"	Nit	Fixed	Now "set by Flash"
R1-N2	401 hint implies every miss shows it	Nit	Fixed	Cross_Endpoint_Routing.md:198 scopes the message to client Endpoint calls
R1-N3	fix: commit subjects leak into release notes	Nit	Not addressed	9 fix(...) commits; fine if squashed with an edited message
R3 nits	_unwrap duplication, container walk cost, plain_dict case	Nit	Not addressed

Existing feedback

• All Copilot and @deanq threads stay resolved; this commit does not touch their lines.
• R2-1 and R2-2 threads were resolved by the author; verified fixed, replies posted.
• CI: Guard and Pre-check pass. Quality Gates (3.10 to 3.14) and Nix flake were still running at review time.

PR description vs code

Claim	Status	Where
Docs describe the detection rules	✅	Cross_Endpoint_Routing.md:186-192, including the unresolved-import rule
Shared templates and user-set keys are kept	✅	Deployment_Architecture.md:191, deployment.py:84-89, serverless.py:1277
Partials and containers around "any of the above"	⚠️	Not for project helpers (R3-1)
Rolling restart on first deploy after upgrade	✅	Body line 20

Findings

Medium

• R3-1 carried (scanner.py:646). Reply in the existing thread.

Low

• R1-7 partially fixed (test_serverless.py:3615). Inline.

Nits

• deployment.py:113 dropped !r. An exception with an empty message (for example a bare ConnectionError()) now logs nothing after the colon. TimeoutError was the main case and now has its own branch, so this is minor.

Verified clean

• except asyncio.TimeoutError catches the wait_for timeout on 3.10 (where it is not the builtin) and 3.11+ (where it is an alias). It sits before except Exception, so it wins.
• _template_update_payload → template_update_payload: both callers updated (serverless.py:872, deployment.py:58). No test referenced the old name. The rename removes a cross-module private access.
• test_slow_api_is_bounded now fails if the wait_for bound is removed: hang sleeps 10s and the test asserts under 2s.
• test_deploy.py:663 matches the new summary string exactly.

Questions for the author

• Has this run against a live deploy yet? The body still says "Not yet verified with a live deploy".

---

_{Reviewed by Claude (claude-opus-5-5) via the pr-review skill. Tokens: 18 in / 5,068 out / 445,612 cache read / 52,907 cache write = 503,605 (main session, no sub-agents; diff under 300 lines). Estimated cost: $1.42 at the Opus 5.5 rate card in the skill's sum_usage.py (cache writes priced at 1-hour TTL), as of posting plus this summary. Round 4 of this PR. This is automated assistance; a human owns the final call.}

[OneAsterisk]

Claude review, round 5

Verdict: Approved. Both open items from round 4 are fixed at fbf29985; nothing new above Nit.
Approval gate: 0 Critical, 0 High, 0 Medium+Low → approved.

Reviewed eeac18c7...fbf29985 (2 commits, 3 files, +58/-5) in full, plus the surrounding _needs_key, _project_function, _helper_targets, and the serverless.py update branch.

Status of prior findings

#	Finding	Severity	Status	Evidence
R1-1 to R1-6	Scanner, cleanup, and PR body fixes	Medium/Low	Fixed (rounds 2 and 4)	Unchanged
R1-7	No test pins a user-set key on the env-unchanged update branch	Low	Fixed	test_serverless.py:3618-3619, fbf29985
R2-1, R2-2	Docs lag, tautological timeout test	Low	Fixed (round 4)	Unchanged
R3-1	Partial or container around a project helper is missed	Medium	Fixed	scanner.py:644-647, 20cc5c4b
R1-N3	fix: commit subjects leak into release notes	Nit	Not addressed	10 fix(...) commits; fine if squashed with an edited message
R3 nits, R4 nit	_unwrap duplication, container walk cost, plain_dict case, deployment.py:113 dropped !r	Nit	Not addressed

Existing feedback

• All Copilot and @deanq threads stay resolved; these commits do not touch their lines.
• R3-1 and R1-7 threads were resolved by the author. Verified fixed; replies posted.
• CI: Guard and Pre-check pass. Quality Gates (3.10 to 3.14) and Nix flake were in progress at review time.

PR description vs code

Claim	Status	Where
Partials and containers around "any of the above", including project helpers	✅	scanner.py:644-647; test test_wrapped_and_contained_project_helpers
A key the user set in env is never touched	✅	Both update branches now tested: env-unchanged hits serverless.py:1277, env-changed hits _preserve_platform_env new_keys skip

Findings

Nits

• scanner.py:647 A helper called through a module attribute now reports twice (["generate", "clients.generate"]): once from the Call branch (line 704) and once from the Attribute branch (line 713). Only the boolean calls_remote_functions is consumed (manifest.py:595), so this is cosmetic. The two updated test expectations pin the duplicate; fine to leave.
• test_serverless.py:3558 In the env-unchanged case the returned key is the mock's live "old-key", not "user-key". The test asserts presence only, so it still fails if the line 1277 guard is removed. Asserting env["RUNPOD_API_KEY"] would be stronger only if the mock's live template held the user value.

Verified clean

• R3-1 path: call = functools.partial(call_model, ...) → _needs_key partial branch → call_model → new _project_function branch → _helper_targets → ["vllm"] → True. Same for {"gen": call_model} (container branch) and map(call_model, xs) (Name branch). PURE = {"double": double} returns [] → False.
• The new branch cannot turn a former True into False. For a plain function _project_class returned (None, None) anyway, so the old fall-through was already False.
• Cycles: _helper_targets seeds scan.cache[id] = [] before recursing, so a helper that references itself or a partial of itself terminates.
• Unparseable helper behind a partial returns [_UNANALYZED] → True, matching the err-toward-the-key rule.
• R1-7 test: with old and new env equal, env_changed is False at serverless.py:1249, so the else branch runs. Removing the "RUNPOD_API_KEY" not in new_config.env guard makes _should_drop_api_key return True (local-only, unshared template) and the key drops, so the test fails as intended.
• old_env or {...} default keeps the other TestStaleApiKeyCleanup cases unchanged.

Questions for the author

• Has this run against a live deploy yet? The body still says "Not yet verified with a live deploy".

---

_{Reviewed by Claude (claude-opus-5-5) via the pr-review skill. Tokens: 26 in / 6,530 out / 760,723 cache read / 66,110 cache write = 833,389 (main session, no sub-agents; diff under 300 lines). Estimated cost: $1.84 at the Opus 5.5 rate card in the skill's sum_usage.py (cache writes priced at 1-hour TTL), as of posting plus this summary. Round 5 of this PR. This is automated assistance; a human owns the final call.}

[OneAsterisk]

Claude review, round 6

Verdict: Approved. The **kwargs change errs toward the key, as the PR intends. One Low: it also matches @Endpoint(..., **cfg) decorators.
Approval gate: 0 Critical, 0 High, 1 Medium+Low → approved.

Reviewed 2832431e...78055475 (1 commit, 2 files, +6/-3) in full, plus _remote_call_targets and _analyze_cross_calls_ast, which call the changed function.

Status of prior findings

#	Finding	Severity	Status	Evidence
R1-1 to R1-7, R2-1, R2-2, R3-1	Scanner, cleanup, docs, test, and PR body fixes	Medium/Low	Fixed (rounds 2 to 5)	Unchanged by this commit
R1-N3	fix: commit subjects leak into release notes	Nit	Not addressed	11 fix(...) commits now. Fine if the squash message is edited
R3 to R5 nits	_unwrap duplication, container walk cost, plain_dict case, dropped !r, duplicate helper report, "old-key" assertion	Nit	Not addressed	Not counted

Existing feedback

• Copilot and @deanq threads stay resolved. This commit does not touch their lines.
• No open review threads at this head.
• CI: Guard and Pre-check pass. Quality Gates (3.10 to 3.14) and Nix flake are still pending.

PR description vs code

Claim	Status	Where
Client-mode Endpoint(id=...) / Endpoint(image=...) detected inline and under aliases	✅	scanner.py:538, now also **kw
Detection errs toward giving the key	✅	**kw treated as possibly carrying id or image

Findings

Low

• @Endpoint(name=..., **cfg) decorators now flag their own function (scanner.py:538). _analyze_cross_calls_ast passes the whole FunctionDef/ClassDef to _remote_call_targets (line 799). ast.walk there visits decorator_list, so the decorator's Endpoint(...) call reaches the new kw.arg is None match. That decorated endpoint gets reported as ["Endpoint"] and keeps the deployer's key, which is the #384 behavior this PR removes. It fails safe, and the summary lists it, so it is Low. The fix: skip decorator calls on the analyzed node. Client mode is never a decorator (endpoint.py:82-89 raises). Add a negative test with @Endpoint(name="w", **CFG).

Nits

• docs/Cross_Endpoint_Routing.md:189 and the PR body list only Endpoint(id=...) / Endpoint(image=...). One clause covering Endpoint(**kwargs) would match the code.

Verified clean

• id and image are keyword-only (endpoint.py:423-442), so positional args cannot carry them. Matching kw.arg is None covers the only other way to pass them.
• Endpoint(name="x") inline without ** still returns False. Old behavior is unchanged for explicit keywords.
• The new parametrized case **{"id": "abc123"} produces a keyword with arg=None. The test asserts ["Endpoint"] and would fail without None in the tuple.
• Module-level ep = Endpoint(**cfg) goes through the live-object check (_is_client_endpoint, line 462), not this AST path, so it is classified by the real is_client.

Questions for the author

• Has this run against a live deploy yet? The body still says "Not yet verified with a live deploy".

---

_{Reviewed by Claude (claude-opus-5-5) via the pr-review skill. Tokens: 20 in / 5,213 out / 507,442 cache read / 53,285 cache write = 565,960 (main session, no sub-agents; diff under 300 lines). Estimated cost: $1.45 at the Opus 5.5 rate card in the skill's sum_usage.py (cache writes priced at 1-hour TTL), as of posting plus this summary. Round 6 of this PR. This is automated assistance; a human owns the final call.}

[promptless]

Promptless documentation updates

• Document which endpoints flash deploy gives RUNPOD_API_KEY explains which endpoints flash deploy gives RUNPOD_API_KEY and when it removes the key, and adds a troubleshooting entry for the related 401 error.