Skip to content

Add tests for UI proxy and service lifecycle; enable static /ui/root serving#15

Open
Psyborgs-git wants to merge 1 commit intomainfrom
codex/add-tests-for-deployment/runtime-paths
Open

Add tests for UI proxy and service lifecycle; enable static /ui/root serving#15
Psyborgs-git wants to merge 1 commit intomainfrom
codex/add-tests-for-deployment/runtime-paths

Conversation

@Psyborgs-git
Copy link
Copy Markdown
Owner

Motivation

  • Cover critical deployment/runtime paths around the root UI proxy, static builds, and service lifecycle to catch regressions between Vite debug mode and static (dist) deployments.
  • Ensure the UI contract that workspace open links are stable and derived from app manifest data so the React UI and backend remain compatible.

Description

  • Implement static-mode serving for /ui/root in harness/server/routes/proxy.py with a _react_dist_dir helper, safe path resolution, injected harness vars into HTML, and explicit errors for missing dist or assets when harness_debug is false.
  • Add tests/test_proxy_routes.py with isolated tests that mock httpx.AsyncClient and use tmp_path to verify /ui/root in debug (Vite proxy) and static (react/dist) modes, plus failure modes for missing Vite and missing dist.
  • Add tests/test_service_manager.py exercising ProcessManager start/stop/restart, cleanup semantics, background-task cancellation, and watchdog crash logging using a DummyComponent and short-lived async tasks.
  • Add a UI contract test to tests/test_agent_routes.py asserting workspace URLs open as /ui/{first_react_view} derived from AppManifest.react_views to match the React AppManifestPanel behavior, and keep tests fast and isolated via tmp dirs and mocks.

Testing

  • Ran python -m compileall harness/server/routes/proxy.py tests/test_proxy_routes.py tests/test_service_manager.py tests/test_agent_routes.py which completed successfully.
  • Attempted to run pytest -q tests/test_proxy_routes.py tests/test_service_manager.py tests/test_agent_routes.py, but the run was blocked in this environment due to a missing dependency (ModuleNotFoundError: No module named 'pytest_asyncio').

Codex Task

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1451a5016d

ℹ️ 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".

if not dist_dir.exists():
raise HTTPException(status_code=503, detail=f"React dist directory not found: {dist_dir}")

rel = path or "index.html"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Stop hard-coding dist/index.html as static entry

When harness_debug is false and no subpath is requested, this branch always resolves index.html. In this repo, the React production build is configured with JS/TS Rollup inputs (react/vite.config.ts build.rollupOptions.input), which produces hashed bundles in dist/assets but no dist/index.html; as a result, /ui/root returns 404 in static mode even after a successful vite build. This makes the new static serving path non-functional in the default production build.

Useful? React with 👍 / 👎.

raise HTTPException(status_code=404, detail="Asset not found")
content = candidate.read_text(encoding="utf-8") if candidate.suffix == ".html" else candidate.read_bytes()
if candidate.suffix != ".html":
return Response(content=content)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve MIME type for static UI assets

For non-HTML files in static mode, the response is returned without forwarding or setting Content-Type. Browsers enforce strict MIME checks for module scripts and styles, so /ui/root/{path} JS/CSS assets can be refused even when they exist, breaking static dashboard loads. Set media_type (or headers) based on the asset extension when returning these files.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant