Repository navigation
Conversation
…stable Todo ids Extend the maintainer-correction input to v1 with a required source_event (issue_comment/review/review_comment id, author, timestamps) that LoopX verifies against body-free PR comment and review metadata. v0 inputs stay accepted as host_asserted. Admission runs deterministic gates in order (event observed, revision not stale, not self/bot author, maintainer association, PR not terminal) and records every rejection in the pr-lifecycle ledger as a context_only entry with its reason instead of dropping it silently. The correction Todo is written under a stable operation id derived from repo, PR, event id and updated_at (or the correction fingerprint for v0), so replays yield already_exists even after the Todo is done, distinct events with identical wording yield distinct Todos, and a newer revision supersedes the earlier entry. The ledger row gains maintainer_corrections entries written with todo_id null before the Todo write and filled after; each execute run re-gates and retries pending accepted entries, refusing a stopped Goal with goal_not_active. Legacy maintainer_correction fields keep the latest accepted correction for older readers. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Signed-off-by: hhyykk <4408344+hhyykk@users.noreply.github.com>
Address review findings on maintainer-correction admission: - a stale replay keeps the existing superseded entry, its todo_id and superseded_by link instead of rebuilding it - event timestamps are canonicalized to UTC before identity hashing, ordering and Todo text, so equivalent forms share one operation id - the Goal is resolved before the first ledger write; a missing Goal fails cleanly without leaving a pending entry - a replay with a different host summary settles from the stored todo_intent and reports host_summary_mismatch instead of erroring - the test-only fault hook fires only before a first Todo write - fold duplicate entry reader and readback receipt literals Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Signed-off-by: hhyykk <4408344+hhyykk@users.noreply.github.com>
…return results A maintainer-correction patch can record which source-side request it answers, with the origin runtime root and registry recorded in the project-local ledger. Cancellation of the origin request is checked against the source registry before any effect and fails closed when the origin record is unreadable. Bound identity (goal_id, agent_id, goal_ref) is resolved from the origin request entry itself, never from project-side CLI arguments, and is re-resolved and compared on every settlement run. Terminal outcomes return to the origin conversation through the existing roundtrip report owner with explicit receipt/authority/misroute reason codes; disabled origins change nothing. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Signed-off-by: hhyykk <4408344+hhyykk@users.noreply.github.com>
Use the full head SHA in the return update id so distinct heads sharing a short prefix each enqueue their own result. Compare the freshly resolved goal ref with the stored binding and pass it to the return report. Only OSError/ValueError/KeyError become origin_request_unreadable; programming errors propagate. Document that cancellation suppresses later returns. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Signed-off-by: hhyykk <4408344+hhyykk@users.noreply.github.com>
…igin return Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Signed-off-by: hhyykk <4408344+hhyykk@users.noreply.github.com>
…e CLI adapter Move maintainer-correction settlement and origin-request identity resolution into issue_fix/correction_settlement.py unchanged, and replace the new Any annotations with TypedDicts and object-typed mappings. Keeps cli.py within its reviewed Any ceiling and the default module line limit. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Signed-off-by: hhyykk <4408344+hhyykk@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 309cbf5321
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| accepted = reason is None | ||
| replay = existing_entry is not None | ||
| if existing_entry is not None: | ||
| entry = dict(existing_entry) |
There was a problem hiding this comment.
If the first observation lacks event metadata, the correction is persisted as context_only. Retrying the same event revision with valid metadata then reports an accepted admission, but this branch preserves the rejected entry unchanged. Because it is also classified as a replay, no todo_intent is generated, and settlement permanently skips it. Verified through the File-backed CLI: the second run reports accepted, while the ledger remains context_only and no correction Todo exists. Distinguish previously rejected observations from previously accepted replays so a transient metadata failure does not permanently suppress valid work.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 68abedc. An existing entry is now treated as a replay only when the current observation is rejected or the existing entry is already accepted/superseded. A verified retry of an event first recorded as context_only therefore rebuilds the entry as accepted and emits Todo intent once; a later identical run stays a no-op replay.
Regression: test_unverified_correction_is_accepted_on_verified_retry (context_only -> accepted with one Todo -> replay without a duplicate). Validation at 68abedc: issue_fix/catalog capability tests 235 passed, 20 adversarial checks passed, maintainer-correction smoke passed, ruff clean, loopx canary premerge --from-git-diff --git-diff-base origin/main passed.
| refusal = recheck_issue_fix_maintainer_correction_effect_gates( | ||
| entry, | ||
| observation=observation, | ||
| ledger_entries=ledger_entries, | ||
| goal_stopped=goal_stopped, | ||
| ) |
There was a problem hiding this comment.
If execution stops after recording an accepted entry but before creating its Todo, a later replay can fail provider verification and still create the Todo. Settlement reloads the earlier accepted entry, and this recheck does not incorporate the current admission's source_event_not_observed or source_event_revision_mismatch rejection. Using the supplied fault hook, a replay with the event absent reported context_only yet created the patch Todo. Carry current event-verification failures into settlement so an interrupted correction cannot bypass admission after its source is deleted or edited.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 68abedc. When the current admission fails event verification (source_event_not_observed or source_event_revision_mismatch), settlement refuses the accepted-but-unsettled entry with that reason and creates no Todo. The entry stays pending, so a later verified observation can still settle it.
Regression: test_pending_correction_requires_current_event_verification (interrupt after the accepted write, then replay with the event absent and with a revision mismatch: no Todo; replay with a matching event: one Todo). Validation at 68abedc: issue_fix/catalog capability tests 235 passed, 20 adversarial checks passed, maintainer-correction smoke passed, ruff clean, loopx canary premerge --from-git-diff --git-diff-base origin/main passed.
| update_id=(update_id if any( | ||
| r["phase"] == "conclusion" for r in reply_status(origin_root, origin) | ||
| ) else None), workspace=project, request_revision=view["revision"], |
There was a problem hiding this comment.
If the first report() succeeds but the subsequent ledger write fails, retrying the same operation/head publishes a duplicate conclusion. The first attempt passes no update_id; the retry sees that conclusion and switches to the computed update ID, which the result owner treats as a new update rather than a replay. Fault injection at the return-ledger write reproduced two queued replies for one operation/head, with only one recorded return. Persist the selected publication identity before enqueueing, or reconcile the existing initial conclusion before choosing an update, so recovery retains the promised deduplication.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 68abedc. Before calling report(), settlement durably records a pending_return with the head and the selected publication identity (initial or computed update id). A retry after an interrupted return-ledger write reuses that identity instead of switching to an update, and the marker clears with the return record. A genuinely new head still publishes an update.
Regression: test_return_ledger_failure_preserves_publication_identity (fault at the return-ledger write after a successful report: one queued conclusion and one recorded return after retry). Validation at 68abedc: issue_fix/catalog capability tests 235 passed, 20 adversarial checks passed, maintainer-correction smoke passed, ruff clean, loopx canary premerge --from-git-diff --git-diff-base origin/main passed.
…osed - A verified retry of an event first recorded as context_only now upgrades the ledger entry and emits Todo intent once; accepted entries still replay as no-ops. - Settling an accepted-but-unsettled correction honors the current observation: an absent event or revision mismatch leaves it pending without creating a Todo. - The return publication identity is persisted before report(), so a retry after an interrupted return-ledger write reuses it instead of publishing a duplicate conclusion; a new head still publishes an update. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> Signed-off-by: hhyykk <4408344+hhyykk@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68abedc608
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| result = report(origin_root, identity["goal_id"], identity["agent_id"], | ||
| origin["request_id"], "conclusion", text, registry=Path(origin["registry_path"]), | ||
| update_id=pending["update_id"], workspace=project, request_revision=view["revision"], |
There was a problem hiding this comment.
If report() succeeds but the final return-ledger write is interrupted, a retry at the same head fails when CI status or PR state has changed: the new pending_return preserves the publication identity, but this call uses text rebuilt from the latest observation. planCollaborationResult() rejects changed content under an existing identity. Reproduced with the File-backed CLI by interrupting the final ledger write and changing checks from passing to failing: the retry exits with reply already committed; conflicting replacement rejected, and returns remains empty. Persist the publication text and request revision alongside its identity, or reconcile the committed result, so recovery can replay the exact publication.
Useful? React with 👍 / 👎.
loopx-agent
left a comment
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent | gpt-6.1-sol | OpenAI | runtime_reported | reasoning_effort=max
动机 / Motivation
维护 PR 的 Agent 和等待原请求结论的用户。维护者给出修正后，进程恰好在写入待办或回传结果时退出；原来依赖摘要和重跑，容易丢掉后续工作或重复处理。本 PR 尝试用来源事件身份、稳定待办和原请求回传把修正接续起来，但两条恢复分支仍未闭合。
可验证的目标是修正可靠形成一项待办，完成后回到原请求，并能在中断后继续。当前 head 已修复此前三个具体问题，但普通轮询复核和结果恢复仍失败。本评审不授予合并或执行权限；不验收真实 Lark 投递、安装后采用、长期成本收益，也不把排队视为送达。
改动思路 / Approach
该方案沿用 issue-fix pr-lifecycle，把维护者修正从 Host 的文字判断扩展为带来源事件身份和版本的输入。GitHub adapter 负责分页读取不含正文的事件元数据；admission 判断事件、身份关联和 PR/head 条件；私有 ledger 保存恢复映射，真正的待办和原请求结果仍由现有 native owner 决定。完成待办后，在新的完整 head 上调用原请求的 report，并分别展示排队和投递状态。
Provider IO、修正 eligibility、ledger projection 与 native Todo/result owner 的分工符合现有边界；两个修复都应落在 pending settlement 和不可变 publication 快照内，现有证据不要求新框架或整体语言迁移。本 PR 可交付同一 CLI 中的验证、待办、恢复与原请求排队闭环；当前两项恢复缺陷须在此边界修复，真实投递和配置化 actor-login 是明确未验收范围。不改动确实会留下修正接续和原请求回传的缺口；让 Host 靠重复文字指令手工同步也无法证明来源和幂等性。当前分工有合理价值，但不能因为增加了恢复账本就认定恢复成立。
| 中断点 | 当前 head 的实际恢复结果 |
|---|---|
| 事件最初未取得，稍后同输入验证成功 | PASS：同一 operation 创建一个 Todo |
| 已保存 pending entry，随后同 correction 输入的事件缺失/被编辑 | PASS：拒绝新 Todo |
| 同一 pending entry，随后普通轮询不传 correction | FAIL：缺失或被编辑的事件仍生成 Todo |
| 已入队结果后 ledger 写入中断，观察值相同 | PASS：一个结论、一个 return mapping |
| 同一中断，但同 head 的 checks 变化 | FAIL：稳定键冲突，return mapping 无法补齐 |
上表来自实际 CLI、File/SQLite 和真实内部 outbox，不代表真实外部消息投递。
具体改动 / Changes
完整评审范围为 111519af76e56141c56eb7a27443da59def775b9 → 68abedc6088c4b24df211ced9d79135cb48e40df，10 个文件，+2460/-67；期间另独立检查 309cbf532152e58cdf3ce14fb612a5ed3437171a → 68abedc6088c4b24df211ced9d79135cb48e40df 的四文件修复，没有发布旧 head 的结论。原来的 8 个反例在旧 head 全部失败，保留原断言、输入和公共调用后在当前 head 全部通过；这只关闭原先的具体路径，不能覆盖下述新增的六个恢复反例。
规范依据是修改前的 loopx/capabilities/issue_fix/README.md#pr-lifecycle-monitor，revision 111519af76e56141c56eb7a27443da59def775b9，并参照同 revision 的 loopx/control_plane/collaboration/README.md 原请求和不可变结果契约。v1 功能细节来自本 PR 的自包含请求，没有另行书面 RFC；以下是描述性评审锚点，并非另造 RFC 条款。
- material-correction-successor — not_met：修正应成为可执行 successor/blocker，来源在效果前有效。普通轮询恢复漏掉 provider 事件复核，见 P2-1。
- quiet-replay-and-rollback — implemented：保留现有监控、v0 和 legacy 字段；原 8 个反例及普通监控重放验证了单个待办身份和终态处理。未把新 v1 当成旧 base 已有功能。
- origin-result-recovery — not_met：沿用原请求 native owner 和稳定结果身份，但目前只持久化部分 publication，见 P2-2；排队、送达和采用仍是不同状态。
关键代码讲解
pr_lifecycle.py::build_issue_fix_pr_lifecycle_monitor_packet扩展 v1 事件、association、来源版本及拒绝原因；context_only 在后续验证成功时可以升级为 accepted。recheck_issue_fix_maintainer_correction_effect_gates保留 Goal/PR/head/修订约束。v0 兼容有现存调用和持久化依据，不应为减行数直接删除。correction_settlement.py把恢复写待办与原请求结果的流程放在已有 capability 内。其第 220–222 行把新的 provider 拒绝判断限制为is_current；第 338–350 行只保存head_sha/update_id，随后用当前观察重建文本并调用report，分别构成两个剩余缺陷。github_public.py::fetch_github_pull_request_event_metadata读取 comment、review、review-comment 的分页元数据，不导入正文。我独立执行过仅含这三类来源的公开只读读取，共 13 个事件；没有读取远端 CI，也未执行外部消息写入。domain_packs/issue_fix.py::upsert_issue_fix_pr_lifecycle_ledger_jsonl合并事件 entries、Todo/return 映射并保留 legacy observation；cli.py负责 origin flags、绑定解析和 execute 调度。两个新增测试模块及 durable smoke、英中 README 均在完整 diff 范围内，不把测试数量当成交付证明。
对主干的风险 / Risks
P2-1：普通轮询恢复也要复核待写事件。 当前 correction_settlement.py 第 220–236 行 只对 current correction 的 source failure 拒绝写入。实际先接纳一个 v1 event，并在 Todo 写入前中断，下一次用当前 metadata 普通轮询、不给 correction：原事件缺失或被编辑时，is_current 为 false，四个 File/SQLite 组合仍各写入一个 Todo。当前 same-input 修复的绿色测试无法覆盖它。复核每个尚未写入 Todo 的 v1 entry 的 provider event/revision；普通轮询和不同 current correction 也必须走同一复核，再保留稳定 operation_id 恢复。请补普通轮询以及另一个 current correction 的恢复用例，断言无效 pending event 零新增工作，合法恢复仍只生成同一操作对应的一个待办。
P2-2：恢复时重放完整的原 publication。 我独立验证了现有讨论。真实 report 已提交，紧随其后的 ledger 写入中断；在相同完整 head 上只把 synthetic checks 从 SUCCESS 改为 FAILURE，再重试，第 349 行 的当前文本与已提交文本不同，native owner 正确拒绝 conflicting replacement。两种 backend 都 exit 1，仍只有一个 queued reply、零 return mapping；当前问题是恢复停住，并非声称再次重复投递。首发前保存完整不可变 publication（至少 text、request_revision 及 result identity），或读取并接纳已提交的 publication；恢复先完成原记录，新的观察另作合法 update。回归应保留真实 report/outbox，只在其后的私有 ledger commit 注入故障，并覆盖同 head 的可变观察及 request revision。
语义与 CI 对齐
这是对已有 issue_fix vocabulary 的扩展，不需要另造 actor lifecycle 或通用权限协议。provider_verified 只代表事件身份/版本匹配，不代表新增执行、合并或发送权限；当前仅有 [bot] suffix 过滤，配置化 actor-login 明确 deferred，不能声称普遍过滤所有自身消息。provider gate 是 enforced obligation，不能称为可忽略 guidance；原结果不可变性也不能用新的文本覆盖规避。两个 blocker 应在现有 capability 和 publication owner 边界修复。
| 验证 | 当前结果及限制 |
|---|---|
pytest -q tests/capabilities -k "issue_fix or catalog" |
PASS，235 项，当前完整 head |
| 独立恢复反例 | FAIL，14 项中 8 PASS、6 FAIL；File/SQLite 均复现上述两处 |
diff-selected canary premerge |
PASS，6 个 direct checks、19 个 selected checks（10 catalog、8 risk、1 public boundary） |
| Ruff，8 个 changed Python paths | PASS |
| 默认关闭/普通监控比较 | PASS，有明确比较边界，见下段 |
| 远端 CI、真实 Lark 投递、安装后采用 | 未查询 CI；其余 UNVERIFIED，未借此推断代码通过或失败 |
默认关闭不是从对象缺席推断：在不可变 base 与初审 head 进行了 28 次真实 CLI 调用。8 个固定时间 preview packet 完全相同；File、SQLite 的 OPEN→重复 OPEN→MERGED 都保留同一记录，状态 open/open/done。执行输出有 54 个实际差异：独立 fixture/store/Todo 标识、IO receipt 路径和哈希，以及 head 新增的空 maintainer_corrections:[]；这些没有被伪称为全字节等价，旧 base validator 也接纳了六个实际普通执行 packet。当前修复只改 active correction/return 分支，base、依赖和普通 caller 未变，故复用该证据；当前 active 恢复则重新完整执行。未穷尽分页次序、生产性能或安装后消费者，未作这些资格声明。
我的整体评价 / Verdict
REQUEST_CHANGES，针对 68abedc6088c4b24df211ced9d79135cb48e40df。长程持续工作的判断为 not_yet_proven：普通重启可从缺失/改版事件产生工作，已提交结果也可能无法恢复映射。用户体验同为 not_yet_proven：实际任务是用户在原请求拿到修正后的结果，不是成功配置几个 flags；已有身份和授权能够复用，无须加确认轮次，但恢复失败仍会转为人工处理。
Provider IO、修正 eligibility、ledger projection 与 native Todo/result owner 的分工符合现有边界；两个修复都应落在 pending settlement 和不可变 publication 快照内，现有证据不要求新框架或整体语言迁移。本 PR 可交付同一 CLI 中的验证、待办、恢复与原请求排队闭环；当前两项恢复缺陷须在此边界修复，真实投递和配置化 actor-login 是明确未验收范围。兼容性保留有实际 v0 caller/legacy ledger 依据；不要求独立新框架、通篇 TypeScript 重写或未被接受的将来 RFC。最有价值的局部收敛是所有 pending entries 统一走当前 source recheck，并让恢复重放完整不可变 publication。视觉检查不适用：该 diff 改 CLI/ledger/结果 payload，没有渲染布局变化；真实 Lark 展示/投递、安装 Host 与模型采用仍未验证。
复审需要两个最小修复，在同一真实 CLI、File/SQLite 和 outbox 边界上使六个当前反例通过，同时保留原八个反例与 235 项测试的有效断言，并重跑相关 premerge。既不因旧失败自动否定新 head，也不因原三个具体缺陷已修复继承 Approval。
English verdict: REQUEST_CHANGES at 68abedc6088c4b24df211ced9d79135cb48e40df. Pending-source revalidation and immutable publication recovery still fail in 6 real CLI cases; 235 repository tests and local premerge pass.
Goal And Delivered Outcome
loopx-project/loopx:main. No issue closure claimed.Author Declaration
Implemented against
origin/main.loopx/capabilities/issue_fix/pr_lifecycle.pytests/capabilities/test_issue_fix_maintainer_correction_admission.pyloopx/capabilities/issue_fix/pr_lifecycle.pytests/capabilities/test_issue_fix_maintainer_correction_admission.pyloopx/capabilities/issue_fix/github_public.pytests/capabilities/test_issue_fix_maintainer_correction_admission.pyloopx/capabilities/issue_fix/pr_lifecycle.pytests/capabilities/test_issue_fix_maintainer_correction_admission.pyloopx/domain_packs/issue_fix.pytests/capabilities/test_issue_fix_maintainer_correction_admission.pyloopx/capabilities/issue_fix/correction_settlement.pytests/capabilities/test_issue_fix_maintainer_correction_admission.py;tests/capabilities/test_issue_fix_correction_origin_return.pyloopx/capabilities/issue_fix/pr_lifecycle.py;loopx/capabilities/issue_fix/correction_settlement.pytests/capabilities/test_issue_fix_maintainer_correction_admission.py;tests/capabilities/test_issue_fix_correction_origin_return.pyloopx/capabilities/issue_fix/cli.py;loopx/capabilities/issue_fix/correction_settlement.pytests/capabilities/test_issue_fix_correction_origin_return.pyloopx/capabilities/issue_fix/correction_settlement.pytests/capabilities/test_issue_fix_correction_origin_return.py; queueing is not confirmed deliveryAny),cli.pystays within its reviewedAnyceiling and the default line limitloopx/capabilities/issue_fix/correction_settlement.py;loopx/capabilities/issue_fix/cli.pyexamples/control_plane/control-plane-maintainability-ratchet-smoke.pyloopx/capabilities/issue_fix/README.md; loopx/capabilities/issue_fix/README.zh-CN.mdloopx/capabilities/issue_fix/pr_lifecycle.pyloopx/capabilities/issue_fix/README.mdloopx/capabilities/issue_fix/pr_lifecycle.pyScope And Continuation
Validation
unitpassedpytest tests/capabilities -k "issue_fix or catalog": 235 passed (admission gates, verified retry upgrading a context_only entry, current verification failures blocking pending settlement, publication identity surviving an interrupted return-ledger write, stable operation ids, supersede/stale, crash reconciliation, origin binding, cancellation, return once per full head, goal-ref mismatch, programming errors propagate), File and SQLite authority.regression_paritypassedreal_entrypointpassedexamples/issue-fix-maintainer-correction-smoke.pythrough the CLI.staticpassedruff check;loopx canary premerge --from-git-diff --git-diff-base origin/main(standard tier) including maintainability ratchet and semantic vocabulary drift; merge gate passed.github_public.py) is covered through fixture payloads, not a live API call. No real Lark or GitHub writes were made. After cancellation, later completed corrections are deliberately not returned; the request's cancellation record is the accountability trail (documented in both READMEs).See validation disclosure guidance.
Frontend / Visual Evidence
Type of Change
LoopX Area
Technical Direction
Shared-authority RFC fixture impact
Boundary Checklist
none.Signed-off-bytrailer (git commit -s).