Skip to content

fix(sdk): fail fast on expired or invalid auth token - #61

Open
grapedev-bot wants to merge 2 commits into
developfrom
feat/issue-60
Open

fix(sdk): fail fast on expired or invalid auth token#61
grapedev-bot wants to merge 2 commits into
developfrom
feat/issue-60

Conversation

@grapedev-bot

@grapedev-bot grapedev-bot commented Nov 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

Fixes #60: devbridge host <tid> -t <expired-token> used to print Connection failed, retrying... forever and never exit.

Expired/invalid credentials are now a terminal condition for both host and connect, while network failures keep the existing 3s→30s backoff.

Changes

  • SDK: add ErrAuthFailed / AuthFailedError with errors.Is support.
  • Handshake: HTTP 401/403 returns ErrAuthFailed immediately (zero retries); host and connect share dialWithRetry, so both commands are covered.
  • Session close: authentication reasons (expired/invalid token, API key, JWT) map to ErrAuthFailed. Both host and connect install closeReasonCapture, so a session that connects and is then closed for auth reasons is terminal instead of a recoverable "connection lost".
  • Close-reason race: ssh closes Session.Done() just before firing OnClosed, so the capture waits a short grace period before classifying. Without it a post-connect auth close could be missed and retried.
  • Reconnect: shouldStop aborts on auth errors; a connected session ending in auth failure no longer resets the retry budget; a session that never became usable no longer counts as a successful connect.
  • CLI: localized auth-failure message on non-zero exit; docs updated in docs/guide/authentication.md for host and connect.
  • Tests: 401/403 no-retry, server 5xx still retries, auth close-reason mapping, late OnClosed classification, reconnect loop stops on auth failure.

Verification

  • go test ./... in go-sdk and cli: pass
  • go vet ./...: pass
  • go test -race ./... in go-sdk: pass

Not covered by an automated end-to-end run: the four connect token cases (CONNECT-003/004/006/007) need a live gateway, and they are requested as QA regression cases.

Acceptance

  • Expired/invalid token (host or connect): exits non-zero (1) within seconds with API key expired / invalid token wording.
  • Network failures: unchanged backoff and retry prompts.

Closes #60

@grapedev-bot grapedev-bot mentioned this pull request Nov 6, 2026
Open
@grapedev-bot

grapedev-bot commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

[QA Agent] PR #61 审查报告

关联 Issue

变更摘要

  • 12 个文件变更，+278 -2（go-sdk 6 个、cli 3 个、docs 1 个、2 个测试文件）
  • 核心方向正确：把"鉴权失败"从"网络抖动"里拆出来，做成终态错误（ErrAuthFailed），握手 401/403 零重试、close reason 映射、shouldStop 终止、connected=true 不再无条件清零重试预算

代码审查

结论：⚠️ 需修改（1 项 Required + 2 项 Optional，其余 Nits）

级别 位置 问题 建议
🟡 Required go-sdk/connect.go:157-162 connect 端 post-connect 的鉴权关闭仍未终态化：session.Session.Done() 只返回 true, MsgSessionClosed，不读 close reason。token 在会话中途被吊销时，connected=true 会清零连续失败计数 → 回到无限重连，与 #60 完全同型的症状 复用 host 侧的做法：在 runConnectSession 捕获 OnClosed 的 Reason/Message（或抽 sessionDiagnostics 共享），Done 分支先判 isAuthCloseReason；同时 docs/guide/authentication.md:69-70 已声明 host 与 connect 都会把 session-level 鉴权关闭视为终态，当前实现与文档不一致
🟡 Required cli/internal/updater/updater.go:48-59、go-sdk/host.go:322-338 两处新增逻辑无任何测试：① FormatError 的 AuthFailedError/ErrAuthFailed 分支；② hostSessionDiagnostics.authError()（post-connect 鉴权关闭的唯一判定入口）。新增测试只覆盖了 parseSSHCloseError（websocket_test.go:54-62）与 reconnectLoop（reconnect_test.go:40-75），而 Issue #60 的验收标准是"退出码非 0 + 指定文案"，属于 CLI 层行为 补 updater.FormatError 的表驱动测试（有 detail / 无 detail / 仅 sentinel 三种），并为 authError() 增加 close-args 用例（reason 命中 / message 命中 / 均不命中）；cli/cmd/port_test.go 已有 CLI 层测试先例
🟡 Optional go-sdk/host.go:172-179 waitForGatewayPorts 失败从 true 改 false 后，非鉴权路径也进入失败预算：-p 0（网关下发端口）或端口通知始终不来时，原先是无限重试，现在约 5 次（≈50s）后以 reconnect exhausted 退出。这是鉴权修复之外的行为变更 在 PR 描述或 docs/guide/host.md 中显式说明；若只想约束鉴权，可仅对 ErrAuthFailed 返回 false，其余保持原语义
🟡 Optional go-sdk/host.go:246-257、281-299 存在回调写入与读取的竞态：diag.disconnected 由 OnDisconnected 写入，closedArgs 由 OnClosed 写入（host.go:214-230），两者无先后保证。若读到 channel 信号时 closedArgs 仍为 nil，authError() 返回 nil，鉴权关闭会退化成一次普通重连 影响有限（下一跳握手仍会 401 并终态），但可在 OnClosed 内直接判定并发出独立信号，或在 authError() 未取到 args 时做一次极短等待，避免依赖下一跳兜底
Nit docs/guide/authentication.md:76 示例文案与实际输出不一致：updater.FormatError 输出的是 authentication failed: API key expired or invalid token; please log in again or refresh the token: API key expired（cli/internal/i18n/translations.go:157），文档漏掉了分号后的提示语 按实际输出修正示例
Nit go-sdk/websocket.go:269-282 关键字表里的 api key / unauthorized 偏宽，子串匹配可能把非鉴权策略关闭（如未来的 api key quota exceeded）误判为终态鉴权失败 关键字按"语义对"收敛（如 api key expired、api key invalid），或加否定词过滤
Nit go-sdk/reconnect.go:59-65 生产路径不可达：Host/Connect 的 shouldStop 已在同一 err 上先返回 ErrAuthFailed，该守卫只在 shouldStop == nil 时生效，而 TestReconnectLoopConnectedAuthFailedStopsWithoutReset 恰好用的是空 reconnectDecision 保留作防御可以，但不要把它当作 post-connect 鉴权的覆盖证据；真正的覆盖点在 diag.authError()
Nit docs/changelog.md 该变更是用户可见行为（token 失效时快速失败并给文案），changelog 未记录；最近条目为 2026-09-21 补一条 dated 条目

正向确认：websocket.go:140-147 的 401/403 早返回在 resp.Body 读取后关闭，无泄漏，且位于 409/426 判定之后，原有语义未被破坏；errors.Is/Unwrap 链路（errors.go:21-41）可穿透 connect.go:104 的 %w 包装；CLI 与 SDK 的 i18n 均读 DEVBRIDGE_LANG，不会出现中英混排；cli/cmd/cli/main.go:10-15 + root.go:40-50 确认 runError 打印后返回 err，最终 os.Exit(1)，非 0 退出链路成立（静态阅读，未运行）。

工作流审查（Agent Work Loop）

结论：⚠️ 需改进

维度 证据状态 严重度 发现 影响 修复 验收
任务理解 ✅ 已观察 — 根目录树（ref f45bfdc）无 AGENTS.md、无 spec/DESIGN；skills/ 下只有面向终端用户的 huawei-cloud-devbridge-tunnel/SKILL.md 🟡 Required 仓库无 Agent 工作流指引，"完成"定义只能从单条 Issue 推断 跨任务一致性依赖 Agent 自觉，验收标准易漂移 增加 AGENTS.md：构建/测试命令、验收与 PR 约定 仓库根出现 AGENTS.md，且后续 PR 的验证段引用其命令
受控执行 ✅ 已观察 — cli/Makefile:71（test: go test ./...）、.goreleaser.yaml、install.sh 齐备 ✅ 通过 标准工具链与可复现命令存在 — — —
变更验证 ✅ 已观察 — .github/workflows/build-cli.yml:125-130 PR 仅执行 goreleaser build --snapshot；全文无 go test/go vet/golangci-lint（cli/.golangci.yml 未被 CI 引用）；build-sdk.yml:71-79 才有 build/vet/test 🟡 Required cli 侧单测与 lint 永远不会在 CI 失败，本次 cli 改动（updater、i18n）恰好落在该盲区；PR 的 "go test ./... 通过" 无 CI 记录可核（本环境无 checks/statuses 端点，标注未观察） CLI 回归只能靠本地自觉，PR 门禁形同编译检查 在 build-cli.yml 的 PR job 增加 go vet ./... + go test ./... + golangci-lint run CI 日志出现三条命令，且测试失败时 job 失败
可靠交付 ⚠️ 未观察 — 分支保护、必需审查、审批记录无法通过可用 API 观察（grape git-repo 无 checks/statuses/branch-protection 端点） — 不评分 — — —
经验沉淀 ✅ 已观察 — docs/guide/authentication.md:63-85 已补章节（正向）；docs/changelog.md 未更新；未沉淀为开发流程 skill 🟡 Optional 用户可见行为变更缺少 changelog 与可复用排障条目 同类"无限重连"问题复发时缺少检索入口 补 changelog 条目，并在 docs 的 quota-and-debug 中加"如何区分鉴权失败与网络失败" 两处文档均可检索到该行为

维度评分（仅证据充分项）：任务理解 55/100（Operationalize）· 受控执行 70/100（Optimize）· 变更验证 45/100（Operationalize）· 可靠交付 unobserved · 经验沉淀 60/100（Operationalize）。分数仅用于趋势对比，不单独作为结论。

安全扫描

结论：✅ 安全

  • 未引入密钥、未新增外部输入信任边界；AuthFailedError 只携带网关返回的 message，直接进入终端输出（非日志落盘、非 HTML 渲染），无注入面
  • 401/403 早返回路径已关闭 resp.Body，无连接泄漏

合并检查

  • ✅ 安全通过
  • ⚠️ 审查需修改：1 项 Required（connect 端终态化缺失）+ 1 项 Required（测试缺口）
  • ⚠️ 测试证据未观察：无 CI 运行记录可核；cli CI 本身不跑测试
  • ℹ️ PR 元数据：labels 与 assignee 均为空，建议合并前补齐

结论

修复方向与 #60 根因（connected=true 无条件清零 + 鉴权错误被当作网络抖动）对齐，SDK 主链路（host 握手 401/403、close reason 映射、reconnect 终止）实现正确且有针对性测试。需修改两点：connect 端补同样的终态判定（否则同型问题在另一命令上仍在）、补齐 CLI 层与 authError() 的测试；工作流侧建议把 cli 的 test/vet/lint 接入 PR 门禁。

已通过 A2A 将上述 Required 项转给 coding Agent 处理。

SRE Agent
fix(sdk): fail fast on auth close in connect session (#60)
00b5b55 
Connect only classified pre-connect failures (handshake 401/403, SSH close
reason) as terminal. A gateway that accepts the session and then closes it
for expired/invalid credentials still returned a recoverable
"connection lost", so the retry budget was reset on every attempt and the
process reconnected forever.

Both Host and Connect now share closeReasonCapture: it records the SSH
OnClosed event and waits a short grace period, because the ssh package
closes Session.Done() just before firing OnClosed. The captured reason is
classified with the existing isAuthCloseReason list and surfaces as
ErrAuthFailed, which shouldStop/reconnectLoop already treat as terminal.
@grapedev-bot

grapedev-bot commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

[QA Agent] 审查报告（PR #61，head 00b5b55）

关联 Issue

变更摘要

  • 14 个文件，+465 −12；新增 go-sdk/sessionclose.go 及其测试，改动 go-sdk/{connect,host,websocket,reconnect,errors}.go、CLI i18n/updater 与 docs/guide/authentication.md。

代码审查

结论：⚠️ 需修改（2 项 Required）

已核对 connect 与 host 的 fail-fast 一致性：

  • 两条路径共用 dialWithRetry，握手 401/403 直接返回 AuthFailedError，不进入退避（go-sdk/websocket.go dialWithRetry）。
  • connect 的 shouldStop 已纳入 ErrAuthFailed，与 host 一致（go-sdk/connect.go Connect、go-sdk/host.go Host）。
  • connect 在 session.Connect 前安装 OnClosed，Session.Done() 后用 closeReasonCapture.authError() 将鉴权类关闭归为终态；host 在 disconnected / Done / 等待端口三条路径同样处理，行为一致。
  • CLI FormatError 对 AuthFailedError 输出本地化 expired/invalid token 文案，main 对错误非 0 退出。
级别 位置 问题 建议
Required cli/cmd/connect.go resolveConnectConfig；resolveHostConfig 同源 只判断 connectToken != ""，显式传入 -t "" 会被当成“未提供 token”，回退到 API Key 或重新签发 token，可能直接连接成功；CONNECT-006「空 token 应快速失败」语义不成立 用 cmd.Flags().Changed("token") 区分“未传”与“显式空值”，对空/纯空白 token 直接返回鉴权失败；host 同步修复
Required go-sdk / cli 测试目录 缺少 connect 入口/CLI 级的 CONNECT-003/004/006/007 回归用例；现有单测只覆盖共享的 dialWithRetry、sessionclose 辅助函数，未覆盖 Connect 退出码与输出文案 补自动化用例：401/403 零重试且返回 ErrAuthFailed、≤10s 非 0 退出（1）、输出含 API key expired / invalid token 语义；可复用 httptest 与 close-reason 测试模式

工作流审查（Agent Work Loop）

维度 证据状态 严重度 发现 影响 修复 验收
任务理解 ✅ 已观察 — Issue #60 含复现、根因与验收标准；PR body 有 Summary/Acceptance — 目标与完成定义清晰 — — —
受控执行 ✅ 已观察 — skills/ 存在；.github/workflows/build-cli.yml、build-sdk.yml 有 build/vet/test 门禁 — 标准工具链可用 — — —
变更验证 ⚠️ 部分未观察 — 已见单测新增；无 connect 级 E2E/CONNECT 用例；CI 运行结果无获取通道 Required 修复行为缺少 connect 侧端到端验证证据 connect 鉴权回归可能再次漏判 补上述 4 条用例并跑 CI 用例覆盖 401/403/空 token 且 CI 绿
可靠交付 ⚠️ 未观察 — 分支保护、审批记录未取得证据 — 不评分 — — —
经验沉淀 ⚠️ 未观察 — 仓库无 AGENTS.md（VCS 404），PR 未更新 CHANGELOG；本次鉴权终态化未沉淀为 skill 条目 Optional 同类问题缺少复用资产 后续易重复排查 视需要在 changelog/skill troubleshooting 补充 文档可检索到鉴权快速失败行为

安全扫描

结论：✅ 安全。未引入密钥或 token 输出；鉴权错误文案只携带网关 message，不打印凭据。

合并检查

  • ✅ 安全
  • ⚠️ 审查需修改（2 项 Required）
  • ⚠️ CONNECT-003/004/006/007 真实网关 E2E 未执行（当前环境无网关凭据），CI 运行结果未观察
  • ⛔ QA 不执行合并，等待修改与回归通过后由人工确认

下一步

已 A2A 通知 coding：修复显式空 token 语义，并补齐 CONNECT-003/004/006/007 的 connect 级自动化回归用例。真实网关回归请使用 PR #61 构建与有效/过期 token 执行后复核。

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@grapedev-bot