fix: reclaim stale automation actions - #56
ggboyxkw666 wants to merge 2 commits into
Conversation
XiaoCow666
left a comment
There was a problem hiding this comment.
CodeSense 自动评审
变更将提交详情接口读取的会话字段从 user_type 修正为 usertype,并新增管理员、本人和其他学生三类访问边界的回归测试。根据提供的 diff,未发现安全风险、明显回归、无法运行证据或任务目标未完成的问题。
已有验证信息
- 新增 tests/test_submission_api_authorization.py,覆盖管理员读取其他学生提交、学生读取本人提交、学生读取他人提交返回 403 三种场景。
- 事件未提供测试命令执行或 CI 结果,无法确认新增测试已实际通过。
评审事件:aab01250-b2b0-11f1-8c93-39b58aeab2af
63325cf to
82e3ea8
Compare
XiaoCow666
left a comment
There was a problem hiding this comment.
CodeSense 自动评审
该 PR 声明的目标是修复“submission detail API”中的管理员访问权限,但所提供的 diff 仅修改了 Cloudflare automation 的 action_log 超时任务认领逻辑及其测试,未包含任何 submission detail API 的鉴权变更。因此任务目标未完成,不能合并为该修复 PR。
需要先处理的问题
- 任务目标未完成:PR 标题为“fix: honor admin access in submission detail API”,但变更文件仅为 cloudflare/automation/src/index.js 和 cloudflare/automation/test/review-gates.test.js,内容是允许回收超过 15 分钟仍处于 running 状态的 action_log 记录。diff 中没有 submission detail API、管理员角色判断或授权逻辑的修改,无法实现所声明的管理员访问修复。
请按下面的步骤修改
- 请在负责 submission detail API 授权的文件和路由/处理函数中补齐实际修复:当前 diff 没有任何管理员授权判断,因此管理员仍无法因本 PR 获得预期访问权限。应将访问控制改为在资源所有者之外,具有项目定义的管理员权限/角色的已认证用户也可读取 submission detail,同时继续拒绝无权限用户。可直接交给 AI 的操作提示:"定位 submission detail API 的路由和鉴权逻辑;保留 owner/现有授权用户访问;新增项目现有 admin 判定的允许分支;确保未认证和非 owner 非 admin 用户仍返回原有拒绝结果。"
- 请为 submission detail API 的授权修复添加或更新对应测试文件:当前新增测试只覆盖 cloudflare automation 的 claimAction,不能证明管理员访问 submission detail 已修复。应至少覆盖管理员访问成功、资源所有者访问仍成功、普通无权限用户被拒绝,以及现有认证边界行为。可直接交给 AI 的操作提示:"找到 submission detail API 的现有测试;增加 admin 用户读取非本人 submission detail 成功的断言,并保留/新增非 admin 非 owner 被拒绝的断言;使用仓库现有的角色、会话和响应状态码测试约定。"
合并后可以继续改进的地方
- cloudflare/automation/test/review-gates.test.js 中新增了 stale running、recent running、completed 和 failed action 的单元测试覆盖,测试意图与 action_log 回收逻辑一致;但事件未提供这些测试或 CI 已实际通过的证据。
- PR 标题与当前实际修改范围不一致。若 action_log 超时回收是独立修复,建议拆分为单独 PR 并使用与实际内容匹配的标题,避免掩盖 submission API 授权修复的缺失。
已有验证信息
- 事件提供的 diff 新增了 claimAction 的单元测试代码,但未提供测试命令输出、CI 状态或其他已通过检查的证据。
评审事件:817be7e0-b2b2-11f1-8c63-7fbdf947ac41
XiaoCow666
left a comment
There was a problem hiding this comment.
通过复审。依据提供的 diff,未发现需要阻塞合并的必要修改项。本次仅进行静态检查,没有运行测试。
已检查范围:
cloudflare/automation/src/index.js的claimAction:允许重新领取超过 15 分钟的 running 记录;UPDATE 同时检查状态和时间,能够防止并发请求再次领取已更新时间的记录。completed 和近期 running 记录继续返回 false,failed 保留重试路径。cloudflare/automation/test/review-gates.test.js:新增四个用例,覆盖超时 running、近期 running、completed 和 failed 分支。
后续建议(不阻塞):actionLogDb 按预设值返回更新数量,没有执行 SQL,也没有检查绑定参数。建议补充使用真实 SQLite 的测试,验证时间边界,以及同一超时记录被连续领取时只有首次成功,增强对重复执行问题的检验能力。
提交说明与验证提醒:当前标题提及 admin 和 submission detail API,与此次 diff 内容不符。建议修改为自动化 action 超时重试相关标题,并在 PR description 中说明 15 分钟重新领取条件和四个测试场景。运行 node --test cloudflare/automation/test/review-gates.test.js,记录实际结果;当前审查不代表测试已经通过。
合并前隔离验证
- 结果:未通过
- 命令:gh repo clone
- 耗时:6438ms
- 摘要:Cloning into 'E:\CodeSense\pr-review-relay\data\work\premerge-vnr4TB\repo'...
fatal: unable to access 'https://github.com/XiaoCow666/CodeSense.git/': schannel: failed to receive handshake, SSL/TLS connection failed
failed to run git: exit status 128
合并前验证未通过,本次不合并。请按上面摘要逐条处理:打开摘要中点名的文件和位置,把当前错误改成命令要求的行为;修复后重新运行同一条命令,再提交新的 head。
XiaoCow666
left a comment
There was a problem hiding this comment.
CodeSense 自动评审
本 PR 的 diff 只修改了 automation action claim 逻辑及其测试,没有实现标题所述的 submission detail API 管理员权限修复,因此核心目标未完成。现有 action reclaim 改动本身未发现明确阻塞问题,但不能替代所声明的 API 鉴权变更。
需要先处理的问题
- PR 标题目标是让 submission detail API 正确尊重管理员访问权限,但 diff 未修改任何 submission detail API handler、鉴权逻辑或相关测试,核心需求未实现。
请按下面的步骤修改
- 位置:submission detail API 的实际 handler 或鉴权函数;现在:管理员访问权限修复未出现在本次 diff 中,当前实现无法证明管理员能够按预期访问 submission detail;改成:在 API 鉴权逻辑中明确允许具备管理员权限的请求,并保留非管理员只能访问其有权查看的 submission 的限制,同时添加覆盖管理员、普通用户和未认证请求的回归测试;交给 AI:请定位并修改 submission detail API 的实际鉴权实现,补充上述权限场景测试,并运行该 API 及 cloudflare/automation 相关测试。
已有验证信息
- 事件中未提供测试运行结果、CI 检查结果或其他验证证据。
评审事件:d7e25310-b377-11f1-8657-4e38278c403e
XiaoCow666
left a comment
There was a problem hiding this comment.
通过复审;提供的 diff 中没有发现必要修改项。
已检查范围(仅静态分析,未执行命令):
cloudflare/automation/src/index.js的claimAction:复用staleBefore,并在 UPDATE 中检查failed或超时的running状态。数据库更新时重新检查条件,可以防止其他请求刷新记录后仍按旧查询结果领取任务;返回值继续依赖实际更新行数。cloudflare/automation/test/review-gates.test.js:新增测试覆盖超时领取、连续两次领取、近期运行、已完成和失败重试。近期运行与已完成记录继续跳过,失败记录保留重试路径。
后续建议(不阻塞合并,按优先级排序):
only the first retry can reclaim the same stale action当前依次执行两次调用,第二次可以在 SELECT 后提前返回。建议增加使用真实 SQLite 或本地 D1 的交错请求测试,让两个请求都读取旧记录后执行 UPDATE,确认更新行数分别为 1 和 0。当前测试替身自行实现状态判断,无法验证 SQL 的实际执行行为。- 增加
updated_at恰好等于staleBefore的边界测试,确认 JavaScript 判断与 SQL 的<=条件一致。
贡献者下一步可运行 npm --prefix cloudflare/automation test、node --test cloudflare/automation/test/review-gates.test.js 和 git diff --check。当前 PR description 已说明复现、变更和验证限制;追加提交后,请更新对应验证结果并注明提交 SHA。正文中的通过记录属于贡献者报告,本次复审未独立运行验证。
合并前隔离验证
- 结果:未通过
- 命令:gh repo clone
- 耗时:5096ms
- 摘要:Post "https://api.github.com/graphql": EOF
合并前验证未通过,本次不合并。请按上面摘要逐条处理:打开摘要中点名的文件和位置,把当前错误改成命令要求的行为;修复后重新运行同一条命令,再提交新的 head。
Summary
Fix the durable automation action claim lifecycle after a worker interruption. A stale
runningaction older than 15 minutes can now be reclaimed atomically, while recent work and completed work keep their existing ownership and idempotency behavior.This PR is limited to the Cloudflare automation action-log path. It does not change CodeSense submission-detail authorization, GitHub/Feishu API contracts, D1 schema, deployment configuration, secrets, or merge behavior.
End-to-end path
GitHub/Feishu event → Cloudflare Queue →
persistEvent→runAction/claimAction→ D1action_log→ AI review or GitHub/Feishu side effects →event_inboxresult.If a Worker stops after claiming an action but before marking it completed or failed, the retry sees the D1 row as
running. Before this change, onlyfailedrows were reclaimable, so the same action could be skipped indefinitely. The new conditional UPDATE allows only a stalerunningrow to be reclaimed.Reproduction before the fix
In an isolated local worktree, a D1-like state with
status=runningandupdated_at16 minutes ago produced:{"claimed":false,"calls":["INSERT","SELECT","UPDATE"]}The final UPDATE affected no row because the old condition only accepted
status='failed'.Changes
failedrows as before.runningrows only whenupdated_at <= staleBefore.runningrows owned by the active worker.completedrows idempotent and non-reclaimable.Validation
Commands run from the repository root unless noted:
npm testincloudflare/automation: 18 tests passed, 0 failed.node --test cloudflare/automation/test/review-gates.test.js: 14 tests passed, 0 failed.git diff --check: passed.{"claimed":true}for a stale running action.Compatibility and rollback
completedactions remain skipped.runningactions remain skipped to avoid duplicate side effects.failedretry behavior remains.82e3ea8or of this PR; no schema rollback is required.Risks and unverified items
runningrows recoverable.AI assistance
AI was used to locate the event → queue → action-log lifecycle, compare the existing retry conditions, and suggest the conditional-update shape. The final scope, stale-state reproduction, test-double behavior, and test results were checked locally. A suggestion to add a real SQLite dependency was not adopted because the project has no Node database test dependency and the production contract is Cloudflare D1; the regression instead uses a stateful D1-shaped mock and explicitly records live D1 as unverified.