Repository navigation
Dev - #6
Merged
Merged
Dev#6
Conversation
- Removed API key generation and management features from the backend and frontend, including the deletion of related routes and components. - Updated authentication to support only Bearer JWT tokens, enhancing security and simplifying the codebase. - Revised documentation to reflect the removal of API key features and updated user authentication methods. - Adjusted database schema and migration scripts to drop the API keys table, ensuring a clean transition. - Enhanced user experience by streamlining the authentication process and removing unnecessary complexity.
- Updated `README.md` and `README-compose.md` to clarify the behavior of the `stop.sh` script, including the effect of the `--wipe` option on the development upload directory. - Increased the maximum upload size in the environment configuration from 30 MB to 200 MB and added new timeout settings for project jobs and image builds. - Removed the `api_keys` table and related migration scripts to streamline the database schema. - Introduced a new `preview_proxy` router for authenticated reverse proxying of in-app container previews, enhancing user experience and security. - Implemented job artifact cleanup functionality to remove on-disk and queued artifacts tied to analysis jobs, improving resource management. - Enhanced the project analysis job to automatically select Dockerfiles for builds based on the primary compose file, optimizing the build process.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e89f885664
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| alembic_subcmd="upgrade head" | ||
|
|
||
| run_alembic_once() { | ||
| compose_cmd run --rm --no-deps -w /app api sh -lc "set -euo pipefail |
There was a problem hiding this comment.
In the inspected startup workflow, migrations are now run via compose_cmd run ... api sh -lc, but the API image is based on python:3.12-slim and does not switch /bin/sh to bash; Debian slim uses dash for sh, which rejects set -o pipefail (/bin/sh: set: Illegal option -o pipefail). As a result ./scripts/start.sh exits before Alembic runs whenever this path is used. Invoke bash -lc after ensuring bash is installed, or drop pipefail for the sh command.
Useful? React with 👍 / 👎.
Comment on lines
+388
to
+389
| if path.startswith(("/api", "/docs", "/redoc", "/openapi.json", "/metrics")): | ||
| return True |
There was a problem hiding this comment.
This new platform-API guard rejects any upstream path that starts with /api, /metrics, etc., and _build_upstream_url() calls it for every iframe proxy request. For a legitimate container app at a private DinD host that serves its own backend under /api/..., the proxy will return 400 instead of forwarding the request, breaking common SPA/API deployments after the HTML loads. Restrict this check to actual platform origins/ports or to the initial URL probe rather than every container path.
Useful? React with 👍 / 👎.
- Added support for `build_dockerfiles` in the `_make_payload` function to allow specifying Dockerfiles for builds. - Updated tests to utilize the new `build_dockerfiles` parameter, ensuring accurate payload generation during project builds. - Refactored `fake_build_image` function signatures in tests to accommodate additional parameters, improving test flexibility. - Simplified file data retrieval in `FileUpload` component by removing unnecessary memoization. - Reset elapsed seconds in `ProjectUpload` component during various state transitions to ensure accurate timing tracking.
There was a problem hiding this comment.
Pull request overview
This PR expands the platform’s job/runtime UX (container previews, richer monitoring/logs, client session cleanup), increases operational limits (uploads/timeouts), and simplifies auth/db schema by removing API key support.
Changes:
- Add authenticated preview proxy + client-side preview guards to support iframe/browser previews of deployed containers.
- Implement job artifact cleanup (disk + queued runtime/image teardown) and enhance runtime status/stop flows, monitoring metrics, and log streaming.
- Remove API key feature end-to-end (DB/model/router/UI/docs) and adjust docs + scripts (dev
--wipeupload cleanup, startup migration sequencing).
Reviewed changes
Copilot reviewed 77 out of 78 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/stop.sh | Dev --wipe also clears dev upload storage. |
| scripts/stop.ps1 | Dev --wipe also clears dev upload storage (PowerShell). |
| scripts/start.sh | Run migrations pre-API boot; stage compose startup; improved messaging. |
| scripts/start.ps1 | Updated --wipe help text to match dev/prod behavior. |
| scripts/common.sh | Add helper to clear dev upload directories. |
| scripts/common.ps1 | Add helper to clear dev upload directories (PowerShell). |
| README.md | Remove API keys mention; clarify --wipe effects. |
| README-compose.md | Clarify --wipe clears dev uploads on disk. |
| frontend/src/utils/previewCheck.ts | Add client-side preview URL sanity checks. |
| frontend/src/utils/previewBrowser.ts | Add pop-out preview opener + browser hint collection. |
| frontend/src/utils/metricsSeries.ts | Add metric series shaping + rate calculations + persistence trimming. |
| frontend/src/utils/jobSessionCleanup.ts | Centralize cleanup of browser state for deleted jobs. |
| frontend/src/utils/deploySession.ts | Add session flags for deploy stop lifecycle across routes. |
| frontend/src/utils/api.ts | Add preview/check APIs, logs WS, safe socket close, runtime fields; remove API keys API. |
| frontend/src/routes.tsx | Remove API keys route. |
| frontend/src/pages/ResultsDashboard.tsx | Improve deploy stop reconciliation + uploaded file restoration rules. |
| frontend/src/pages/ResearchAnalytics.tsx | Add richer job detail modal + “open full results” for own jobs. |
| frontend/src/pages/ProjectUpload.tsx | Add phased import progress UI + update upload limit text. |
| frontend/src/pages/Landing.tsx | Improve batch analysis messaging and upload component props. |
| frontend/src/pages/History.tsx | Use centralized job session cleanup on delete. |
| frontend/src/pages/FileUpload.tsx | Safer sessionStorage parsing helper. |
| frontend/src/pages/ContainerExecution.tsx | Improve stop flow and deploy stop session flagging. |
| frontend/src/pages/ApiKeys.tsx | Remove API keys page. |
| frontend/src/pages/AnalysisProgress.tsx | Prevent StrictMode double-enqueue + improve bootstrap logic. |
| frontend/src/components/ui/tabs.tsx | Hide inactive tab content to avoid layout/scroll issues. |
| frontend/src/components/QuickDemo.tsx | Route quick demo directly into analysis flow; styling tweaks. |
| frontend/src/components/NotificationEventBridge.tsx | Improve WS lifecycle handling; add container stopped notification. |
| frontend/src/components/monitoring/EChartPanel.tsx | Add ECharts rendering wrapper with resize handling. |
| frontend/src/components/monitoring/ContainerMetricsPanel.tsx | Add container metrics UI (charts + KPIs + ports/interfaces). |
| frontend/src/components/Layout.tsx | Remove API keys nav link; use shared footer component. |
| frontend/src/components/DragDropUpload.tsx | Add max batch files UI messaging/prop. |
| frontend/src/components/DockerLoader.tsx | Redesign loader SVG/animation and accessibility attributes. |
| frontend/src/components/auth/AuthShell.tsx | Use shared footer and flex layout adjustments. |
| frontend/src/components/AppFooter.tsx | Add shared app footer component with GitHub link. |
| frontend/package.json | Add echarts dependency. |
| frontend/package-lock.json | Lockfile updates for echarts + deps. |
| frontend/nginx.conf | Increase client_max_body_size to 200m. |
| examples/README.md | Remove API keys walkthrough section. |
| context/ui-and-jobs.md | Update product-truth notes for job deletion + runtime teardown. |
| context/refactor-backlog.md | Remove API keys mention from backlog item. |
| backend/tests/test_ws_logs.py | Add tests for container log websocket. |
| backend/tests/test_worker_events.py | Update worker event tests for new teardown/log streaming hooks. |
| backend/tests/test_research_api.py | Add is_own_job coverage in research API. |
| backend/tests/test_project_scan.py | Add env_file object-form mapping test. |
| backend/tests/test_project_build.py | Update build tests for selected dockerfiles + error handling. |
| backend/tests/test_job_cleanup.py | Add tests for artifact cleanup + runtime teardown worker tasks. |
| backend/tests/test_event_bus.py | Add tests for log event routing isolation. |
| backend/tests/test_docker_logs.py | Add tests for docker log parsing/streaming + runtime inspection. |
| backend/tests/test_container_lifecycle.py | Add tests for ports/ip in deploy status + stop pending + unhealthy teardown. |
| backend/tests/test_build_selection.py | Add tests for compose/dockerfile selection logic. |
| backend/tests/test_api_routes.py | Add tests for preview-check/proxy, deploy stop pending, cleanup hook; remove API key tests. |
| backend/README.md | Remove API key mention from backend features. |
| backend/app/workers/worker.py | Add teardown/cleanup tasks; add project timeout config usage. |
| backend/app/infrastructure/events/bus.py | Route container.log only to dedicated logs channel. |
| backend/app/infrastructure/docker/client.py | Add image removal, log tail/follow, runtime inspect, build timeout. |
| backend/app/infrastructure/db/repositories.py | Remove API key repository implementation. |
| backend/app/infrastructure/db/models.py | Remove api_keys model. |
| backend/app/core/config.py | Increase upload cap; add timeouts + max builds settings. |
| backend/app/application/services/project_scanner.py | Increase zip limits; sort compose files using new key. |
| backend/app/application/services/job_cleanup.py | Add artifact cleanup + runtime teardown enqueue logic. |
| backend/app/application/services/compose_mapper.py | Support env_file object syntax and required=false semantics. |
| backend/app/application/services/build_selection.py | Add compose/dockerfile selection for auto image builds. |
| backend/app/application/schemas.py | Remove API key schemas; add preview/ports + is_own_job. |
| backend/app/api/routers/ws.py | Add container logs websocket endpoint + cached tail hydration. |
| backend/app/api/routers/research.py | Add is_own_job field in public research jobs. |
| backend/app/api/routers/project.py | Select dockerfiles for build; scale GitHub import timeouts. |
| backend/app/api/routers/preview_proxy.py | Add authenticated reverse proxy for container previews. |
| backend/app/api/routers/history.py | Call artifact cleanup before job delete. |
| backend/app/api/routers/compose.py | Add preview-check endpoint; deploy-stop pending persistence; status semantics update. |
| backend/app/api/routers/api_keys.py | Remove API keys router. |
| backend/app/api/router.py | Remove API keys router; add preview proxy router. |
| backend/app/api/deps.py | Remove X-API-Key auth path; keep Bearer JWT only. |
| backend/alembic/versions/0001_initial_schema.py | Remove api_keys table creation from initial migration. |
| backend/.env.example | Update upload cap and add timeout/max-build env vars. |
| .cursor/rules/backend-infrastructure.mdc | Update docs to remove API key repository references. |
| .cursor/rules/backend-api-routers.mdc | Update docs to reflect Bearer-only auth. |
Files not reviewed (1)
- frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+151
to
+159
| def _filter_request_headers(headers: Any, upstream_host: str) -> dict[str, str]: | ||
| forwarded: dict[str, str] = {} | ||
| for key, value in headers.items(): | ||
| lowered = key.lower() | ||
| if lowered in HOP_BY_HOP_REQUEST: | ||
| continue | ||
| forwarded[key] = value | ||
| forwarded["Host"] = upstream_host | ||
| return forwarded |
Comment on lines
+162
to
+170
| def _strip_response_headers(headers: httpx.Headers) -> dict[str, str]: | ||
| out: dict[str, str] = {} | ||
| for key, value in headers.items(): | ||
| if key.lower() in STRIP_RESPONSE_HEADERS: | ||
| continue | ||
| if key.lower() in {"content-encoding", "transfer-encoding", "content-length"}: | ||
| continue | ||
| out[key] = value | ||
| return out |
Comment on lines
+398
to
+405
| export function buildPreviewProxyEmbedUrl(sessionId: string): string { | ||
| const token = getAccessToken(); | ||
| const base = API_BASE_URL.replace(/\/$/, ""); | ||
| const prefix = base.length > 0 ? base : ""; | ||
| const path = `/api/v1/compose/deploy/preview-proxy/${sessionId}/`; | ||
| if (!token) return `${prefix}${path}`; | ||
| return `${prefix}${path}?token=${encodeURIComponent(token)}`; | ||
| } |
Comment on lines
+304
to
+310
| if (uploadEnqueuePromise) { | ||
| activeJobId = await uploadEnqueuePromise; | ||
| if (cancelled) return; | ||
| sessionStorage.setItem("analysisJobId", activeJobId); | ||
| navigate(`/analysis?jobId=${activeJobId}`, { replace: true }); | ||
| return; | ||
| } |
Comment on lines
18
to
+20
| class WorkerSettings: | ||
| redis_settings = RedisSettings.from_dsn(settings.redis_url) | ||
| functions = [run_dockerfile_analysis, run_compose_analysis, run_project_analysis, run_compose_deploy, run_compose_stop] | ||
| job_timeout = settings.project_job_timeout_seconds |
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.
README.mdandREADME-compose.mdto clarify the behavior of thestop.shscript, including the effect of the--wipeoption on the development upload directory.api_keystable and related migration scripts to streamline the database schema.preview_proxyrouter for authenticated reverse proxying of in-app container previews, enhancing user experience and security.