Skip to content

Add approval read feature#52

Open
forwaryan wants to merge 3 commits intoriba2534:mainfrom
forwaryan:feature/add-approve-read
Open

Add approval read feature#52
forwaryan wants to merge 3 commits intoriba2534:mainfrom
forwaryan:feature/add-approve-read

Conversation

@forwaryan
Copy link
Copy Markdown

Summary

  • 新增 approval get,用于查询审批定义详情
  • 新增 approval task query,用于查询当前登录用户相关的审批任务
  • 支持基于当前 OAuth 登录态自动识别用户,并将当前用户资料缓存到 ~/.feishu-cli/user_profile.json
  • 新增 --output raw-json,支持直接输出飞书 API 原始响应
  • 同步补充审批相关文档、命令帮助和权限说明

Changed files

  • cmd/approval.go / cmd/approval_get.go / cmd/approval_task.go / cmd/approval_task_query.go
    • 新增审批命令组、审批定义查询和审批任务查询命令
  • cmd/utils.go
    • 补充当前用户解析逻辑,供审批任务查询复用
  • internal/client/approval.go / internal/client/user.go
    • 新增审批相关 API 封装和当前登录用户信息获取能力
  • internal/auth/user_cache.go / internal/auth/token.go
    • 新增当前用户缓存,并在 token 变化或登出时清理缓存
  • README.md / cmd/root.go / CLAUDE.md / AGENTS.md / skills/feishu-cli-toolkit/SKILL.md
    • 同步审批命令、输出模式和审批权限说明
  • cmd/approval_get_test.go / cmd/approval_task_query_test.go / internal/auth/user_cache_test.go / internal/client/approval_test.go
    • 补充审批命令、客户端逻辑和用户缓存测试

Test plan

  • go test ./...
  • 验证 approval task query --help 已显示 json / raw-json
  • 手动验证 approval task query --output json--output raw-json 输出符合预期
  • 验证 README 中已补充审批相关权限 approval:approval:readonlyapproval:task

Copy link
Copy Markdown
Owner

@riba2534 riba2534 left a comment

Choose a reason for hiding this comment

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

感谢提交!审批功能是飞书核心模块,这个 PR 整体方向正确,代码质量不错。以下是审查发现的问题,请修复后可以合并。


🔴 Critical — 必须修复

internal/auth/token.go:81SaveTokenDeleteCurrentUserCache 失败会阻断 token 保存流程

SaveToken 在成功写入 token 文件后调用 DeleteCurrentUserCache(),如果缓存文件删除失败(例如文件权限问题),整个 SaveToken 返回 error。但此时 token 已经写入磁盘了。

这会导致所有触发 token 自动刷新的命令(搜索、任何用 user token 的命令)误判为刷新失败,是一个全局回归风险

建议:缓存清理失败不应阻断核心 token 持久化,改为 best-effort(忽略错误或仅 debug 日志):

// Token 更新后清理旧的当前用户缓存(best-effort)
_ = DeleteCurrentUserCache()

DeleteToken 中同理。


🟡 Warning

  1. cmd/approval_task_query.go:47--user-id 检查是死代码

    init() 中未注册 --user-id flag,cobra 对未注册 flag 调用 GetString 始终返回空字符串,这个分支永远不会进入。要么注册为 deprecated flag,要么直接删除这段代码。


💡 Suggestion

  1. cmd/approval_task_query.go:166--topic 建议标记为 Required

    当前 --topic 为空时在 normalizeApprovalTaskTopic 中返回错误,但报错信息不如 cobra 原生的 required flag 直观。建议在 init() 中添加 mustMarkFlagRequired(approvalTaskQueryCmd, "topic"),与项目其他命令风格一致。

@forwaryan forwaryan force-pushed the feature/add-approve-read branch from 3a63a7d to 889fa5e Compare March 27, 2026 07:11
@forwaryan
Copy link
Copy Markdown
Author

感谢 review,三点都已按建议修复并推送更新。

  1. SaveToken / DeleteToken 中的当前用户缓存清理已改为 best-effort,不再阻断 token 主流程,并补了对应回归测试。
  2. approval task query 中未生效的 --user-id 检查已删除。
  3. approval task query--topic 已标记为 required,并补了相应测试。

已本地验证相关测试通过,麻烦再帮忙看一轮。

@riba2534
Copy link
Copy Markdown
Owner

感谢提交!审批模块的实现质量不错,审查结果整体良好。

合并前需要一个小修复:

需修复

  • 删除死函数 approvalTaskToInfointernal/client/approval.go:327)及其测试 TestApprovalTaskToInfointernal/client/approval_test.go)。该函数在生产代码中无调用方——实际的任务查询走的是 queryApprovalTasksRawBodyparseApprovalTaskQueryResponseapprovalTaskAPIToInfo 路径。保留未使用的代码会混淆维护者对实际数据流的理解。

建议(不阻塞合并)

  • approval get--output 可考虑也支持 raw-json,与 approval task query 保持一致
  • Long 描述中示例的缩进可与其他命令对齐(参考 approval_task_query.go

修复后请 push,我会合入。

@forwaryan forwaryan force-pushed the feature/add-approve-read branch 2 times, most recently from 7b97aa4 to 3dc1126 Compare March 29, 2026 14:28
@forwaryan forwaryan force-pushed the feature/add-approve-read branch from 3dc1126 to e7a886a Compare March 29, 2026 14:29
@forwaryan
Copy link
Copy Markdown
Author

forwaryan commented Mar 29, 2026

已处理:

  1. 删除了未使用的 approvalTaskToInfo 及测试 TestApprovalTaskToInfo,避免与实际链路 queryApprovalTasksRawBody -> parseApprovalTaskQueryResponse -> approvalTaskAPIToInfo 混淆。
  2. approval get 已补充 --output raw-json 支持,并同步调整了帮助文案和示例缩进。

已 push,麻烦再帮忙看一轮。

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants