Production cutover 1.0: manifest, doctor, OW4 /chat proxy, workspace image - #165
Conversation
…orkspace image. Make v2 cutover enforceable: canonical register manifest + CI, doctor observers with staging reds and upstream smokes, /chat reverse-proxy to the workspace openwork door, and a Railway-compatible workspace image (no Dockerfile VOLUME, Node for pnpm, PORT=8787).
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
Warning Review limit reached
Next review available in: 40 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe pull request establishes a versioned production manifest, adds manifest and deployment health checks, routes console chat through OpenWork, exposes implementation markers, updates workspace packaging, and documents production cutover requirements. ChangesCommonPlace production cutover
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ProductionDoctorWorkflow
participant DoctorScripts
participant ConsoleDoctorAPI
participant ProductionServices
ProductionDoctorWorkflow->>DoctorScripts: Run manifest and staging validation
DoctorScripts->>ConsoleDoctorAPI: Probe /api/doctor
ConsoleDoctorAPI->>ProductionServices: Probe registered routes and upstream services
ProductionServices-->>ConsoleDoctorAPI: Return health observations
ConsoleDoctorAPI-->>DoctorScripts: Return structured results
DoctorScripts-->>ProductionDoctorWorkflow: Return pass or fail status
sequenceDiagram
participant Browser
participant ChatMiddleware
participant WorkspaceService
Browser->>ChatMiddleware: Request /chat
ChatMiddleware->>WorkspaceService: Forward request data
WorkspaceService-->>ChatMiddleware: Return chat response
ChatMiddleware-->>Browser: Return response with openwork.chat marker
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 |
code-server honors \$PORT over --bind-addr, so exporting PORT=8787 for Railway healthchecks made the IDE steal the chat door and fail the deploy.
|
Follow-up commit: code-server was binding $PORT (8787) and colliding with OpenWork. Entrypoint now starts code-server with |
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
Pull request overview
This PR implements the “Production cutover 1.0” execution scaffolding by promoting .commonplace-canonical into a JSON manifest, adding a production-boundary “doctor” (script + /doctor + /api/doctor + CI workflow), routing /chat through a console-origin proxy to the Railway workspace openwork door (OW4), and adjusting the Railway workspace image build/runtime behavior.
Changes:
- Promote
.commonplace-canonicalinto a v1 JSON manifest and add CI gates to keep it consistent withregistry.tsxand register-impl mappings. - Add a production doctor across scripts + console routes, and wire it into a deploy-blocking GitHub Actions workflow.
- Implement OW4
/chatfetch-proxy middleware and update workspace chat routes/tests; update workspace Dockerfile + entrypoint for Railway constraints.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/doctor.sh | Bash entrypoint delegating to scripts/doctor.mjs. |
| scripts/doctor.mjs | CLI doctor that probes production health, manifest routes, and upstreams. |
| scripts/doctor-upstream-smokes.mjs | Unauth health + auth smokes for cutover upstream services. |
| scripts/doctor-staging-reds.mjs | “Fail closed” staging-red proofs for manifest/doctor assumptions. |
| scripts/check-register-manifest.mjs | Gate: validates .commonplace-canonical vs registry.tsx + register-impl map. |
| scripts/assert-canonical-root.mjs | Extends canonical-root gate to validate JSON schema when present. |
| packaging/workspace/entrypoint.sh | Prefer PORT for Railway healthchecks and export it for the service. |
| packaging/workspace/Dockerfile | Fix build stages for pnpm workspace + remove VOLUME for Railway. |
| docs/records/013-vscode-surface.md | Adds GL9 production-boundary amendment note. |
| docs/records/012-twenty-ui-fork.md | Adds GL9 production-boundary amendment note. |
| docs/plans/console/SPEC-COMMONPLACE-OPENWORK-FORK-1.0.md | Adds GL9 amendment note. |
| docs/plans/console/SPEC-COMMONPLACE-MODEL-CANVAS-FORK-1.0.md | Adds GL9 amendment note. |
| docs/plans/commonplace-production-cutover/SPEC-COMMONPLACE-PRODUCTION-CUTOVER-1.0.md | New cutover spec defining GL1–GL9 deliverables and acceptance. |
| docs/plans/commonplace-production-cutover/EXECUTE-REPORT.md | New execution report tracking progress and risks. |
| CONVENTIONS.md | New conventions file including GL9 production-boundary rules. |
| apps/console/src/views/ThreadView.tsx | Moves ThreadRuntimeAvailable context out to a stable lib export. |
| apps/console/src/views/registry.tsx | Repoints chat descriptors to OpenworkChatRegister; updates sourcing metadata. |
| apps/console/src/views/OpenworkChatRegister.tsx | New fallback “openwork chat register” UI with data-register-impl. |
| apps/console/src/views/model/ModelView.tsx | Stamps model surface with data-register-impl="model-canvas.owox". |
| apps/console/src/middleware.ts | New OW4 /chat reverse-proxy middleware (console-origin fetch proxy). |
| apps/console/src/lib/thread-runtime-available.ts | New shared ThreadRuntimeAvailable context module. |
| apps/console/src/lib/register-impl.ts | New descriptor-id → manifest impl mapping used for data-register-impl. |
| apps/console/src/components/ConsoleApp.tsx | Updates import for ThreadRuntimeAvailable. |
| apps/console/src/components/chat/ChatPage.tsx | Stamps assistant-ui ChatPage with data-register-impl for superseded tracking. |
| apps/console/src/components/blocks/BlockShell.tsx | Stamps block shell with data-register-impl derived from descriptor id. |
| apps/console/src/components/blocks/AgentRailBlock.tsx | Replaces embedded composer with lightweight submit form using submitThreadText. |
| apps/console/src/app/workspace/[workspaceSlug]/chat/page.tsx | Collapses workspace chat route onto /chat (OW4 proxy owns the body). |
| apps/console/src/app/workspace/[workspaceSlug]/chat/page.test.tsx | Updates tests to assert redirects rather than server-rendered ChatPage. |
| apps/console/src/app/doctor/page.tsx | New human-glance doctor page reading /api/doctor. |
| apps/console/src/app/chat/page.tsx | Keeps /chat as the canonical entry route; returns openwork fallback when proxy absent. |
| apps/console/src/app/chat/[threadId]/page.tsx | Thread route fallback to openwork register when proxy absent. |
| apps/console/src/app/chat/[threadId]/page.test.tsx | Updates tests to assert fallback register behavior and redirect behavior. |
| apps/console/src/app/api/doctor/route.ts | New server-side doctor endpoint with env + route + resurrection inventory. |
| apps/console/package.json | Adds gate:register-manifest and includes it in gates. |
| .harness/checklists/commonplace-production-cutover--plan-local-20260803.json | Adds local harness checklist for GL tracking. |
| .github/workflows/production-doctor.yml | New workflow: staging reds on PRs, live doctor + auth upstream smokes on main. |
| .commonplace-canonical | Converts sentinel marker into v1 JSON manifest including registers/services/env contract. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const headers = new Headers(); | ||
| const accept = request.headers.get('accept'); | ||
| const requestContentType = request.headers.get('content-type'); | ||
| const cookie = request.headers.get('cookie'); | ||
| if (accept) headers.set('accept', accept); | ||
| if (requestContentType) headers.set('content-type', requestContentType); | ||
| if (cookie) headers.set('cookie', cookie); | ||
| headers.set('x-forwarded-host', request.headers.get('host') ?? ''); | ||
| headers.set('x-forwarded-proto', request.nextUrl.protocol.replace(':', '')); | ||
| const token = process.env.CONSOLE_WORKSPACE_TOKEN?.trim(); | ||
| if (token) headers.set('authorization', `Bearer ${token}`); | ||
|
|
| const upstreamContentType = upstream.headers.get('content-type') ?? ''; | ||
| const responseHeaders = new Headers(upstream.headers); | ||
| responseHeaders.set('x-register-impl', 'openwork.chat'); | ||
|
|
||
| if (upstreamContentType.includes('text/html')) { | ||
| const html = await upstream.text(); | ||
| const stamped = html.includes('data-register-impl=') | ||
| ? html | ||
| : html.replace( | ||
| /<html([^>]*)>/i, | ||
| '<html$1 data-register-impl="openwork.chat">', | ||
| ); | ||
| return new NextResponse(stamped, { | ||
| status: upstream.status, | ||
| headers: responseHeaders, | ||
| }); | ||
| } |
| async function probeRoute(base: string, route: string): Promise<{ status: number; impl: string | null; body: string }> { | ||
| const url = route === '/' ? base : `${base}${route}`; | ||
| const response = await fetch(url, { redirect: 'manual' }); | ||
| const body = await response.text(); | ||
| return { status: response.status, impl: extractImpl(body), body }; | ||
| } |
| async function probe(url, init) { | ||
| const response = await fetch(url, init); | ||
| const text = await response.text(); | ||
| return { response, text }; | ||
| } |
![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: b367aaeab5
ℹ️ 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".
| const token = process.env.CONSOLE_WORKSPACE_TOKEN?.trim(); | ||
| if (token) headers.set('authorization', `Bearer ${token}`); |
There was a problem hiding this comment.
When CONSOLE_WORKSPACE_URL and CONSOLE_WORKSPACE_TOKEN are configured, middleware intercepts /chat/* before the page's resolveHarnessPrincipal() check and attaches a shared collaborator bearer token to every request. Consequently, an unauthenticated caller can request paths such as /chat/workspaces or mutation endpoints, have them translated to the corresponding workspace API route, and act with the injected token; forwarding the cookie does not mitigate this because the workspace container explicitly refuses the console cookie-signing secret. Validate the console session and workspace membership before proxying or injecting the credential.
AGENTS.md reference: apps/console/AGENTS.md:L119-L121
Useful? React with 👍 / 👎.
| export const config = { | ||
| matcher: ['/chat', '/chat/:path*'], | ||
| }; |
There was a problem hiding this comment.
With the workspace proxy enabled, /chat returns the Vite app, but the matcher only proxies URLs beginning with /chat. The app is built with base: "/", so its HTML requests /assets/*, and resolveOpenworkConnection() selects window.location.origin, causing API calls such as /session/console and /workspaces; all of those go to the Next console instead of the workspace service. A normal /chat load therefore cannot fetch its bundle or use its API unless the build and connection base are changed to /chat or the required root paths are also proxied.
Useful? React with 👍 / 👎.
| const pass = | ||
| response.status === 200 || | ||
| response.status === 401 || | ||
| response.status === 403 || | ||
| response.status === 404; |
There was a problem hiding this comment.
When the proactivity token is expired, incorrect, or the configured endpoint is missing, this "authenticated" smoke treats 401, 403, and even 404 as success, so the production-doctor workflow can report green without establishing either authentication or endpoint availability. The same false-positive pattern appears in authDataApi() and authHarness(); authenticated checks should require the service's expected successful response rather than merely proving that some HTTP door answered.
Useful? React with 👍 / 👎.
| } | ||
| ], | ||
| "retired": [], | ||
| "env_contract": [ |
There was a problem hiding this comment.
The new canonical chat path depends on CONSOLE_WORKSPACE_URL and CONSOLE_WORKSPACE_TOKEN, but neither appears in env_contract, and /api/doctor derives all required-environment checks solely from this list. In particular, a deployment can have the URL configured but omit the token, still return and stamp the public Openwork HTML for /chat, and be reported green even though workspace API requests cannot authenticate. Include both proxy variables in the required contract so the cutover doctor fails closed.
Useful? React with 👍 / 👎.
| const ok = | ||
| (probed.status >= 200 && probed.status < 400 && probed.impl === row.manifest_impl) || | ||
| (probed.status >= 200 && probed.status < 400 && loginish && probed.impl === null); |
There was a problem hiding this comment.
For the Console routes in the manifest, this check cannot observe the implementation it claims to validate: /records, /Data-model, /documents, /workspace, and / all render ConsoleApp, whose server snapshot is only the aria-busy placeholder, while data-register-impl is added by client-rendered BlockShell instances after hydration. Because probeRoute() merely scans the raw response HTML, these healthy routes produce observed_impl: null; alternatively, the loginish branch accepts login HTML with no implementation marker at all. The production doctor will therefore either reject a healthy deployment or pass an authentication page without proving the registered surface, so this needs a server-visible route stamp or a browser-based authenticated probe.
Useful? React with 👍 / 👎.
| - name: Wait for console health | ||
| run: | | ||
| for i in $(seq 1 30); do | ||
| code=$(curl -sS -o /tmp/healthz.json -w '%{http_code}' https://v2.theoremharness.com/api/healthz || true) | ||
| if [ "$code" = "200" ]; then | ||
| cat /tmp/healthz.json | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
On a main push, this health loop exits as soon as the currently deployed Console returns 200, without checking that production is running the commit which triggered the workflow. The workflow therefore races Railway and usually tests the previous revision; moreover, the file explicitly says Railway should wait for this check, which creates a cycle where the new /doctor implementation cannot deploy until a check requiring that implementation has passed. Trigger the live job from deployment completion or wait for a revision/build identifier matching github.sha before running the doctor.
Useful? React with 👍 / 👎.
| const doctorApi = path.join(repoRoot, 'apps/console/src/app/api/doctor/route.ts'); | ||
| const doctorSource = readFileSync(doctorApi, 'utf8'); | ||
| if (!/pending_retirement|resurrection|retired/.test(doctorSource)) { | ||
| fail('doctor API missing resurrection / pending_retirement handling'); | ||
| } | ||
| console.log('ok red.doctor_source: doctor API carries resurrection checks'); |
There was a problem hiding this comment.
This staging proof never invokes the doctor or asserts a red result; it only searches the route source for any of three words. In the committed manifest the condition at line 69 is also false because the chat superseded entry already has paths, so the planted corpse is never added to retired and would not be part of the doctor's inventory anyway. The CI step can therefore remain green even if resurrection handling always reports success; construct a manifest that names the corpse and execute the actual observation logic, asserting a failing status.
Useful? React with 👍 / 👎.
| // SOURCING: none. Chat page route (CH1). Unscoped /chat redirects into the | ||
| // active workspace chat once membership and scope are verified. | ||
| // SOURCING: none. Chat index stays on /chat so OW4 middleware can reverse-proxy | ||
| // the workspace openwork door. Do not redirect into /workspace/*/chat — that |
There was a problem hiding this comment.
This newly added comment contains an em dash, but the scoped Console constitution explicitly bans em and en dashes in code comments as well as UI strings and Markdown. Replace it with permitted punctuation so the changed file follows the app's writing rule.
AGENTS.md reference: apps/console/AGENTS.md:L169-L172
Useful? React with 👍 / 👎.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (7)
packaging/workspace/Dockerfile (1)
16-18: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin the Node image used by both stages.
node:22-bookworm-slimis a mutable tag. Docker recommends digest pinning for reproducible image content. (docs.docker.com) The final stage also repeats the image reference at Lines 59-60. Use one immutable, verified reference and copy the runtime files from that named stage.Proposed change
-FROM node:22-bookworm-slim AS web +FROM node:22-bookworm-slim@sha256:<verified-digest> AS web ... -COPY --from=node:22-bookworm-slim /usr/local/bin/node /usr/local/bin/node -COPY --from=node:22-bookworm-slim /usr/local/lib/node_modules /usr/local/lib/node_modules +COPY --from=web /usr/local/bin/node /usr/local/bin/node +COPY --from=web /usr/local/lib/node_modules /usr/local/lib/node_modulesAlso applies to: 57-66
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packaging/workspace/Dockerfile` around lines 16 - 18, Pin the Node base image used by both the web and final stages to one verified immutable digest, define that reference once, and reuse the named web stage for copying runtime files instead of repeating the mutable image reference. Update the stage declarations around the web stage and the final-stage COPY flow while preserving the existing runtime contents.Source: MCP tools
.github/workflows/production-doctor.yml (1)
45-46: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winBoth checkout steps persist the job token.
actions/checkout@v4stores the credential in.git/configby default. No step in this workflow pushes to the repository, and both jobs execute repository scripts.
.github/workflows/production-doctor.yml#L45-L46: addwith: persist-credentials: falseto thestaging-redscheckout..github/workflows/production-doctor.yml#L65-L66: addwith: persist-credentials: falseto thedoctorcheckout.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/production-doctor.yml around lines 45 - 46, Disable credential persistence for both checkout steps in .github/workflows/production-doctor.yml at lines 45-46 (staging-reds) and 65-66 (doctor) by configuring each actions/checkout@v4 step with persist-credentials set to false.Source: Linters/SAST tools
scripts/check-register-manifest.mjs (2)
81-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe gate validates by literal source text, so formatting changes change gate coverage. Both checks assume single quotes, one space after each colon, and specific newline placement in TypeScript sources. A formatter run or a quote-style change either silences the swap rule or reports every mapping as missing.
scripts/check-register-manifest.mjs#L81-L87: accept both quote styles and flexible whitespace when extracting descriptor ids, or read ids from a generated JSON export of the registry.scripts/check-register-manifest.mjs#L132-L139: replace theimplSource.includes(expected)literal match with a regex that allows either quote style and flexible whitespace around the colon.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-register-manifest.mjs` around lines 81 - 87, Make the manifest checks formatting-tolerant: in scripts/check-register-manifest.mjs lines 81-87, update the registry id extraction used by registryIds to accept both quote styles and flexible whitespace; in lines 132-139, replace the implSource.includes(expected) literal comparison with a regex allowing either quote style and flexible whitespace around the colon.
110-112: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a
production_routeuniqueness check.The loop rejects duplicate
registry_entryvalues but accepts duplicateproduction_routevalues..commonplace-canonicalcurrently maps botheditorandplanto/workspace, andscripts/doctor.mjscannot satisfy two impl stamps on one route. A uniqueness check here catches that class of manifest error in CI.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-register-manifest.mjs` around lines 110 - 112, Add duplicate detection for row.production_route in the manifest validation loop alongside the existing required-field check, tracking previously seen routes and pushing a validation error when a route repeats. Preserve the existing registry_entry uniqueness behavior and missing-production_route validation.scripts/doctor-upstream-smokes.mjs (1)
47-144: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAuthenticated smokes lack the timeout pattern used by the SSE probe.
authGraphql,authDataApi, andauthHarnesscallfetchwith noAbortController/timeout, unlikeauthProactivity(Lines 146-185), which already uses one. A hung upstream here stalls the whole script until the CI job's own timeout intervenes, instead of failing fast with a clear per-probe message.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/doctor-upstream-smokes.mjs` around lines 47 - 144, Add the same AbortController-based timeout pattern used by authProactivity to authGraphql, authDataApi, and authHarness, passing the signal and timeout to each probe request and cleaning up the timer afterward. Ensure timeout failures are caught and recorded through each probe’s existing record call with a clear per-probe error message.apps/console/src/app/doctor/page.tsx (1)
8-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting doctor logic into a shared module instead of an SSR self-fetch.
This page re-declares
DoctorPayloadto mirror the API route'sEnvRow/RouteRow/ResurrectionRowtypes, then fetches its own/api/doctorroute over HTTP during server rendering. Extracting the manifest read, env check, and route-probe logic into a shared module (e.g.lib/doctor.ts) that bothroute.tsand this page call directly would remove the network hop, remove the type duplication, and remove the dependency onDOCTOR_PUBLIC_BASE_URL/RAILWAY_PUBLIC_DOMAINresolving identically from two separate call sites.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/app/doctor/page.tsx` around lines 8 - 44, Extract the doctor manifest, environment validation, and route-probe logic from the API route into a shared module such as lib/doctor.ts, and have both route.ts and loadDoctor call that module directly during server rendering. Reuse the shared result types instead of redeclaring DoctorPayload and its nested rows in page.tsx, and remove the page’s SSR self-fetch plus DOCTOR_PUBLIC_BASE_URL/RAILWAY_PUBLIC_DOMAIN base-resolution logic while preserving the existing response and error behavior.apps/console/src/views/registry.tsx (1)
208-259: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winUse an existing package for the chat source metadata.
chat.thread,chat.surface, andthread.listpointsource.packageto@commonplace/chat, but no workspace package declares that package andOpenworkChatRegisteris imported from./OpenworkChatRegisterinsideapps/console. Use an existing package path, or add the package definition consistently before resolving through the library ledger.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/console/src/views/registry.tsx` around lines 208 - 259, Update the source metadata for CHAT_THREAD, CHAT_SURFACE, and THREAD_LIST to use the existing package path that owns the locally imported OpenworkChatRegister, or consistently add `@commonplace/chat` to the workspace package definitions before ledger resolution. Ensure all three descriptors resolve through the same valid library-ledger package entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.commonplace-canonical:
- Around line 134-159: Resolve the duplicate production route declared by the
editor and plan register entries in .commonplace-canonical. Either assign
distinct production_route values, or update the /workspace document and its
doctor validation path so it carries both manifest_impl stamps; ensure
checkRegisterRoutes can satisfy both console.code.file and console.goal.stack
without relying on the loginish fallback.
In @.github/workflows/production-doctor.yml:
- Around line 73-89: Update the “Wait for console health” step to require both
HTTP 200 and the deployed commit identity from /api/healthz matching github.sha
before succeeding; otherwise continue polling and fail after the existing
retries. Add curl’s --max-time option to bound each health request, and expose
or parse the health response’s commit SHA without changing the subsequent
doctor.sh invocation.
In `@apps/console/src/app/api/doctor/route.ts`:
- Around line 55-60: Update probeRoute to use an AbortController-based timeout
with fetch, matching the existing timeout pattern in the doctor upstream smoke
script, and ensure the signal is cleaned up after each probe. Update the
manifest probe loop around probeRoute to run probes concurrently rather than
awaiting them sequentially, while preserving the existing result collection and
response behavior.
- Around line 119-148: Update the ok calculation in the doctor route to fail
when any resurrections entry from manifest.retired has pending_retirement false
and absent false, while preserving the existing reporting for superseded
entries. Include this resurrection check alongside envOk and routesOk so
restored retired files make the doctor red and deploy-blocking.
- Line 57: Update the sign-in fallback around the fetch using redirect: 'manual'
to recognize a real 3xx response by checking response.headers.get('location').
Accept the redirect-based auth gate without requiring probed.impl === null,
while preserving the existing response/body handling for other cases.
In `@apps/console/src/app/chat/page.tsx`:
- Around line 15-17: Update the unavailable-state branch in the chat page to
derive settingsHref from resolution.principal.workspaceId: pass
/workspace/${resolution.principal.workspaceId}/settings when a workspace ID
exists, and retain null only when no workspace is known. Keep the existing
ChatUnavailable condition and rendering behavior otherwise unchanged.
In `@apps/console/src/app/doctor/page.tsx`:
- Around line 25-44: Add an explicit timeout to the fetch in loadDoctor by using
an AbortController or equivalent signal with a bounded timeout, and ensure
timeout failures follow the existing catch fallback. Keep the request URL and
no-store caching behavior unchanged.
In `@apps/console/src/components/blocks/AgentRailBlock.tsx`:
- Line 14: Update the AgentRailBlock rail-field submission flow to route
submitted text through submitThreadText, preserving the existing thread-submit
contract and ensuring /do action instructions are sent as user input rather than
opening the action sheet. If a native form handler remains, add equivalent /do
routing there.
In `@apps/console/src/lib/register-impl.ts`:
- Around line 19-25: Update registerImplForDescriptor to verify descriptorId is
an own key of REGISTER_IMPL_BY_DESCRIPTOR before indexing it; return undefined
for inherited or unknown keys such as constructor and __proto__, while
preserving the existing empty-id behavior and valid descriptor lookups.
In `@apps/console/src/middleware.ts`:
- Around line 44-48: Update the catch block in the middleware’s `/chat` request
handling to log the caught error server-side, then replace the interpolated
error details in the 502 HTML response with a generic workspace-unavailable
message. Do not expose `error.message` or its stringified equivalent to
unauthenticated callers.
- Around line 17-18: Update the upstream target construction around upstreamPath
and target to prevent paths beginning with // from becoming a different URL
host; reject or canonicalize doubled leading slashes after removing /chat. Build
the URL from WORKSPACE by assigning the sanitized pathname and
request.nextUrl.search separately, preserving the intended proxy origin.
- Around line 41-43: Update the upstream proxy fetch in the middleware around
fetch(target, init) to use an AbortController with an explicit timeout, passing
its signal through the request init and clearing the timer after completion.
Ensure timeout-triggered aborts follow the existing upstream error handling
path.
In `@apps/console/src/views/OpenworkChatRegister.tsx`:
- Line 20: Update the inline font weight on the “Openwork chat register” element
in OpenworkChatRegister to use the appropriate Int UI weight token, keeping the
element within the existing Int UI token system and removing the
--rec-weight-cap reference.
- Around line 22-27: Replace the internal-navigation anchor in
OpenworkChatRegister with Next.js’s Link component imported from next/link,
preserving the existing className, href="/chat", and link text.
In `@packaging/workspace/entrypoint.sh`:
- Around line 18-21: Update the entrypoint port initialization and related
Dockerfile configuration: remove the image-level OPENWORK_PORT default, export
the shell-resolved port variable without overriding runtime-provided PORT
values, and make the healthcheck use that same resolved port rather than
performing a separate OPENWORK_PORT lookup.
In `@scripts/doctor-staging-reds.mjs`:
- Around line 58-89: The Red 2 proof must execute the doctor’s
retirement/resurrection evaluation instead of grepping route source. Extract the
relevant logic from the doctor API route into an importable function or module,
have the route reuse it, and update this script to invoke that function against
the mutated marker and planted corpse, asserting a non-green result.
- Around line 62-65: Update the generated content in writeFileSync to replace
the em dash in the corpse file comment with allowed punctuation, while
preserving the rest of the comment and export statement.
In `@scripts/doctor.mjs`:
- Around line 71-74: Replace the broad body-text-based loginish fallback near
the route check with validation of an actual authentication redirect: compare
response.url to the expected sign-in path, or use a manual redirect request and
accept only a 3xx redirect targeting that path. Ensure routes without the
cutover marker cannot pass merely because their HTML contains terms such as
“login” or “callbackUrl”.
- Around line 41-48: Update fetchText to apply an abort timeout to every fetch
request, including calls that provide init options, so hung probes terminate
within a bounded interval and report failure. Use the existing fetch signal when
present or compose the timeout signal without overriding caller configuration.
---
Nitpick comments:
In @.github/workflows/production-doctor.yml:
- Around line 45-46: Disable credential persistence for both checkout steps in
.github/workflows/production-doctor.yml at lines 45-46 (staging-reds) and 65-66
(doctor) by configuring each actions/checkout@v4 step with persist-credentials
set to false.
In `@apps/console/src/app/doctor/page.tsx`:
- Around line 8-44: Extract the doctor manifest, environment validation, and
route-probe logic from the API route into a shared module such as lib/doctor.ts,
and have both route.ts and loadDoctor call that module directly during server
rendering. Reuse the shared result types instead of redeclaring DoctorPayload
and its nested rows in page.tsx, and remove the page’s SSR self-fetch plus
DOCTOR_PUBLIC_BASE_URL/RAILWAY_PUBLIC_DOMAIN base-resolution logic while
preserving the existing response and error behavior.
In `@apps/console/src/views/registry.tsx`:
- Around line 208-259: Update the source metadata for CHAT_THREAD, CHAT_SURFACE,
and THREAD_LIST to use the existing package path that owns the locally imported
OpenworkChatRegister, or consistently add `@commonplace/chat` to the workspace
package definitions before ledger resolution. Ensure all three descriptors
resolve through the same valid library-ledger package entry.
In `@packaging/workspace/Dockerfile`:
- Around line 16-18: Pin the Node base image used by both the web and final
stages to one verified immutable digest, define that reference once, and reuse
the named web stage for copying runtime files instead of repeating the mutable
image reference. Update the stage declarations around the web stage and the
final-stage COPY flow while preserving the existing runtime contents.
In `@scripts/check-register-manifest.mjs`:
- Around line 81-87: Make the manifest checks formatting-tolerant: in
scripts/check-register-manifest.mjs lines 81-87, update the registry id
extraction used by registryIds to accept both quote styles and flexible
whitespace; in lines 132-139, replace the implSource.includes(expected) literal
comparison with a regex allowing either quote style and flexible whitespace
around the colon.
- Around line 110-112: Add duplicate detection for row.production_route in the
manifest validation loop alongside the existing required-field check, tracking
previously seen routes and pushing a validation error when a route repeats.
Preserve the existing registry_entry uniqueness behavior and
missing-production_route validation.
In `@scripts/doctor-upstream-smokes.mjs`:
- Around line 47-144: Add the same AbortController-based timeout pattern used by
authProactivity to authGraphql, authDataApi, and authHarness, passing the signal
and timeout to each probe request and cleaning up the timer afterward. Ensure
timeout failures are caught and recorded through each probe’s existing record
call with a clear per-probe error message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 21165cde-ea0c-4d61-a769-1a353fba2d61
📒 Files selected for processing (37)
.commonplace-canonical.github/workflows/production-doctor.yml.harness/checklists/commonplace-production-cutover--plan-local-20260803.jsonCONVENTIONS.mdapps/console/package.jsonapps/console/src/app/api/doctor/route.tsapps/console/src/app/chat/[threadId]/page.test.tsxapps/console/src/app/chat/[threadId]/page.tsxapps/console/src/app/chat/page.tsxapps/console/src/app/doctor/page.tsxapps/console/src/app/workspace/[workspaceSlug]/chat/page.test.tsxapps/console/src/app/workspace/[workspaceSlug]/chat/page.tsxapps/console/src/components/ConsoleApp.tsxapps/console/src/components/blocks/AgentRailBlock.tsxapps/console/src/components/blocks/BlockShell.tsxapps/console/src/components/chat/ChatPage.tsxapps/console/src/lib/register-impl.tsapps/console/src/lib/thread-runtime-available.tsapps/console/src/middleware.tsapps/console/src/views/OpenworkChatRegister.tsxapps/console/src/views/ThreadView.tsxapps/console/src/views/model/ModelView.tsxapps/console/src/views/registry.tsxdocs/plans/commonplace-production-cutover/EXECUTE-REPORT.mddocs/plans/commonplace-production-cutover/SPEC-COMMONPLACE-PRODUCTION-CUTOVER-1.0.mddocs/plans/console/SPEC-COMMONPLACE-MODEL-CANVAS-FORK-1.0.mddocs/plans/console/SPEC-COMMONPLACE-OPENWORK-FORK-1.0.mddocs/records/012-twenty-ui-fork.mddocs/records/013-vscode-surface.mdpackaging/workspace/Dockerfilepackaging/workspace/entrypoint.shscripts/assert-canonical-root.mjsscripts/check-register-manifest.mjsscripts/doctor-staging-reds.mjsscripts/doctor-upstream-smokes.mjsscripts/doctor.mjsscripts/doctor.sh
| { | ||
| "id": "editor", | ||
| "canonical_package": "apps/console CodeFileView + editor-model seam (+ vscode-surface pack when parked/live)", | ||
| "production_route": "/workspace", | ||
| "registry_entry": "code.file", | ||
| "manifest_impl": "console.code.file", | ||
| "superseded": [], | ||
| "parked": [ | ||
| { | ||
| "impl": "vscode-surface.pack", | ||
| "reason": "PR #162/#163 merged; V7 desktop/web workbench smoke still pending before doctor can observe a live IDE door.", | ||
| "prs": [ | ||
| 162, | ||
| 163 | ||
| ] | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| "id": "plan", | ||
| "canonical_package": "apps/console GoalStackView", | ||
| "production_route": "/workspace", | ||
| "registry_entry": "goal.stack", | ||
| "manifest_impl": "console.goal.stack", | ||
| "superseded": [] | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Two registers claim the same production_route, which the doctor cannot both satisfy.
editor and plan both declare production_route: "/workspace" with distinct manifest_impl values (console.code.file, console.goal.stack). scripts/doctor.mjs checkRegisterRoutes fetches each row's route and requires data-register-impl="<manifest_impl>" in the HTML. A single /workspace response can carry only one of the two stamps, so one row stays red unless the loginish fallback masks it. Either stamp both impl ids on the /workspace document, or give each register a distinct route.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.commonplace-canonical around lines 134 - 159, Resolve the duplicate
production route declared by the editor and plan register entries in
.commonplace-canonical. Either assign distinct production_route values, or
update the /workspace document and its doctor validation path so it carries both
manifest_impl stamps; ensure checkRegisterRoutes can satisfy both
console.code.file and console.goal.stack without relying on the loginish
fallback.
| - name: Wait for console health | ||
| run: | | ||
| for i in $(seq 1 30); do | ||
| code=$(curl -sS -o /tmp/healthz.json -w '%{http_code}' https://v2.theoremharness.com/api/healthz || true) | ||
| if [ "$code" = "200" ]; then | ||
| cat /tmp/healthz.json | ||
| exit 0 | ||
| fi | ||
| sleep 10 | ||
| done | ||
| echo "console health did not become ready" | ||
| exit 1 | ||
|
|
||
| - name: Run doctor.sh against production | ||
| run: bash scripts/doctor.sh | ||
| env: | ||
| DOCTOR_BASE_URL: https://v2.theoremharness.com |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
The health wait does not prove the new commit is live, so the doctor can grade the previous deploy.
On a push to main, this job starts immediately. The already-running deployment returns 200 from /api/healthz, so the loop exits on the first attempt. scripts/doctor.mjs then checks routes and impl markers against the old build. The job reports green for a cutover that is not deployed yet.
Gate the wait on deploy identity, not liveness. Return the commit SHA from /api/healthz and loop until it equals github.sha. Alternatively, trigger this job from a Railway deployment success event rather than from the push.
Also add --max-time to the curl call so one hung request does not consume a full 10 second interval plus an unbounded connect wait.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/production-doctor.yml around lines 73 - 89, Update the
“Wait for console health” step to require both HTTP 200 and the deployed commit
identity from /api/healthz matching github.sha before succeeding; otherwise
continue polling and fail after the existing retries. Add curl’s --max-time
option to bound each health request, and expose or parse the health response’s
commit SHA without changing the subsequent doctor.sh invocation.
| async function probeRoute(base: string, route: string): Promise<{ status: number; impl: string | null; body: string }> { | ||
| const url = route === '/' ? base : `${base}${route}`; | ||
| const response = await fetch(url, { redirect: 'manual' }); | ||
| const body = await response.text(); | ||
| return { status: response.status, impl: extractImpl(body), body }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
No timeout on production route probes; a hung upstream can block this request indefinitely.
probeRoute (Lines 55-60) calls fetch(url, { redirect: 'manual' }) with no AbortSignal/timeout. The loop at Lines 90-117 then awaits this sequentially, once per manifest register. If the /chat OW4 proxy or any other probed route hangs (e.g., a slow Railway workspace door), this dynamic = 'force-dynamic' route has no bound on total request time, and doctor/page.tsx (which itself fetches this route with no timeout) inherits the hang.
scripts/doctor-upstream-smokes.mjs already uses AbortController with a timeout for its SSE probe; apply the same pattern here.
🔒️ Proposed fix to add a timeout and probe concurrently
async function probeRoute(base: string, route: string): Promise<{ status: number; impl: string | null; body: string }> {
const url = route === '/' ? base : `${base}${route}`;
- const response = await fetch(url, { redirect: 'manual' });
+ const response = await fetch(url, { redirect: 'manual', signal: AbortSignal.timeout(5000) });
const body = await response.text();
return { status: response.status, impl: extractImpl(body), body };
}- const routes: RouteRow[] = [];
- for (const row of manifest.registers ?? []) {
- try {
- const probed = await probeRoute(base, row.production_route);
- ...
- } catch (error) {
- routes.push({...});
- void error;
- }
- }
+ const routes: RouteRow[] = await Promise.all(
+ (manifest.registers ?? []).map(async (row) => {
+ try {
+ const probed = await probeRoute(base, row.production_route);
+ const loginish = /callbackUrl|sign.?in|\/login/i.test(probed.body);
+ const ok = /* unchanged */;
+ return { id: row.id, route: row.production_route, expected_impl: row.manifest_impl, status: probed.status, observed_impl: probed.impl, ok };
+ } catch (error) {
+ console.error(`doctor: probe failed for ${row.id}`, error);
+ return { id: row.id, route: row.production_route, expected_impl: row.manifest_impl, status: null, observed_impl: null, ok: false };
+ }
+ }),
+ );Also applies to: 90-117
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/console/src/app/api/doctor/route.ts` around lines 55 - 60, Update
probeRoute to use an AbortController-based timeout with fetch, matching the
existing timeout pattern in the doctor upstream smoke script, and ensure the
signal is cleaned up after each probe. Update the manifest probe loop around
probeRoute to run probes concurrently rather than awaiting them sequentially,
while preserving the existing result collection and response behavior.
|
|
||
| async function probeRoute(base: string, route: string): Promise<{ status: number; impl: string | null; body: string }> { | ||
| const url = route === '/' ? base : `${base}${route}`; | ||
| const response = await fetch(url, { redirect: 'manual' }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Node.js 22 undici fetch redirect manual response status and body behavior
💡 Result:
In Node.js 22, the Undici-based fetch implementation handles the redirect: 'manual' option by returning the actual HTTP response from the server, rather than the opaque-redirect filtered response defined by the web browser Fetch standard [1][2]. Key behaviors include: 1. Status and Body: Unlike browser fetch—which returns an opaque-redirect response (status 0, null body, empty headers) [3]—Undici returns the complete response, including the actual 3xx status code (e.g., 301, 302, 307, 308) [1][4]. You can read the response body and access the Location header to determine where the redirect points [5][4]. 2. Rationale: This approach is designed to align with server-side environments like Deno and Cloudflare Workers, where it is not possible for the client to automatically handle certain redirect scenarios that might be expected in a browser [1][2]. 3. Implementation: This is a deviation from strict WHATWG Fetch specification compliance regarding the 'manual' redirect mode, which mandates an opaque-redirect response [1][6][4]. Because Undici returns the full response, the developer is responsible for manually inspecting the status, parsing the Location header, and initiating a new request if they wish to follow the redirect [5][4]. In summary, when you use { redirect: 'manual' } in Node.js 22, you receive the full 3xx response object, allowing you to access the status, headers, and body just as you would for any other successful fetch request [1][4].
Citations:
- 1: https://undici.nodejs.org/
- 2: feat(fetch): allow
manualredirect handling nodejs/undici#1210 - 3: https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch
- 4: Fetch: Allow manual redirect handling nodejs/undici#1193
- 5: https://github.com/node-fetch/node-fetch/
- 6: TypeError when redirect is set to 'manual' nodejs/undici#1072
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tracked route files:"
git ls-files | rg 'apps/console/src/app/api/doctor/route\.ts$|apps/console' | head -50
echo
echo "Target route excerpt:"
if [ -f apps/console/src/app/api/doctor/route.ts ]; then
wc -l apps/console/src/app/api/doctor/route.ts
sed -n '1,140p' apps/console/src/app/api/doctor/route.ts | cat -n
fi
echo
echo "Package/runtime declarations:"
for f in apps/package.json package.json apps/console/package.json apps/console/pnpm-workspace.yaml pnpm-workspace.yaml; do
[ -f "$f" ] && { echo "--- $f"; sed -n '1,120p' "$f"; }
done
echo
echo "Search for auth redirect/sign-in patterns under apps/console:"
rg -n --glob '!**/node_modules/**' 'Location|callbackUrl|sign(.* )?in|/login|THEOREM_|CONSOLE_HARNESS_URL|fetch\(|redirect:\s*["'\'']manual["'\'']' apps/console/src apps/console/app 2>/dev/null | head -200Repository: Travis-Gilbert/CommonPlace
Length of output: 3055
🏁 Script executed:
#!/bin/bash
set -euo fish -o pipefail
echo "Repository top files:"
git ls-files | sed -n '1,80p'
echo
echo "Find doctor route by filename (case-insensitive):"
git ls-files | grep -E '(^|/)doctor/route\.ts$|/doctor/route\.ts$' || true
echo
echo "Find Next app package files:"
git ls-files | grep -E '(^|/)package\.json$' | xargs -r -n1 sh -c 'echo "--- $0"; grep -En "\"(name|engines|node|next|undici)\"|\"type\":\"module\"|\"type\":\"commonjs\"|\"next\":|^" || true' "$0" | sed -n '1,120p'
echo
echo "Search fetch/manual redirect and login body checks:"
git ls-files -z | xargs -0 grep -InE 'fetch\(|redirect:\s*"?manual"?|console\.text\(probed|probed\.body|callbackUrl|sign(.* )?in|/login|Location|response\.status' apps 2>/dev/null | sed -n '1,240p'Repository: Travis-Gilbert/CommonPlace
Length of output: 209
🏁 Script executed:
#!/bin/bash
set -u
echo "Repository top files:"
git ls-files | sed -n '1,80p'
echo
echo "Find doctor route by filename:"
git ls-files | grep -E '(^|/)doctor/route\.ts$|/doctor/route\.ts$' || true
echo
echo "Find relevant package/runtime declarations:"
tmp="$(mktemp)"
git ls-files '*package.json' > "$tmp"
while IFS= read -r f; do
echo "--- $f"
grep -En '"(name|engines|next|undici|node)"|type|scripts|devDependencies|dependencies' "$f" 2>/dev/null | sed -n '1,140p' || true
done < "$tmp"
rm -f "$tmp"
echo
echo "Search fetch/manual redirect and login body checks under apps:"
git ls-files | xargs grep -InE 'fetch\(|redirect:\s*"?manual"?|console\.text\(probed|probed\.body|callbackUrl|sign(.* )?in|/login|Location|response\.status' apps 2>/dev/null | sed -n '1,240p' || trueRepository: Travis-Gilbert/CommonPlace
Length of output: 47814
🏁 Script executed:
#!/bin/bash
set -u
echo "Target route excerpt:"
sed -n '1,140p' apps/console/src/app/api/doctor/route.ts | cat -n
echo
echo "Console package relevant fields:"
python3 - <<'PY'
import json
from pathlib import Path
p = Path("apps/console/package.json")
if p.exists():
data = json.loads(p.read_text())
for key in ("name", "version", "scripts", "dependencies", "devDependencies", "packageManager", "engines"):
if key in data:
print(f"{key}:")
for k, v in data[key].items() if isinstance(data[key], dict) else enumerate([data[key]]):
print(f" {k}: {v}")
PY
echo
echo "Root package relevant fields:"
python3 - <<'PY'
import json
from pathlib import Path
p = Path("package.json")
if p.exists():
data = json.loads(p.read_text())
for key in ("name", "version", "scripts", "dependencies", "devDependencies", "packageManager", "engines"):
if key in data:
print(f"{key}:")
for k, v in data[key].items() if isinstance(data[key], dict) else enumerate([data[key]]):
print(f" {k}: {v}")
PY
echo
echo "Node version file:"
for f in .nvmrc node_modules/node/.version node_modules/.bin/node; do
[ -e "$f" ] && { echo "--- $f"; cat "$f" | sed -n '1,16p'; }
doneRepository: Travis-Gilbert/CommonPlace
Length of output: 11972
Check the sign-in fallback for the actual response shape.
Node 22 Undici returns the real 3xx response with headers/body for redirect: 'manual', so the Location header is available for a redirect-based auth gate. If this route can be served by one, also test response.headers.get('location') rather than relying only on probed.body, and avoid requiring probed.impl === null before accepting the redirect page.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/console/src/app/api/doctor/route.ts` at line 57, Update the sign-in
fallback around the fetch using redirect: 'manual' to recognize a real 3xx
response by checking response.headers.get('location'). Accept the redirect-based
auth gate without requiring probed.impl === null, while preserving the existing
response/body handling for other cases.
| const resurrections: ResurrectionRow[] = []; | ||
| for (const row of manifest.registers ?? []) { | ||
| for (const item of row.superseded ?? []) { | ||
| for (const filePath of item.paths ?? []) { | ||
| const abs = path.join(repoRoot(), filePath); | ||
| resurrections.push({ | ||
| id: item.impl, | ||
| path: filePath, | ||
| absent: !existsSync(abs), | ||
| pending_retirement: true, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| for (const row of manifest.retired ?? []) { | ||
| for (const filePath of row.paths ?? []) { | ||
| const abs = path.join(repoRoot(), filePath); | ||
| resurrections.push({ | ||
| id: row.id, | ||
| path: filePath, | ||
| absent: !existsSync(abs), | ||
| pending_retirement: false, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| // Until retirements land, superseded paths still exist. Doctor reports them | ||
| // without failing the overall cutover while deletion_deadline has not passed. | ||
| const routesOk = routes.every((row) => row.ok); | ||
| const ok = envOk && routesOk; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
ok ignores resurrection results; a resurrected retired file will not turn the doctor red.
resurrections is computed at Lines 119-143, but Line 147-148 computes ok from envOk and routesOk only. No resurrections-derived value ever feeds into ok.
For entries with pending_retirement: false (built from manifest.retired), absent: false means a deleted implementation has come back. GL8 in SPEC-COMMONPLACE-PRODUCTION-CUTOVER-1.0.md requires: "the doctor's resurrection smokes are green and demonstrably red when a deleted path is restored in a staging test." As written, restoring a retired file cannot fail the doctor: /api/doctor still reports ok: true and /doctor's header still renders "Green" even while the corresponding resurrection row shows "STILL PRESENT." This defeats the doctor's core purpose as a deploy-blocking gate for GL8.
Gate ok on retired-and-resurfaced entries too.
🐛 Proposed fix to gate `ok` on resurrections
// Until retirements land, superseded paths still exist. Doctor reports them
// without failing the overall cutover while deletion_deadline has not passed.
const routesOk = routes.every((row) => row.ok);
- const ok = envOk && routesOk;
+ const retiredResurfaced = resurrections.filter(
+ (row) => row.pending_retirement === false && !row.absent,
+ );
+ const resurrectionsOk = retiredResurfaced.length === 0;
+ const ok = envOk && routesOk && resurrectionsOk;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const resurrections: ResurrectionRow[] = []; | |
| for (const row of manifest.registers ?? []) { | |
| for (const item of row.superseded ?? []) { | |
| for (const filePath of item.paths ?? []) { | |
| const abs = path.join(repoRoot(), filePath); | |
| resurrections.push({ | |
| id: item.impl, | |
| path: filePath, | |
| absent: !existsSync(abs), | |
| pending_retirement: true, | |
| }); | |
| } | |
| } | |
| } | |
| for (const row of manifest.retired ?? []) { | |
| for (const filePath of row.paths ?? []) { | |
| const abs = path.join(repoRoot(), filePath); | |
| resurrections.push({ | |
| id: row.id, | |
| path: filePath, | |
| absent: !existsSync(abs), | |
| pending_retirement: false, | |
| }); | |
| } | |
| } | |
| // Until retirements land, superseded paths still exist. Doctor reports them | |
| // without failing the overall cutover while deletion_deadline has not passed. | |
| const routesOk = routes.every((row) => row.ok); | |
| const ok = envOk && routesOk; | |
| const resurrections: ResurrectionRow[] = []; | |
| for (const row of manifest.registers ?? []) { | |
| for (const item of row.superseded ?? []) { | |
| for (const filePath of item.paths ?? []) { | |
| const abs = path.join(repoRoot(), filePath); | |
| resurrections.push({ | |
| id: item.impl, | |
| path: filePath, | |
| absent: !existsSync(abs), | |
| pending_retirement: true, | |
| }); | |
| } | |
| } | |
| } | |
| for (const row of manifest.retired ?? []) { | |
| for (const filePath of row.paths ?? []) { | |
| const abs = path.join(repoRoot(), filePath); | |
| resurrections.push({ | |
| id: row.id, | |
| path: filePath, | |
| absent: !existsSync(abs), | |
| pending_retirement: false, | |
| }); | |
| } | |
| } | |
| // Until retirements land, superseded paths still exist. Doctor reports them | |
| // without failing the overall cutover while deletion_deadline has not passed. | |
| const routesOk = routes.every((row) => row.ok); | |
| const retiredResurfaced = resurrections.filter( | |
| (row) => row.pending_retirement === false && !row.absent, | |
| ); | |
| const resurrectionsOk = retiredResurfaced.length === 0; | |
| const ok = envOk && routesOk && resurrectionsOk; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/console/src/app/api/doctor/route.ts` around lines 119 - 148, Update the
ok calculation in the doctor route to fail when any resurrections entry from
manifest.retired has pending_retirement false and absent false, while preserving
the existing reporting for superseded entries. Include this resurrection check
alongside envOk and routesOk so restored retired files make the doctor red and
deploy-blocking.
| # Railway healthchecks probe $PORT. Prefer OPENWORK_PORT, then PORT, then 8787. | ||
| OPENWORK_PORT="${OPENWORK_PORT:-${PORT:-8787}}" | ||
| CODE_SERVER_PORT="${CODE_SERVER_PORT:-8080}" | ||
| export PORT="${OPENWORK_PORT}" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files | rg '(^|/)entrypoint\.sh$|(^|/)Dockerfile$|config\.ts$' | sed -n '1,120p'
echo
echo "== entrypoint.sh relevant lines =="
fd -a 'entrypoint\.sh$' . | while read -r f; do
echo "--- $f"
nl -ba "$f" | sed -n '1,80p'
done
echo
echo "== search OPENWORK_PORT/PORT references =="
rg -n "OPENWORK_PORT|CODE_SERVER_PORT|process\.env\.(OPENWORK_PORT|PORT)|PORT\s*=" -S . | sed -n '1,220p'
echo
echo "== config.ts likely section =="
git ls-files | rg 'apps/chat-server/src/config\.ts$' | while read -r f; do
echo "--- $f"
nl -ba "$f" | sed -n '290,330p'
done
echo
echo "== workspace Dockerfile relevant lines =="
git ls-files | rg 'packaging/workspace/Dockerfile$' | while read -r f; do
echo "--- $f"
nl -ba "$f" | sed -n '90,150p'
doneRepository: Travis-Gilbert/CommonPlace
Length of output: 1409
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== entrypoint.sh =="
awk '{printf "%5d %s\n", NR, $0}' packaging/workspace/entrypoint.sh | sed -n '1,180p'
echo
echo "== workspace Dockerfile relevant lines =="
awk '{printf "%5d %s\n", NR, $0}' packaging/workspace/Dockerfile | sed -n '1,190p'
echo
echo "== chat-server config OPENWORK_PORT section =="
awk '{printf "%5d %s\n", NR, $0}' apps/chat-server/src/config.ts | sed -n '280,340p'
echo
echo "== all OPENWORK_PORT/PORT references in relevant files =="
for f in apps/chat-server/src/config.ts apps/chat-server/src/openwork-runtime-config.ts packaging/workspace/entrypoint.sh packaging/workspace/Dockerfile; do
if [ -f "$f" ]; then
echo "--- $f"
rg -n "OPENWORK_PORT|PORT|export |ENV " "$f" || true
fi
done
echo
echo "== shell default semantics probe =="
python3 - <<'PY'
import os, subprocess, tempfile, textwrap
cases = [
("unset_env_unset", {}, ""),
("unset_env_empty", {}, ""),
("unset_env_default", {}, ""),
("port_only", {"PORT": "3000"}, ""),
("openwork_only", {}, "OPENWORK_PORT=9999"),
("both_override_port", {"PORT": "3000"}, "OPENWORK_PORT=9999"),
("openwork_override_default", {}, "OPENWORK_PORT=9999"),
]
for name, env_vars, extra in cases:
cfg = dict(os.environ)
cfg.pop("OPENWORK_PORT", None)
cfg.pop("PORT", None)
cfg.update(env_vars)
script = textwrap.dedent(f'''
set -e
: {extra}
export OPENWORK_PORT="${{OPENWORK_PORT:-{{PORT:-8787}}}}"
export PORT="${{OPENWORK_PORT}}"
echo "$OPENWORK_PORT $PORT"
''').strip()
result = subprocess.check_output(["/bin/sh", "-c", script], env=cfg, text=True).strip()
print(f"{name}: {result}")
PYRepository: Travis-Gilbert/CommonPlace
Length of output: 21746
Do not override runtime port variables with image defaults.
packaging/workspace/Dockerfile defines ENV OPENWORK_PORT=8787, so Railway-provided PORT cannot flow through the entrypoint or chat-server config. Remove the image-level default, export the variable that the shell resolves, and let the healthcheck use the same precedence instead of its own OPENWORK_PORT lookup.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packaging/workspace/entrypoint.sh` around lines 18 - 21, Update the
entrypoint port initialization and related Dockerfile configuration: remove the
image-level OPENWORK_PORT default, export the shell-resolved port variable
without overriding runtime-provided PORT values, and make the healthcheck use
that same resolved port rather than performing a separate OPENWORK_PORT lookup.
Source: MCP tools
| // Red 2: plant a superseded corpse under a retired path name and assert doctor | ||
| // API logic would treat it as present. We call the resurrection helper shape by | ||
| // writing a file the doctor resurrection inventory names. | ||
| mkdirSync(corpseDir, { recursive: true }); | ||
| writeFileSync( | ||
| corpse, | ||
| '// doctor staging red corpse — must not remain after this script\nexport {};\n', | ||
| ); | ||
|
|
||
| const restored = JSON.parse(readFileSync(marker, 'utf8')); | ||
| const chat = restored.registers.find((row) => row.id === 'chat'); | ||
| if (!chat?.superseded?.[0]?.paths?.length && !restored.retired?.length) { | ||
| // Force a retired entry pointing at the planted corpse for this proof. | ||
| restored.retired = [ | ||
| { | ||
| id: 'doctor-staging-red-corpse', | ||
| paths: ['apps/console/src/components/chat/__doctor_red_corpse__.tsx'], | ||
| retired_at: 'staging-red', | ||
| }, | ||
| ]; | ||
| writeFileSync(marker, `${JSON.stringify(restored, null, 2)}\n`); | ||
| } | ||
|
|
||
| const doctorApi = path.join(repoRoot, 'apps/console/src/app/api/doctor/route.ts'); | ||
| const doctorSource = readFileSync(doctorApi, 'utf8'); | ||
| if (!/pending_retirement|resurrection|retired/.test(doctorSource)) { | ||
| fail('doctor API missing resurrection / pending_retirement handling'); | ||
| } | ||
| console.log('ok red.doctor_source: doctor API carries resurrection checks'); | ||
|
|
||
| if (!existsSync(corpse)) fail('failed to plant corpse'); | ||
| console.log('ok red.corpse_present: planted superseded path for staging proof'); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Red 2 does not prove that the doctor fails closed.
The header at Lines 3-4 states the script proves the doctor fails closed when a superseded corpse path still exists. Red 2 does three things instead: it plants a file, it optionally writes a retired row into the marker, and it greps apps/console/src/app/api/doctor/route.ts for the strings pending_retirement|resurrection|retired. A source grep passes even when the resurrection logic is inverted or unreachable. The mutated marker written at Lines 71-78 is never consumed by any check.
Invoke the retirement/resurrection evaluation directly and assert a non-green result. If the logic lives inside the route handler, extract it into an importable module so both the route and this proof call the same function.
Do you want me to draft that extraction and the corresponding assertion?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/doctor-staging-reds.mjs` around lines 58 - 89, The Red 2 proof must
execute the doctor’s retirement/resurrection evaluation instead of grepping
route source. Extract the relevant logic from the doctor API route into an
importable function or module, have the route reuse it, and update this script
to invoke that function against the mutated marker and planted corpse, asserting
a non-green result.
| writeFileSync( | ||
| corpse, | ||
| '// doctor staging red corpse — must not remain after this script\nexport {};\n', | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the em dash from the generated file content.
The corpse file is written to apps/console/src/components/chat/__doctor_red_corpse__.tsx, and its comment contains an em dash.
✏️ Proposed fix
- '// doctor staging red corpse — must not remain after this script\nexport {};\n',
+ '// doctor staging red corpse: must not remain after this script\nexport {};\n',As per coding guidelines: "Do not use em dashes or en dashes in code comments, UI strings, or markdown."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| writeFileSync( | |
| corpse, | |
| '// doctor staging red corpse — must not remain after this script\nexport {};\n', | |
| ); | |
| writeFileSync( | |
| corpse, | |
| '// doctor staging red corpse: must not remain after this script\nexport {};\n', | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/doctor-staging-reds.mjs` around lines 62 - 65, Update the generated
content in writeFileSync to replace the em dash in the corpse file comment with
allowed punctuation, while preserving the rest of the comment and export
statement.
Source: Coding guidelines
| async function fetchText(url, init) { | ||
| const response = await fetch(url, { | ||
| redirect: 'follow', | ||
| ...init, | ||
| }); | ||
| const text = await response.text(); | ||
| return { response, text }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a timeout to every probe.
fetch has no timeout here. A hung upstream leaves the doctor job blocked until the CI runner limit expires, instead of reporting red. .github/workflows/production-doctor.yml runs this script as a gate.
🛡️ Proposed fix
async function fetchText(url, init) {
const response = await fetch(url, {
redirect: 'follow',
+ signal: AbortSignal.timeout(Number(process.env.DOCTOR_TIMEOUT_MS || 15000)),
...init,
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/doctor.mjs` around lines 41 - 48, Update fetchText to apply an abort
timeout to every fetch request, including calls that provide init options, so
hung probes terminate within a bounded interval and report failure. Use the
existing fetch signal when present or compose the timeout signal without
overriding caller configuration.
| // Unauthenticated routes may redirect to login HTML without the marker. | ||
| // Accept 200 with marker, or a login redirect that still returns HTML. | ||
| const loginish = response.status === 200 && /callbackUrl|sign.?in|login/i.test(text); | ||
| const pass = response.ok && (hasMarker || loginish); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The loginish escape hatch makes the impl marker check nearly unenforceable.
loginish passes when the status is 200 and the body matches /callbackUrl|sign.?in|login/i. Most authenticated app shells render a "Sign in" or "Sign out" control, and Auth.js pages and links embed callbackUrl. A route that renders the wrong register, or no register at all, therefore passes the check. The route gate then reports green while the cutover marker is absent.
Narrow the fallback to an actual auth redirect. Compare response.url against the sign-in path, or send a request with redirect: 'manual' and accept only a 3xx to the sign-in route. Better, run the doctor with a session cookie so the marker assertion is the only accepted pass.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/doctor.mjs` around lines 71 - 74, Replace the broad body-text-based
loginish fallback near the route check with validation of an actual
authentication redirect: compare response.url to the expected sign-in path, or
use a manual redirect request and accept only a 3xx redirect targeting that
path. Ensure routes without the cutover marker cannot pass merely because their
HTML contains terms such as “login” or “callbackUrl”.
Summary
.commonplace-canonicalinto the cutover register manifest and gate it in console CI.scripts/doctor.sh//doctor//api/doctor) with staging red proofs and authenticated upstream smokes on main./chatthrough middleware to the Railway workspace openwork door (OW4); collapse workspace chat onto/chat; stampopenwork.chat/model-canvas.owox.VOLUME, Node 22 for pnpm stages, honorPORT=8787for healthchecks.Test plan
node scripts/check-register-manifest.mjsgreennode scripts/doctor-staging-reds.mjsgreenhttps://commonplace-workspace-production.up.railway.app/healthreturns 200commonplace-console;bash scripts/doctor.shagainsthttps://v2.theoremharness.com/chatshowsdata-register-impl="openwork.chat"Summary by CodeRabbit
/chatentry point with workspace-aware routing and clearer fallback messaging.