Skip to content

docs: document the gateway key and external sign-in startup codes - #1585

Merged
freeqaz merged 2 commits into
mainfrom
fix-689-docs
Oct 7, 2026
Merged

freeqaz merged 2 commits into
mainfrom
fix-689-docs

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

Problem

  • GATEWAY_API_KEY_UNAVAILABLE had no reference entry. It was only named in passing next to the native admin codes.
  • With native admin's shared cookie domain set, GitHub, Google or OIDC sign-in stops the API with the generic STARTUP_FAILED code. Nothing said so.

Change

docs/reference/controller.md, under "Failures and diagnostics":

  • A GATEWAY_API_KEY_UNAVAILABLE entry: a blank or relative OCC_GATEWAY_API_KEY_PATH; a key file that is missing, not a regular file, over 4 KiB, or not printable ASCII without spaces; and native admin without the path.
  • A STARTUP_FAILED entry for external sign-in combined with native admin's shared cookie domain. Product behaviour is unchanged.

Verification

  • pnpm docs:check passes: 243 pages, 3800 links. controller.md stays below the hard word limit.
  • node --test tests/docs/*.test.mjs: 29/29 pass.
  • The claims were checked against server.mjs startupFailureCode, composition/workspace-files.ts, composition/production.ts and auth/index.ts. The parity test's "OIDC with shared-cookie native administration" case already asserts STARTUP_FAILED.

No new conflicts: #906, #1325, #1496, #519 and #479 have the same conflicted files against this branch as against main, and none touches controller.md.

Risks

Docs only.

GATEWAY_API_KEY_UNAVAILABLE had no reference entry. List what the API
refuses for OCC_GATEWAY_API_KEY_PATH and its key file, and that native
admin without the path reports the same code.

GitHub, Google or OIDC sign-in with native admin's shared cookie domain
set stops the API with the generic STARTUP_FAILED code. Say so, and that
the chart refuses the combination.
Name the trailing newline as the usual cause of an invalid key file, and
say the external sign-in refusal needs native admin enabled with the
cookie domain set and comes after the database connects.
@freeqaz
freeqaz requested a review from a team as a code owner October 7, 2026 04:06
@clawsweeper

clawsweeper Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Oct 7, 2026
@clawsweeper

clawsweeper Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed October 7, 2026, 12:09 AM ET / 04:09 UTC.

ClawSweeper review

What this changes

Adds controller troubleshooting entries for invalid gateway service-key files and external sign-in combined with native administration.

Merge readiness

⛔ Needs changes before merge - 2 items remain

The documentation fills a real gap on current main, but its production startup ordering is inaccurate. Keep the PR open with one focused correction.

Priority: P3
Reviewed head: 95bd915c18e47d2091afdc283f40620895ee7d42

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) A focused and useful documentation patch with one source-proven diagnostic inaccuracy.
Proof confidence 🌊 off-meta tidepool Not applicable: Real behavior proof is not required because this PR only changes files under docs/.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: Real behavior proof is not required because this PR only changes files under docs/.
Evidence reviewed 6 items Introduced documentation and verified merge scope: The pinned introduction adds eleven lines only to controller.md. The verified test merge has the exact fetched-main and PR-head parents; its delta against main contains the same documentation addition, without removing base-only changes.
Gateway-key claims match implementation: The file reader requires an absolute path, a regular file, at most 4096 bytes, and nonempty printable non-space ASCII. The startup wrapper converts file errors into a gateway-key error, and server.mjs maps that error to GATEWAY_API_KEY_UNAVAILABLE.
Production refusal precedes database connection: composeProduction rejects GitHub, Google, and OIDC whenever native administration is enabled at lines 101–108. The PostgreSQL pool is created afterward at line 118. This source is unchanged between fetched main and the reviewed head, disproving the newly documented production ordering.
Findings 1 actionable finding [P2] Correct the production sign-in refusal ordering
Security None None.

How this fits together

The OpenClaw Control Plane API validates operator configuration and composes authentication before serving requests. Its startup error codes help operators distinguish configuration failures from unavailable dependencies.

flowchart TB
  A[Operator configuration] --> B[API startup validation]
  B --> C[Gateway key validation]
  B --> D[Sign-in compatibility check]
  C --> E[Startup error code]
  D --> E
  E --> F[Controller troubleshooting reference]
Loading

Before merge

  • Correct the production sign-in refusal ordering (P2) - For production startup with any of these providers and native administration enabled, composeProduction throws at lines 101–108 before createPostgresPool at line 118, regardless of whether a shared cookie domain is set. The existing parity scenarios also explicitly describe refusal before database work. Saying this error occurs after connection incorrectly suggests that STARTUP_FAILED proves database connectivity. Remove the timing clause or distinguish production's early refusal from development's later authentication check.
  • Complete next step (P2) - Correct the claim that production external sign-in refusal occurs after database connection.

Findings

  • [P2] Correct the production sign-in refusal ordering — docs/reference/controller.md:208-209
Agent review details

Security

None.

Review metrics

None.

Technical review

Best possible solution:

Keep both diagnostic entries, with startup timing stated accurately for production and development.

Do we have a high-confidence way to reproduce the issue?

Yes, source inspection establishes the documentation mismatch: production rejects the provider/native-admin combination before creating its PostgreSQL pool. No runtime reproduction was executed.

Is this the best way to solve the issue?

Yes, extending the existing troubleshooting reference is the narrowest solution, once the unconditional post-database timing claim is corrected.

Full review comments:

  • [P2] Correct the production sign-in refusal ordering — docs/reference/controller.md:208-209
    For production startup with any of these providers and native administration enabled, composeProduction throws at lines 101–108 before createPostgresPool at line 118, regardless of whether a shared cookie domain is set. The existing parity scenarios also explicitly describe refusal before database work. Saying this error occurs after connection incorrectly suggests that STARTUP_FAILED proves database connectivity. Remove the timing clause or distinguish production's early refusal from development's later authentication check.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against c72e8b6e1336.

Labels

Label changes:

  • add P3: This is a bounded troubleshooting documentation improvement with no runtime change.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: Real behavior proof is not required because this PR only changes files under docs/.

Label justifications:

  • P3: This is a bounded troubleshooting documentation improvement with no runtime change.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: Real behavior proof is not required because this PR only changes files under docs/.

Evidence

Acceptance criteria:

  • [P1] pnpm docs:check.
  • [P1] pnpm docs:check-length.
  • [P1] git diff --check.

What I checked:

  • Introduced documentation and verified merge scope: The pinned introduction adds eleven lines only to controller.md. The verified test merge has the exact fetched-main and PR-head parents; its delta against main contains the same documentation addition, without removing base-only changes. (docs/reference/controller.md:200, fc0f892d49f1)
  • Gateway-key claims match implementation: The file reader requires an absolute path, a regular file, at most 4096 bytes, and nonempty printable non-space ASCII. The startup wrapper converts file errors into a gateway-key error, and server.mjs maps that error to GATEWAY_API_KEY_UNAVAILABLE. (apps/controller/src/composition/workspace-files.ts:57, 95bd915c18e4)
  • Production refusal precedes database connection: composeProduction rejects GitHub, Google, and OIDC whenever native administration is enabled at lines 101–108. The PostgreSQL pool is created afterward at line 118. This source is unchanged between fetched main and the reviewed head, disproving the newly documented production ordering. (apps/controller/src/composition/production.ts:101, 95bd915c18e4)
  • Existing parity coverage corroborates ordering and code: All three provider/native-admin scenarios explicitly describe refusal before database work. The shared assertion expects STARTUP_FAILED. These tests were inspected, not executed. (tests/integration/sign-in-chart-parity.test.mjs:608, 95bd915c18e4)
  • Current-main documentation still lacks the entries: Fetched main's controller reference has neither new troubleshooting entry, so the central documentation contribution remains useful. No merged replacement is established by the supplied related context. (docs/reference/controller.md:199, c72e8b6e1336)
  • Current-main routing history: Current-main history records freeqaz's recent controller diagnostic documentation work and kevinlin-openai's gateway workspace-file routing work. Deeper pickaxe/blame inspection encountered unavailable historical objects; these are routing candidates without verified introduction attribution. (docs/reference/controller.md, 06fcf6f5840d)

Likely related people:

  • freeqaz: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • kevinlin-openai: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Correct the production startup ordering in the external sign-in entry.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@freeqaz
freeqaz merged commit 0573af5 into main Oct 7, 2026
23 checks passed
@freeqaz
freeqaz deleted the fix-689-docs branch October 7, 2026 04:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant