Skip to content
This repository was archived by the owner on Jul 13, 2026. It is now read-only.

fix: use apiPublicGet for /v1/spot/status endpoint#333

Merged
tjackiet merged 2 commits into
mainfrom
claude/bitbank-status-404-fix-35djzo
Jul 6, 2026
Merged

fix: use apiPublicGet for /v1/spot/status endpoint#333
tjackiet merged 2 commits into
mainfrom
claude/bitbank-status-404-fix-35djzo

Conversation

@tjackiet

@tjackiet tjackiet commented Jul 6, 2026

Copy link
Copy Markdown
Owner

概要

status.ts が誤って publicGet/v1/spot/status を呼び出していた問題を修正しました。/v1/ 系エンドポイントは api.bitbank.ccapiPublicGet を使う必要があります。

変更内容

  • cli/commands/public/status.ts: publicGetapiPublicGet に修正
  • cli/tests/public/status.test.ts: 正しいホスト(api.bitbank.cc)を呼び出していることを検証するテストを追加
  • cli/tests/chaos/conventions/x19-host-path-mapping.test.ts: 新規追加。publicGet / apiPublicGet の呼び出しと path の対応を静的に検査する chaos テスト

背景

  • publicGetpublic.bitbank.cc(market data 系: /ticker, /tickers 等)
  • apiPublicGetapi.bitbank.cc/v1/spot/* 系)

呼び出し側の path 先頭リテラルで host 選択を静的に検査することで、コンパイルは通るが実行時に HTTP 404 になる問題を防ぎます。

チェックリスト

  • 目的 / 変更概要を記載した
  • テストを追加・更新した
  • npx vitest run が green
  • npx tsc --noEmit が green
  • npx biome check cli/ が green
  • chaos 規約(cli/__tests__/chaos/conventions/)を満たす
  • シークレットを含めていない

https://claude.ai/code/session_017QHckt8uYZCWmW7pX1gcpv

/v1/spot/status は api.bitbank.cc 配下のエンドポイントだが、
public.bitbank.cc に prefix する publicGet を使っていたため
bitbank status が常に HTTP 404 になっていた。
pairs.ts と同様に apiPublicGet を使うよう修正。

再発防止:
- status.test.ts にリクエスト URL の検証を追加(pairs.test.ts と同型)
- chaos x19 を新設: publicGet に /v1/ path を渡していないか、
  apiPublicGet が /v1/ path のみかを呼び出し側リテラルで静的検査

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017QHckt8uYZCWmW7pX1gcpv
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary

/v1/spot/status が誤って publicGet(public host)経由で叩かれていた点を修正し、apiPublicGet(api host)を使うように変更しました。加えて、実URLのホストを確認するテストと、publicGet / apiPublicGet/v1/ パス・ホスト対応を静的に検査するchaosテストを追加して、HTTP 404 の再発を防止します。

Category

  • 📚 Docs / tests
  • 🔧 Infra (router / output / cache / http-core / validators)

Risk Level

  • 🟢 Low

Impact

  • cli/commands/public/status.ts
  • cli/__tests__/public/status.test.ts
  • cli/__tests__/chaos/conventions/x19-host-path-mapping.test.ts

Walkthrough

status コマンドの呼び出し先が publicGet から apiPublicGet に変更され、https://api.bitbank.cc/v1/spot/status を検証するテストが追加された。あわせて、cli/commands 配下の publicGet/apiPublicGet/v1/ の対応を静的に検査するテストが追加された。

Changes

status のホスト切り替え

Layer / File(s) Summary
status.ts の apiPublicGet への切り替え
cli/commands/public/status.ts, cli/__tests__/public/status.test.ts
status の import と取得先を publicGet から apiPublicGet に変更し、要求 URL が https://api.bitbank.cc/v1/spot/status になることをテストで確認。
host/path 対応の規約チェックテスト
cli/__tests__/chaos/conventions/x19-host-path-mapping.test.ts
cli/commands 配下を走査して publicGet/apiPublicGet 呼び出しのパス対応を検査し、違反ゼロと対象呼び出しの存在を確認。

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

ぴょんと跳ねて host を直す
publicGet さん さようなら
apiPublicGet で v1へ
テストうさぎも見張り番
規約チェックで安心跳躍 🐇✨

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed 変更内容である/v1/spot/statusapiPublicGet化を簡潔に示しており、タイトルとして適切です。
Description check ✅ Passed 概要・変更内容・背景・チェックリストが揃っており、テンプレート要件を概ね満たしています。
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/bitbank-status-404-fix-35djzo

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 `@cli/__tests__/chaos/conventions/x19-host-path-mapping.test.ts`:
- Line 12: CALL_RE currently only captures literal path text, so it misses calls
where apiPublicGet/publicGet uses a template literal with a dynamic prefix.
Update the extraction logic in x19-host-path-mapping.test to recognize template
literals whose leading segment is a variable interpolation, and flag those cases
separately instead of treating them as a normal static path. Keep the existing
static-string handling for current files like pairs.ts and status.ts, but ensure
the test still detects host/path mix-ups even when the path starts with an
expression.
- Around line 46-52: The self-check in x19-host-path-mapping.test.ts is too
coarse because it only asserts a combined count for publicGet and apiPublicGet,
so one function can disappear without failing the test. Update the test around
the CALL_RE scan in the x19-host-path-mapping suite to count each call
separately and assert both publicGet and apiPublicGet are present independently.
Use the existing identifiers publicGet, apiPublicGet, CALL_RE, and tsFilesUnder
so the guard still detects when either branch of the host-path mapping logic is
removed.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 40251adb-677d-46a6-812b-eb155764a9e8

📥 Commits

Reviewing files that changed from the base of the PR and between b6e8a6e and 9ed4e75.

📒 Files selected for processing (3)
  • cli/__tests__/chaos/conventions/x19-host-path-mapping.test.ts
  • cli/__tests__/public/status.test.ts
  • cli/commands/public/status.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • bitbankinc/bitbank-api-docs (manual)

Comment thread cli/__tests__/chaos/conventions/x19-host-path-mapping.test.ts
Comment thread cli/__tests__/chaos/conventions/x19-host-path-mapping.test.ts Outdated
CodeRabbit レビュー (#333) の 2 指摘に対応:
- 自壊検知を publicGet / apiPublicGet の個別カウントに変更。
  片方の呼び出しが全滅しても検知できるようにする
- path 先頭が変数展開のテンプレートリテラルは host 選択を
  静的検査できないため violation としてフラグする

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017QHckt8uYZCWmW7pX1gcpv

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
cli/__tests__/chaos/conventions/x19-host-path-mapping.test.ts (1)

27-39: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

動的テンプレートリテラルの apiPublicGet 呼び出しで偽陽性の違反が重複発生します。

path"${" から始まる場合(Line 35-39 の新チェック対象)、Line 30 の !path.startsWith("/v1/") も真になってしまうため、apiPublicGet 呼び出しに対して「market data 系は publicGet を使う」という誤った違反(実際は静的に判定不能なだけ)が Line 35-39 の「静的検査不能」違反と同時に重複して出力されます。動的パスを持つ apiPublicGet 呼び出しが将来追加されると、このテストは常に失敗し続け、原因追跡が混乱します。

path.startsWith("${") のケースは、既存の /v1/ 判定より先に判定して以降の判定をスキップするべきです。

💡 対応例
     for (const m of src.matchAll(CALL_RE)) {
       const [, fn, path] = m;
       const line = src.slice(0, m.index).split("\n").length;
+      if (path.startsWith("${")) {
+        violations.push(
+          `${file}:${line}: ${fn}(\`${path}...\`) — path 先頭が変数展開だと host 選択を静的検査できない。先頭は文字列リテラルにする`,
+        );
+        continue;
+      }
       if (fn === "publicGet" && path.startsWith("/v1/")) {
         violations.push(`${file}:${line}: publicGet("${path}...") — /v1/ 系は apiPublicGet を使う`);
       }
       if (fn === "apiPublicGet" && !path.startsWith("/v1/")) {
         violations.push(
           `${file}:${line}: apiPublicGet("${path}...") — market data 系は publicGet を使う`,
         );
       }
-      if (path.startsWith("${")) {
-        violations.push(
-          `${file}:${line}: ${fn}(\`${path}...\`) — path 先頭が変数展開だと host 選択を静的検査できない。先頭は文字列リテラルにする`,
-        );
-      }
     }
🤖 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 `@cli/__tests__/chaos/conventions/x19-host-path-mapping.test.ts` around lines
27 - 39, The host-path mapping checks are producing duplicate violations for
dynamic template-literal paths in apiPublicGet. Update the convention logic in
x19-host-path-mapping.test.ts so the path.startsWith("${") case is handled first
and short-circuits further checks, preventing the later !path.startsWith("/v1/")
branch from also reporting a false “use publicGet” violation for the same call.
🤖 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.

Outside diff comments:
In `@cli/__tests__/chaos/conventions/x19-host-path-mapping.test.ts`:
- Around line 27-39: The host-path mapping checks are producing duplicate
violations for dynamic template-literal paths in apiPublicGet. Update the
convention logic in x19-host-path-mapping.test.ts so the path.startsWith("${")
case is handled first and short-circuits further checks, preventing the later
!path.startsWith("/v1/") branch from also reporting a false “use publicGet”
violation for the same call.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c6ee5fe8-3e07-4f8f-8524-00810d412134

📥 Commits

Reviewing files that changed from the base of the PR and between 9ed4e75 and fc02e26.

📒 Files selected for processing (1)
  • cli/__tests__/chaos/conventions/x19-host-path-mapping.test.ts
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • bitbankinc/bitbank-api-docs (manual)

@tjackiet tjackiet merged commit c01c35d into main Jul 6, 2026
4 checks passed
@tjackiet tjackiet deleted the claude/bitbank-status-404-fix-35djzo branch July 6, 2026 01:10
@tjackiet tjackiet mentioned this pull request Jul 6, 2026
8 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants