Skip to content

refactor: reuse delivery guards and derive order email status from EffectJob - #114

Merged
LIghtJUNction merged 1 commit into
mainfrom
codex/architecture-reuse-notifications-20260928
Sep 28, 2026
Merged

LIghtJUNction merged 1 commit into
mainfrom
codex/architecture-reuse-notifications-20260928

Conversation

@LIghtJUNction

Copy link
Copy Markdown
Contributor

Scope

Refs #85 and #81; design/coordination refs #71, #72, #83. This is the first bounded implementation slice of the user-approved architecture convergence, not closure of the umbrella issues.

Base: acc542ab7cbb6983c1e44d294e116d2b0eb6ed68 (main push CI 36388965574 succeeded). Head: 2fc4916b6cfc36889d9c5cd0b5972b3d58166720. All ten submitted blobs match the locally checked source/test/docs by Git blob SHA.

Actual changes

  • Share live account-email lookup, enablement, strict address validation and safe authenticated pickup-link construction across legacy and v3 notification paths. Keep each endpoint encoding, generation binding and wire message format.
  • Reject all ASCII control characters in v3 addresses and credentials/query/fragment/control/backslash/malformed origins in pickup URLs, without echoing secrets. Previously the newer mail path did not use the legacy origin guard.
  • Read v3 notification state from its linked, buyer/order/endpoint-bound EffectJob plus result. Missing jobs do not prove delivery; failed jobs show failed; disabled and uncertain work is not reactivated.
  • Remove four market-projection callbacks from generic effect completion/retry/expired-lease handling. The worker updates job facts, not order rows; existing attempt/state/deadline and current-authority checks are retained.
  • Keep actual goods delivery, buyer acceptance/claim and ledger settlement separate. No migrations, issued schemas, short codes, money movement, historical receipts or orders.buy@1/@2/@3 semantics changed.
  • Add docs/ARCHITECTURE_REUSE.md, update the versioned security rule and _index.md; Google authoritative design and issue [P2] 跨模块代码质量与测试债:拆分长路径、统一不变量 #85 have also been updated.

Verification

Local Python 3.13.5 unit evidence only (not a substitute for project Python 3.15/real PostgreSQL):

PYTHONPATH=src python -m pytest -q tests/test_market_notification_reuse.py -k unit
# Before implementation: 49 failed, 9 passed, 2 integration cases deselected.
# After implementation: 58 passed, 2 integration cases deselected.
python -m compileall -q src tests/test_market_notification_reuse.py
git diff --check

The added two real-application PostgreSQL tests verify (1) SMTP completion changes no order, delivery, payment or immutable contract rows while read projections report the current job result; (2) the lifecycle renderer enforces the same invalid-origin rejection. They are part of normal full CI collection, not skipped/xfail. All existing worker-lease, ownership, email, marketplace, schema, migration and transport tests remain in the complete four-shard CI gate. Target-environment full CI is pending verification; do not merge on the local unit result alone.

Integration boundaries / remaining work

#103 and #112 remain untouched. This PR has small overlapping edits in market/orders.py, market/targets.py and the worker files; integrate by preserving their financial/recovery changes and this job-state projection, not replacing either entire file. No CI/workflow/shared schema/config change and no production deployment, issuance, live banking or outbound email.

ServerOffer→Listing, Purchase→Order, typed account migrations and a new default explicit-acceptance policy are recorded remaining work, not implemented here. Existing explicit auto-settlement policies must retain their meaning and regression tests.

…ery guards

Preserve checkout versions, endpoint generations, signed payloads and worker lease fences. Remove worker writes to order notification projections and consolidate account-email, address and pickup-link validation.

Add 58 boundary tests and two PostgreSQL integration regressions; publish scoped architecture guidance and versioned system rule changes.

Refs #85, #81, #71, #72.
Copilot AI lite review requested due to automatic review settings September 28, 2026 07:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
🔒 Security Review ✅ Completed 2026-09-28T07:43:11.782508Z 2fc4916 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Copy link
Copy Markdown
Contributor Author

本轮已复核本 PR 全部 10 文件改动,并下载当前 head 2fc4916b6cfc36889d9c5cd0b5972b3d58166720 的 CI 36392802044 四个产物,逐个核对 GitHub 公布 SHA-256;用该源码的 scripts/ci_shards.py check 4 校验 JUnit msg.nodeid 与分片 manifest:1324 项核心测试完整、不重复、不遗漏且零失败/错误/跳过;conformance/build 运行状态 success。没有未解决 review thread;Copilot 配额耗尽不计外部审核批准。

确认此切片仅复用收件地址/提货地址校验并把邮件状态回归 EffectJob 事实,保留旧接口/消息格式、endpoint generation、当前授权、attempt/deadline 栅栏。未改历史签署数据、资金流转或 orders.buy@1/@2/@3。将使用 expected head SHA 合并;#85/#81 仍按完整验收范围保持开放。随后在独立集成分支处理 #112/#103 冲突,不覆盖双方安全逻辑,也不进行生产部署或实际外发。

@LIghtJUNction
LIghtJUNction merged commit 6740175 into main Sep 28, 2026
10 checks passed
@github-actions
github-actions Bot deleted the codex/architecture-reuse-notifications-20260928 branch September 28, 2026 10:15
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