fix(codegen): emit a real health timestamp and fail-closed scaffolding - #1289
Conversation
#1257) The generated FastAPI project looked deployable and passed a naive smoke test while being non-functional. `/api/health` returned the literal `"2024-01-01T00:00:00Z"`, so the probe could not distinguish a live process from a wedged one or a served cache. Auth, database and message routes returned convincing 200-shaped payloads with no implementation behind them. Rule applied to the template: implement everything the generator can genuinely implement; make everything it cannot fail loudly with 501. A stub that answers successfully teaches operators to trust a lie. - `/api/health` evaluates `datetime.now(timezone.utc)` per request - `/` reports its own gaps via `UNIMPLEMENTED_ENDPOINTS` - `/api/messages` backed by a real (documented non-persistent) store; POST now returns 201 - `POST /auth/login` and `GET /api/data` return 501 with instructions - `create_access_token` / `decode_access_token` are real implementations Also in the same template: - `SECRET_KEY` no longer defaults to `secrets.token_urlsafe(32)`. A per-process random secret invalidates every token on restart and rejects tokens minted by sibling workers, surfacing as intermittent logouts rather than as the misconfiguration it is. It now reads env and fails closed at signing time. - `passlib[bcrypt]` pinned in generated requirements; the template imports `passlib.context`, so auth projects failed at import. - `HTTPException` import made conditional, unused `secrets` dropped, `JWTError` given a real use. Tests execute the generated app with `TestClient` rather than grepping its source, since the defect was behavioural: two health calls must return different, timezone-aware, near-now timestamps. Closes #1257 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
Agent Completion Truth Gate: NOT_APPLICABLEEvidence agrees. Machine-readable verdict{
"details": {},
"reasons": [],
"verdict": "not_applicable"
} |
There was a problem hiding this comment.
Pull request overview
Updates generated FastAPI projects to expose honest, functional behavior.
Changes:
- Generates request-time health timestamps and explicit 501 scaffolding.
- Adds in-memory message storage and JWT helpers.
- Adds behavioral regression tests for generated applications.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/youtube_extension/backend/code_generator.py |
Improves generated FastAPI behavior and dependencies. |
tests/unit/test_code_generator.py |
Executes and validates generated applications. |
| payload["exp"] = datetime.now(timezone.utc) + ( | ||
| expires_delta or timedelta(minutes=ACCESS_TOKEN_EXPIRE_MINUTES) | ||
| ) |
| # The generated main.py imports passlib.context; omitting it here | ||
| # produced a project that fails at import time. | ||
| requirements.append("python-jose[cryptography]==3.3.0") | ||
| requirements.append("passlib[bcrypt]==1.7.4") |
Remediation verdict — blocked on 2 valid findings, otherwise merge-readyReviewed on head The two 1. payload["exp"] = datetime.now(timezone.utc) + (
expires_delta if expires_delta is not None else timedelta(minutes=ACCESS_TOKEN_EXPIRE_MINUTES)
)2. Generated auth projects can resolve an incompatible bcrypt (line ~469). requirements.append("passlib[bcrypt]==1.7.4")
requirements.append("bcrypt>=3.1.0,<4.1")Consider extending Once both land with CI green, there are no conflicts and no other unresolved findings — this becomes merge-ready pending human approval. I did not push these fixes myself: this branch is not my designated working branch, and I don't have live authorization to commit to it. Generated by Claude Code |
Triggered by webhook pull_request.review_requested on #1289. Prior run wrongly flagged #1289 as superseded (recommend close). That compared its head SHA to the local workspace tip (a15e4bd, which was #1289's own head) instead of origin/main. Verified against origin/main (94b517c): - code_generator.py:1172 still emits the constant 2024-01-01 timestamp that #1289 fixes; a15e4bd is not an ancestor of main. - #1289 is mergeable_state=clean, all checks green. Corrected: #1289 is HALTED(awaiting_merge_approval), recommend MERGE. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wb9kecMa73hrbA23RPE2o2
|
Status check on the review request — this PR is green and merge-ready. Verified against
Correction for the record: an earlier automated triage doc (in #1290) briefly listed this PR as "superseded — recommend close." That was wrong — it compared the head SHA to a local workspace tip rather than to Generated by Claude Code |
Canonical issue
Closes #1257
Outcome
Generated FastAPI projects stop lying about their own state.
ProjectCodeGenerator._generate_fastapi_mainemitted amain.pythat looked deployable and passed a naive smoke test while being non-functional.GET /api/healthreturned the literal"2024-01-01T00:00:00Z", so a liveness probe could not distinguish a live process from a wedged one or a served cache — which defeats the only thing a health route exists to support. Auth, database and message routes returned convincing, correctly-shaped200payloads with nothing behind them.Rule applied to the template: implement everything the generator genuinely can; make everything it genuinely cannot fail loudly with HTTP 501. A stub that answers successfully trains operators to trust a lie. A
501cannot be mistaken for working behaviour.GET /api/health"2024-01-01T00:00:00Z"datetime.now(timezone.utc).isoformat(), evaluated per requestGET /unimplemented_endpoints, so the service reports its own gapsGET/POST /api/messages201POST /auth/login200"implement authentication logic"501+ genuinely workingcreate_access_token/decode_access_tokenGET /api/data200"Connect to your database here"501Three latent defects in the same template were repaired while in there:
SECRET_KEYdefaulted tosecrets.token_urlsafe(32)— a silent per-process random secret. It invalidates every token on restart and rejects tokens minted by sibling workers, so it presents as intermittent logouts rather than as the misconfiguration it is. It now reads env only and fails closed at signing time via_require_secret_key(), which keeps the module importable and testable.passlib[bcrypt]was never pinned even though the template importspasslib.context— generated auth projects failed at import.HTTPExceptionis now imported only when scaffolding routes exist, unusedsecretsdropped,JWTErrorgiven a real use.Scope
_generate_fastapi_maintemplate body, therequirements.txtblock in_generate_python_api, and behavioural regression tests.code_generator.pywithblack(the file predates black; reformatting would bury this change in ~200 lines of unrelated churn — CI enforcesruff, which passes). Also excluded: the two pre-existingrufffindings under the CI gate's flags (backend/deploy/__init__.pyUP035,backend/services/data_service.pyB025), which are present onmainin files this PR does not touch.Risk
200fromPOST /auth/loginorGET /api/datain a generated project now receives501. This is the intended correction — those responses were never backed by an implementation — but it is a visible behaviour change for anyone who wired a client against the placeholder shape. Health-check consumers are strictly better off: the payload shape is unchanged, only the timestamp became real.deployment_manager.verify_projectshort-circuitspassed=Truewhen there is nopackage.json, so no pipeline code imports or executes the generated Pythonmain.py; emitting501cannot break the generation or verification path itself.Verification
All tied to head
a15e4bdf51c250852e976c10787e943c52cd3cb9.tests/unit/test_code_generator.py: 96 passed (89 pre-existing, 7 new inTestGeneratedFastAPIBehaviour).code_generator.pycoverage 93.58%.tests/unit: 7853 passed, 5 xpassed, 0 failed.ruff checkclean on both changed files.The new tests execute the generated app with
fastapi.testclient.TestClientrather than grepping its source, because the defect was behavioural. Acceptance criterion 3 from the issue ("a test asserts the generated/healthpayload is not constant across two calls separated in time") is covered directly bytest_health_timestamp_is_evaluated_per_request, which additionally asserts the value parses as ISO-8601, is timezone-aware, and is within 60s of now.Coverage of the 7 new tests:
POST /auth/loginreturns 501GET /api/datareturns 501unimplemented_endpoints; reports none when no optional features are selectedSECRET_KEYis unsetGenerated source was additionally compiled and executed for all four feature combinations:
[],["authentication"],["database"], and both.Production evidence
Not applicable as a deployed surface — this change alters a code-generation template, not a running service. There is no runtime path in EventRelay that serves these endpoints; the artifact is a
main.pywritten to disk for a downstream developer.The equivalent evidence is direct execution of the produced artifact, which is exactly what the new tests do: the generated app is imported and driven through
TestClient, so every assertion above is made against the real running application rather than against its source text. Two consecutive health calls returned distinct timestamps (...T21:08:05.604263+00:00vs...T21:08:05.631052+00:00), confirming the constant-timestamp defect is gone in the artifact itself.The Vercel preview attached to this PR is unaffected — the frontend is untouched.
Agent handoff