セッションローテーション実装 (Invalid token対策)

[EdamAme-x]

Summary

• refreshAuthToken 関数を core/types.ts に追加 (/api/v2/auth/refresh エンドポイント)
• User クラスに #refreshToken を保持し、refresh() メソッドで自動リフレッシュ (失敗時は匿名トークン再取得にフォールバック)
• uploadFile / createThread で "Invalid token" / 401 エラー時に自動リトライする #withTokenRefresh ラッパーを追加
• WebSocket 再接続失敗時にもトークンリフレッシュを試行してから再接続

Breaking Change

• User コンストラクタの第3引数に refreshToken: string が追加 (手動でコンストラクタを呼んでいる場合のみ影響。User.create() 経由なら変更不要)

Test plan

• User.create() でセッション作成後、トークン期限切れ後に uploadFile が自動リフレッシュで成功するか確認
• WebSocket 切断→再接続時にトークンリフレッシュが正しく動作するか確認
• リフレッシュトークン自体が期限切れの場合、匿名トークン再取得にフォールバックするか確認

closes #15

🤖 Generated with Claude Code

[EdamAme-x]

@claude はよレビューしろ

[claude]

Claude finished @EdamAme-x's task in 59s —— View job

---

コードレビュー

• コンテキスト収集
• 変更ファイルの読み込み
• レビューフィードバック作成

---

全体的にロジックは正しく、意図が明確なPRです。ただし以下の点が気になります。

---

🔴 バグ: #withTokenRefresh でクロージャのトークンが古くなる

chat.ts:39-49 の #withTokenRefresh が最大の問題です。

async #withTokenRefresh<T>(fn: () => Promise<T>): Promise<T> {
  try {
    return await fn()
  } catch (err) {
    if (...) {
      await this.refresh()
      return await fn() // ← ここでトークンがリフレッシュ済みか？
    }
  }
}

fn のクロージャが this.accessToken をキャプチャしていない場合は問題ありませんが、呼び出し側を見ると:

// chat.ts:65-71
const res = await this.#withTokenRefresh(() =>
  uploadFile(this.deviceId, this.accessToken, { ... })  // ← this.accessToken を参照
)

ここで () => uploadFile(this.deviceId, this.accessToken, ...) というアロー関数は クロージャではなく毎回 this.accessToken を評価 します。JavaScriptのアロー関数はプロパティを遅延評価するので、refresh() 後に this.accessToken が更新されていれば正しく動きます。これは問題なしです。✅

ただし createThread も同様の構造 なので確認しておくとよいです。

---

🟡 並行リクエスト時のレース条件

複数の並行リクエストが同時に 401 エラーを受け取った場合、refresh() が複数回並行して呼ばれます。最初のリフレッシュ成功後に発行された refreshToken は無効化される可能性があるため、2回目以降のリフレッシュは匿名フォールバックになります。これはトークン無効化の設計次第ですが、リフレッシュ中のリクエストを dedup するとより堅牢です。

Fix this →

---

🟡 accessToken の readonly が外れた

chat.ts:14:

accessToken: string  // before: readonly accessToken: string

refresh() で書き換える必要があるので必然的な変更ですが、外部から自由に書き換え可能になります。#accessToken (private field) にして get accessToken() でアクセサを生やすと安全です。

Fix this →

---

🟡 WebSocket の onclose でトークンエラー以外も refresh() が走る

chat.ts:113-127:

this.#ws.onclose = async () => {
  try {
    this.#ws = await Thread.connectWS(this.#user)
    resolve()
  } catch {
    // トークン期限切れの場合、リフレッシュして再接続
    try {
      await this.#user.refresh()
      this.#ws = await Thread.connectWS(this.#user)
      resolve()
    } catch (err) {
      reject(err)
    }
  }
}

connectWS が失敗する原因はトークン期限切れだけではないはずです（ネットワークエラー、サーバーエラーなど）。現在の実装ではあらゆるエラーで refresh() が呼ばれます。これは安全な匿名フォールバックがあるので実害は少ないですが、不要なトークン再発行が増えます。

---

🟢 良い点

• #refreshToken を private field にして外部から隠蔽している
• フォールバックが fetchAnonymousToken に設計されているのは堅牢
• #withTokenRefresh の汎用ラッパー設計は再利用性が高い
• User.create() 経由の利用者には breaking change なし

---

まとめ

重要度	項目
🔴	なし（致命的バグなし）
🟡	並行リフレッシュのレース条件
🟡	accessToken の可視性
🟡	WebSocket onclose の無条件リフレッシュ

機能としては問題なく動作するはずで、マージ可能なレベルです。上記の🟡を対処すると品質が上がります。

[EdamAme-x]

@claude 再レビューしろ

[claude]

Claude finished @EdamAme-x's task in 52s —— View job

---

再レビュー

• コンテキスト収集
• 変更ファイルの読み込み
• レビューフィードバック作成

---

前回の指摘事項との照合結果です。

前回指摘の対応状況

指摘	対応
並行リフレッシュのレース条件	✅ 対応済み
accessToken の可視性	✅ 対応済み
WebSocket onclose の無条件リフレッシュ	✅ コメントで意図を明記

---

✅ 対応済み

refresh dedup (#refreshPromise)

chat.ts:31-38 の実装を確認:

async refresh(): Promise<void> {
  if (this.#refreshPromise) return this.#refreshPromise
  this.#refreshPromise = this.#doRefresh()
  try {
    await this.#refreshPromise
  } finally {
    this.#refreshPromise = null
  }
}

並行して refresh() が呼ばれても同じ Promise を返すようになっています。✅

#accessToken private field + getter

chat.ts:14-20:

#accessToken: string
get accessToken(): string {
  return this.#accessToken
}

外部からの書き換えを防ぎつつ読み取りはできる設計になっています。✅

---

🟡 残存: refresh() の dedup に軽微なバグ

chat.ts:32-37 の実装:

if (this.#refreshPromise) return this.#refreshPromise  // (A)
this.#refreshPromise = this.#doRefresh()               // (B)
try {
  await this.#refreshPromise                           // (C)
} finally {
  this.#refreshPromise = null                          // (D)
}

(A) で既存の Promise を返した呼び出し元は await して完了を待ちますが、(D) で null にリセットされるのは 最初の呼び出し元が finally を抜けた時点 です。その後に (A) でリターンした呼び出し元の await も既に解決済みの Promise を待っているだけなので、実害はありません。

ただし、#doRefresh() が例外を投げた場合、(A) で同じ Promise を受け取った全ての呼び出し元にも例外が伝播します。これは想定通りの動作と思われますが、意図通りであればコメントがあると読みやすいです。

---

🟡 残存: WebSocket onclose の無条件リフレッシュ

chat.ts:129-144 にコメントが追加されたのは良い対応です:

// NOTE: WS APIではHTTPステータスを取得できないため、
// トークン期限切れ以外のエラーでもリフレッシュが走る

意図が明確になりました。現状の設計として許容範囲です。

---

🟡 新規: onclose 内で resolve() が複数回呼ばれる可能性

chat.ts:129-150:

this.#ws.onclose = async () => {
  const oldWs = this.#ws
  try {
    this.#ws = await Thread.connectWS(this.#user)
  } catch {
    try {
      await this.#user.refresh()
      this.#ws = await Thread.connectWS(this.#user)
    } catch (err) {
      reject(err)
      return
    }
  }
  // 旧WSのハンドラをクリーンアップ
  oldWs.onmessage = null
  oldWs.onerror = null
  oldWs.onclose = null
  resolve()
}

this.#ws を更新してから oldWs.onclose = null まで非同期処理があります。新しい WS の onclose を設定する前に、while ループの次のイテレーションが始まって新しい onclose ハンドラが登録されれば問題ありませんが、タイミング次第では競合の余地があります。これは既存設計上の問題で、このPRの変更範囲を超えているため、別 issue での対応が適切です。

---

🟢 総合評価

前回の主要な指摘（dedup、accessToken隠蔽）はいずれも適切に対応されています。残存する🟡は実害の少ないエッジケースまたは既知の設計上の制約です。

マージ可能と判断します。

---

[EdamAme-x]

@akku1139 review please

[akku1139]

expires ヘッダーに基づいてリフレッシュ用のPromiseを用意しとくのが多分望ましい

[EdamAme-x]

後で見ます

[EdamAme-x]

@claude おい、レビューしろ

[EdamAme-x]

@akku1139 レビュー頼みます♡

[claude]

Claude finished @EdamAme-x's task in 1m 6s —— View job

---

レビュー (再レビュー #3)

• コンテキスト収集
• 変更ファイルの読み込み
• レビューフィードバック作成

---

akku1139 のレビュー指摘を中心に確認した結果です。

---

akku1139 指摘の対応状況

指摘	対応状況
expires ヘッダーに基づいた proactive refresh	✅ 対応済み
チャット継続できない場合はインスタンスを落とす	✅ 対応済み (reject で stream を終了)
スレッド作成毎にリフレッシュしていた問題	✅ 対応済み

---

✅ expires ヘッダーによる proactive リフレッシュ

chat.ts:35-45 に #scheduleRefresh が実装されています:

#scheduleRefresh(expiresAt: number): void {
  if (this.#refreshTimer !== null) {
    clearTimeout(this.#refreshTimer)
  }
  // 有効期限の5分前にリフレッシュ (すでに5分以内なら即時)
  const delay = Math.max(0, expiresAt - Date.now() - 5 * 60 * 1000)
  this.#refreshTimer = setTimeout(() => {
    this.#refreshTimer = null
    this.refresh().catch(() => {})
  }, delay)
}

fetchAnonymousToken と refreshAuthToken の両方で expires ヘッダーを取得し (core/types.ts:142-144, core/types.ts:179-181)、expiresAt として User コンストラクタに渡しています。良い実装です。✅

---

🟡 #refreshTimer のメモリリーク

User インスタンスが使われなくなった際、#refreshTimer がクリアされません。Symbol.dispose や close() に対応するクリーンアップがありません。

長期使用や多数インスタンス生成の場合、タイマーが残り続けます。#doRefresh() が失敗する場合も次のスケジュールがされないため、問題は最小限ですが、User を破棄する際の dispose メソッドがあると安全です。

Fix this →

---

🟡 #withTokenRefresh での 401 マッチングが脆弱

chat.ts:67:

if (err instanceof Error && (err.message.includes('Invalid token') || err.message.includes('HTTP Error 401'))) {

HTTP Error 401 という文字列は createChatThread (core/types.ts:258) では HTTP Error ${response.status}: ${errorText} という形式のため、'HTTP Error 401' にはマッチします。

ただし uploadFile も同じ形式 (core/types.ts:317) なので一貫していますが、エラーメッセージ文字列でのマッチングよりも専用エラークラスを定義したほうが堅牢です。現状は実害なし。

---

🟡 #doRefresh の失敗時フォールバックが削除された

前バージョンでは refreshAuthToken 失敗時に fetchAnonymousToken にフォールバックする設計でしたが、現在の #doRefresh (chat.ts:55-62) はリフレッシュ失敗をそのまま throw します。

async #doRefresh(): Promise<void> {
  const tokens = await refreshAuthToken(this.deviceId, this.#refreshToken)
  // ...
}

refreshToken が期限切れの場合、リフレッシュが永遠に失敗し続け、匿名フォールバックがありません。PR の説明文には「リフレッシュトークン自体が期限切れの場合、匿名トークン再取得にフォールバックするか確認」とありますが、現在の実装はフォールバックしません。PR 説明との齟齬があります。

Fix this →

---

🟢 良い点

• expires ヘッダーによる proactive リフレッシュで、リアクティブなトークン切れを最小化
• #scheduleRefresh のタイマー更新で二重スケジュールを防止
• #refreshPromise dedup は変わらず有効
• #accessToken の private field + getter も維持

---

まとめ

重要度	項目
🔴	なし
🟡	#refreshTimer のクリーンアップ未実装
🟡	#doRefresh の匿名フォールバック削除 (PR説明と齟齬)
🟡	エラーマッチングが文字列依存

akku1139 の指摘は全て対応済みです。#doRefresh のフォールバック削除については、意図的なものであれば PR 説明の「フォールバックするか確認」の記載を削除するか、コメントで意図を明記すると良いです。

[akku1139]

動いてそう

[akku1139]

壊れては無かったけど何故かセッション切れるので後でrevertする

[akku1139]

reverted