Skip to content

fix(terminal-core): tighten docs link URL detection#96439

Merged
vincentkoc merged 2 commits into
openclaw:mainfrom
lin-hongkuan:codex/terminal-docs-link-http-url-detection
Jun 29, 2026
Merged

fix(terminal-core): tighten docs link URL detection#96439
vincentkoc merged 2 commits into
openclaw:mainfrom
lin-hongkuan:codex/terminal-docs-link-http-url-detection

Conversation

@lin-hongkuan

@lin-hongkuan lin-hongkuan commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

formatDocsLink() used trimmed.startsWith("http") to decide whether a docs path was already an absolute URL. That creates two bad edge cases:

  • relative docs paths such as http-status are treated as complete URLs and are not rooted at https://docs.openclaw.ai
  • uppercase absolute URLs such as HTTPS://example.com/page are treated as relative docs paths

Why This Change Was Made

The helper now uses a case-insensitive https?:// check. That keeps actual HTTP(S) URLs untouched while routing merely http-prefixed relative docs paths through the docs root.

The regression tests cover both affected directions.

User Impact

Terminal docs links generated from relative paths that start with http point to the OpenClaw docs site correctly, and already-absolute HTTP(S) URLs remain stable regardless of scheme casing.

Evidence

Direct behavior probe:

$ node --import tsx -e "import('./packages/terminal-core/src/links.ts').then(({ formatDocsLink }) => console.log(JSON.stringify(['http-status', 'HTTPS://example.com/page', 'https://example.com/page'].map((path) => ({ path, out: formatDocsLink(path, 'label') })))))"
[{"path":"http-status","out":"https://docs.openclaw.ai/http-status"},{"path":"HTTPS://example.com/page","out":"HTTPS://example.com/page"},{"path":"https://example.com/page","out":"https://example.com/page"}]

Targeted test:

$ node scripts/run-vitest.mjs run packages/terminal-core/src/links.test.ts

 RUN  v4.1.8 C:/Users/lin20/Documents/New project 3/openclaw


 Test Files  1 passed (1)
      Tests  7 passed (7)
   Start at  21:18:52
   Duration  316ms (transform 32ms, setup 0ms, import 52ms, tests 4ms, environment 0ms)

[test] starting test/vitest/vitest.unit-fast.config.ts
[test] passed 1 Vitest shard in 6.88s

Formatting:

$ node_modules\.bin\oxfmt.cmd --check packages/terminal-core/src/links.ts packages/terminal-core/src/links.test.ts
Checking formatting...

All matched files use the correct format.
Finished in 1342ms on 2 files using 32 threads.

Whitespace:

$ git diff --check

AI-assisted

Prepared with Codex. I reviewed the change, understand the touched code path, and kept the PR focused on the bug described above.

@clawsweeper

clawsweeper Bot commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 5:18 PM ET / 21:18 UTC.

Summary
The PR changes formatDocsLink() in terminal-core to recognize only case-insensitive HTTP(S) absolute URLs and adds regression tests for uppercase schemes and http-prefixed relative docs paths.

PR surface: Source +2, Tests +10. Total +12 across 2 files.

Reproducibility: yes. Current formatDocsLink() treats any trimmed path starting with lowercase http as absolute and misses uppercase absolute HTTP(S) schemes, so both reported cases are source-reproducible without executing code.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Next step before merge

  • [P2] No repair lane is needed because the PR is focused, proof-positive, and this review found no actionable patch defect.

Security
Cleared: The diff only changes URL classification in a local terminal helper and colocated tests, with no dependency, workflow, secret, package, or code-execution surface changes.

Review details

Best possible solution:

Land the shared terminal-core helper fix after normal maintainer validation, keeping docs URL normalization centralized with its regression tests.

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

Yes. Current formatDocsLink() treats any trimmed path starting with lowercase http as absolute and misses uppercase absolute HTTP(S) schemes, so both reported cases are source-reproducible without executing code.

Is this the best way to solve the issue?

Yes. The shared terminal-core helper is the narrowest maintainable layer because SDK, CLI, and setup callers already route docs-link formatting through it, while caller-side fixes would duplicate URL policy.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 13ecca5408cb.

Label changes

Label justifications:

  • P2: This is a focused user-visible docs-link correctness bug with limited blast radius.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes after-fix terminal output from a direct helper probe plus targeted test output for the changed helper.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output from a direct helper probe plus targeted test output for the changed helper.
Evidence reviewed

PR surface:

Source +2, Tests +10. Total +12 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 3 1 +2
Tests 1 10 0 +10
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 13 1 +12

What I checked:

Likely related people:

  • steipete: Authored the original terminal docs-link helper with the broad startsWith("http") behavior and later added/cleaned the plugin SDK setup entrypoints that expose the helper. (role: introduced behavior and recent area contributor; confidence: high; commits: b50ea3ec59ad, a71c61122d41, 4b125762f6bb; files: src/terminal/links.ts, src/plugin-sdk/setup.ts, src/plugin-sdk/setup-tools.ts)
  • hclsys: Authored the merged undefined-path fix that widened and preserved the current helper shape around the same URL-classification branch. (role: recent behavior carrier; confidence: high; commits: be7f4a234258; files: src/terminal/links.ts)
  • gumadeiras: Recent channel setup status work changed the setup flow that consumes formatted docs links, making this a useful routing candidate for setup-surface review. (role: adjacent setup-flow contributor; confidence: medium; commits: 9bcf8f824388; files: src/flows/channel-setup.status.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works
  • 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.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. labels Jun 24, 2026
@vincentkoc vincentkoc self-assigned this Jun 29, 2026
@vincentkoc
vincentkoc merged commit 6d658c7 into openclaw:main Jun 29, 2026
77 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@vincentkoc vincentkoc

Labels

P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lin-hongkuan @vincentkoc