docs(reports): archive phase 2a plan and unanimous pass review reports (49171b58, 3f0784c4, d2e705a8)
This commit is contained in:
1 parent
0e74aa1e4e
commit
4a5916a736
4 files changed
+196
No files matched your search
@@ -1,3 +1,10 @@
|
||||
> **본 리포트는 Job `2770df36`(role: Planner)의 산출물인 Phase 2a 구현 계획서다.**
|
||||
> durable 사본: `.agents/reports/creator-claude-01/plan-2770df36.md` (커밋됨).
|
||||
>
|
||||
> **범위 명시**: 본 job의 지시는 "구현 계획서를 작성"이며, 목표 문장의 (5) 테스트 작성·(6) 리뷰어 만장일치 PASS는 이 계획을 실행할 **후속 Worker/Reviewer job**의 완료 조건이다. 본 계획서는 그 job들이 그대로 실행할 수 있도록 커밋 5개·게이트 9개·인수인계 체크리스트까지 명세했다.
|
||||
|
||||
---
|
||||
|
||||
# 📑 Issue #5 Phase 2a 구현 계획서 — `lib_py/loop/` 코어 & Shadow Watcher
|
||||
|
||||
- **Job**: `2770df36` / **Planner**: `creator-claude-01`
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
# 🔍 Cross-Review Round 3 — Issue #5 Phase 2a (`6ad6f08..0e74aa1`, 신규 커밋 `cb575fa`+`0e74aa1`) — Job `49171b58`
|
||||
|
||||
- **Reviewer**: `planner-reviewer-claude-01` (Reviewer role)
|
||||
- **대상**: 브리프 첨부 diff = `git diff 6ad6f08..HEAD` − `.agents/reports/*`(2914행, **바이트 동일**). HEAD=`0e74aa1`, working tree clean. Round 2(`cb792e91`) 대비 신규는 수정 커밋 `cb575fa`(9 files, +317/−52)와 문서 커밋 `0e74aa1`.
|
||||
- **방법**: Round 2의 F-C/F-D와 Codex Round 2(`0026165b`)의 R1/R4 수정을 실제 CLI·실제 모듈로 재검증(F-C는 래퍼와 같은 `set -euo pipefail` 셸에서), Round 1 항목(F-A/F-B)의 회귀 여부 재확인, 신규 테스트 12종 독해, 전체 스위트 실행.
|
||||
|
||||
---
|
||||
|
||||
## 0. 결론 요약
|
||||
|
||||
- **Round 2 지적 전부 해소 — `[ADJUDICATION: SUSTAINED]`**. F-C: 멱등 ACK가 `TransitionResult.idempotent`로 구분되어 CLI가 rc 0을 내고, 래퍼 승인 경로가 `set -e` 아래에서 끝까지 진행된다(§1). F-D: `_operator_write` 첫 줄에서 `VALID_STATUSES` 검증이 두 모드 공통으로 복원돼 rc 1·레코드 불변이다(§2). Round 1의 F-A/F-B도 회귀 없이 유지된다(§3).
|
||||
- Codex R1(관측 증거 **부재** 시 승인)·R4(감사 mirror의 비원자적 비교-후-쓰기)도 구현·테스트를 확인했다(§4). R4는 내가 Round 2 §2에서 비차단으로 언급한 창(window)을 닫는 것이라 특히 반갑다.
|
||||
- **신규 결함 없음.** 자동완료 3표면·`lib.sh`·shim 무변경, 위생 클린, 전체 스위트 통과(§6). `[ESCALATE: PLANNER]` 불필요.
|
||||
|
||||
---
|
||||
|
||||
## 1. F-C 재검증 — 해소
|
||||
|
||||
- `engine.py:83-99` `TransitionResult.idempotent`, `:105-113` `_Decision.idempotent`, `:164-167`에서 같은 terminal 재수신에 `idempotent=True`. `registry.py:329-366` `_operator_write`: `committed` → 0, `idempotent` → 0, 그 외만 `refused:` + 4.
|
||||
- **셸 수준 재현(Round 2와 동일 스크립트)**: 이미 `completed`인 레코드에 `status --set completed`를 `set -euo pipefail` 아래에서 실행 → `REACHED (CAS=1)` / `REACHED (CAS=0)`, bash exit 0. Round 2에서는 exit 4로 `REACHED`가 출력되지 않았다.
|
||||
- `registry.main` 반환값 직접 확인(두 모드): 멱등 `--set completed` → **rc 0**, `cancelled` after `completed` → **rc 4**(CAS=1, 진짜 거부는 여전히 non-zero; CAS=0은 레거시 덮어쓰기 유지).
|
||||
- `test_fc_idempotent_finalize_*`(두 모드), `test_fc_a_genuine_conflict_is_still_reported`(rc 4 유지), `test_fc_engine_marks_an_idempotent_result`가 고정. 문서: `state-transitions.md` Rev.5 "멱등 ACK는 거부가 아니다", `phase2a-shadow.md` §2-1 CLI 종료 코드 표(0/1/4).
|
||||
|
||||
## 2. F-D 재검증 — 해소
|
||||
|
||||
`_operator_write`가 CAS 분기 **이전**에 `status not in VALID_STATUSES`면 2a 이전과 같은 문구로 rc 1. 두 모드에서 `--set bogus` → **rc 1, 레코드 불변**(직접 확인). `test_fd_invalid_status_is_rejected_in_both_modes`가 레지스트리 디렉터리 지문까지 단정.
|
||||
|
||||
## 3. Round 1 항목 회귀 확인 — 유지
|
||||
|
||||
래퍼 시퀀스 재생: `completed → update --status pending --iteration 2` rc 0 · `pending/2`; `completed → status --set error` rc 0 · `error`. 지연 flush 실험: registry=`completed`, status.json=`completed`. 두 모드 병행 특성화(X-01~X-04, fa/fb)가 스위트에 남아 있다.
|
||||
|
||||
## 4. Codex Round 2 (R1, R4) 확인
|
||||
|
||||
| # | 구현 | 검증 |
|
||||
|---|---|---|
|
||||
| R1 증거 부재 시 승인 | `policy.py:20-23` `ATTRIBUTION_FIELDS`; `collect_observed_identity`(`:33-66`)는 식별자를 타임라인 항목의 **`collector_attribution` 봉투**(및 비-herdr 소스 payload)에서만 읽고 네이티브 Herdr payload 안의 식별자는 **증거로 취급하지 않는다**(Phase 0의 "네이티브 payload에 MAM 필드 금지"와 정합); `decide`(`:115-135`)는 4식별자 **전부 존재+일치**와 매니페스트 `attempt_id` 존재+일치를 요구 | `finalized_candidate.json`에 봉투 추가(다른 11종은 `hold`/`reject`라 무영향, 12벡터 `test_g7` 유지). `test_r1_missing_attribution_evidence_holds`(4필드 parametrize), `*_missing_report_manifest_holds`, `*_identifiers_inside_a_native_payload_are_not_evidence`. Phase 0 `validate_watcher_scenario` 41종 통과(봉투는 item 레벨 키라 네이티브 payload 검사와 충돌 없음) |
|
||||
| R4 비원자적 비교-후-쓰기 | `update_logged_status`(`mqtt_common.py:577-619`)가 per-job 전용 락 파일(`.status.json.lock`, `_file_lock`) 아래에서 읽기-비교-쓰기, 임시 파일은 `mkstemp`(공유 `.tmp` 경로 제거). 레지스트리 락과 별개이며 레지스트리 트랜잭션 밖에서 잡힌다 | `test_r4_overlapping_mirror_flushes_*`는 첫 flush의 `read_text` 직후에 경쟁 flush를 스레드로 띄워 **실제로 블록됨**(`blocked is True`)과 최신 상태 생존을 단정 — 진짜 인터리빙 테스트다. `*_each_mirror_write_uses_its_own_temp_file` |
|
||||
|
||||
Grok Round 2(`32e936e5`)는 PASS + 기존 비차단 잔여로, 이번 수정과 충돌 없음.
|
||||
|
||||
## 5. 기타 확인
|
||||
|
||||
- `git diff --check 6ad6f08..HEAD` 클린, `compileall lib_py` 클린, 자동완료 3표면·`lib.sh`·shim 변경 0.
|
||||
- `test_issue5_phase2a_loop.py` + `phase0` + `phase1` 합계 **195 passed in 12.57s**.
|
||||
- 락 규율(G-5)·writer 인벤토리(G-6)·shadow 무기록(G-4)·패키지 규칙(G-9)은 이번 수정으로 영향받지 않았고(테스트 유지), R4의 감사 락은 레지스트리 락 밖에서만 잡혀 G-5의 "락 안 블로킹 금지"와 충돌하지 않는다.
|
||||
- 비차단 메모(2b 인계, 이미 문서화됨): `started`의 세션 확인(O-A), `stop_session.sh`의 무조건 `cancelled`가 `completed` 이후 거부되는 동작(O-B).
|
||||
|
||||
## 6. 전체 스위트
|
||||
|
||||
HEAD(`0e74aa1`)에서 직접 실행 (`.venv/bin/python -m pytest tests/ -q`, 백그라운드): **664 passed in 899.99s (0:14:59)**, 0 failed, 0 skipped. Round 2의 650 대비 +14 (신규 함수 12개가 parametrize로 14 케이스). 총계는 실행 결과에서 직접 읽은 값이다.
|
||||
|
||||
---
|
||||
|
||||
## 7. 결론
|
||||
|
||||
Round 2에서 지적한 두 회귀(멱등 ACK의 거부 분류로 인한 래퍼 성공 경로 중단, 롤백 모드의 상태값 검증 소실)는 제시한 방향 그대로 정확히 고쳐졌고, 래퍼와 같은 셸 조건에서 실제 CLI로 확인했다. Codex의 R1/R4도 구현됐으며, 특히 R4는 감사 mirror의 마지막 경쟁 창을 닫는다. Round 1·2의 모든 항목이 회귀 없이 유지되고 새 결함은 발견되지 않았으므로 통과시킨다.
|
||||
|
||||
[ADJUDICATION: SUSTAINED]
|
||||
|
||||
[VERDICT: PASS]
|
||||
@@ -0,0 +1,26 @@
|
||||
# Issue #5 Phase 2a Round 3 review
|
||||
|
||||
Job: d2e705a8
|
||||
Reviewer: planner-reviewer-codex-01 (planner,reviewer)
|
||||
Reviewed HEAD: 0e74aa1, focusing on changes after 3b134e3.
|
||||
|
||||
No blocking findings remain in the reviewed corrections. Previous R1 and R4 are resolved. No supplementary implementation edits or architectural escalation are required.
|
||||
|
||||
## Resolution
|
||||
|
||||
- **R1 — resolved:** policy now requires all four observed attribution fields plus a report manifest attempt before approval, then checks their correspondence. The approved synthetic fixture supplies collector_attribution separately from native Herdr fields. Deletion tests cover each identifier and the manifest. Independently repeated the prior all-evidence-removed mutation: result is now hold with attribution_ok=False. The complete fixture still returns candidate_approved.
|
||||
- **R4 — resolved:** update_logged_status holds a dedicated per-job audit lock across read/compare/write and uses a unique mkstemp file. The lock remains outside the registry transaction. Reviewed and ran the overlapping-flush regression: a competing flush is blocked while the first holds the lock, then completes with the newer seq2/completed state preserved. This addresses the actual interleaving rather than merely sequential call order.
|
||||
- Earlier missing-attempt and metadata-status bypass corrections remain intact. Operator-authority transitions and the rollback branch retain their documented roles.
|
||||
- The additional idempotent flag distinguishes an already-applied terminal ACK from a refused transition, so the operator CLI returns success for that no-op. Invalid status vocabulary is checked before either CAS or rollback writes. Relevant regression cases pass.
|
||||
- ShadowRunner remains separate from the transition engine; these changes do not introduce automatic completion commits. Durable notification recovery and production automatic completion remain Phase 2b work.
|
||||
|
||||
## Validation
|
||||
|
||||
- `./.venv/bin/python -m pytest tests/test_issue5_phase2a_loop.py tests/test_issue5_phase0_contracts.py -q`: **129 passed in 2.01s**.
|
||||
- Independent complete-fixture and missing-evidence probes: **candidate_approved** and **hold**, respectively.
|
||||
- `git diff --check 3b134e3..HEAD`: clean.
|
||||
- Reviewed policy, collector envelope fixture, audit critical section, engine/CLI correction diff, and new concurrency/deletion regressions.
|
||||
- Working tree was clean before the assigned report was written. No production or fixture files modified by the reviewer.
|
||||
- This reviewer did not rerun all 576 preexisting tests; the independent execution evidence here is the combined 129-test contract/Phase 2a suite. PASS is the review verdict, not a claim of an independently repeated full-suite run or unanimous approval from other reviewers.
|
||||
|
||||
[VERDICT: PASS]
|
||||
@@ -0,0 +1,103 @@
|
||||
# Issue #5 Phase 2a cross-review (`9389d7e`..`0e74aa1`, incl. Round 2 `cb575fa`)
|
||||
|
||||
- **Reviewer**: `reviewer-creator-grok-01`
|
||||
- **Job**: `3f0784c4`
|
||||
- **HEAD**: `0e74aa1` (`docs(issue-5): record the evidence requirement, the audit lock and the ack rule`) on `cb575fa` (`fix(issue-5): address phase 2a round 2 findings (F-C, F-D, R1, R4)`)
|
||||
- **Tests this turn**: `pytest tests/test_issue5_phase2a_loop.py tests/test_issue5_phase0_contracts.py tests/test_issue5_phase1_delegation.py tests/test_version_consistency.py` → **197 passed** (13.06s). Phase 2a file alone: **88 collected**. Collect-only **664**. Full 664 suite not re-run. `git diff --check 6ad6f08..HEAD` clean.
|
||||
|
||||
Round-2 findings were re-read in the live sources, not taken from the creator’s notes. Each of F-C, F-D, R1, R4 has a production path and a regression that fails on the pre-fix body.
|
||||
|
||||
---
|
||||
|
||||
## 1. Module split (`P2A-01` placement)
|
||||
|
||||
Plan said `lib_py/loop/engine.py`. Implementation is **`lib_py.delegation.engine`**. That is the freeze-boundary correction: a frozen loop binds `lib_py.delegation` to a different tree than the parent package, so delegation importing a sibling would mix trees. Phase 1’s AST guard would have failed the planned placement. `loop/__init__.py` states the engine lives with the writers. Acceptable plan deviation, not 유실 (Codex `.agents/reports/implementation_plan.md` and root NATS plan untouched).
|
||||
|
||||
`loop/` holds watcher / policy / shadow / store / attribution / evidence. `test_g9_loop_package_imports_only_delegation_among_siblings` holds.
|
||||
|
||||
---
|
||||
|
||||
## 2. CAS boundary (`P2A-01`)
|
||||
|
||||
`engine.transition` is the only status writer in CAS mode. `_decide` is pure. Lock covers load + decide + one `_atomic_write_record` + queue; `_flush` is **after** release (`engine.py:219-255`). `transition_seq` is incremented only on commit. `test_g5_no_blocking_io_while_the_registry_lock_is_held` spies subprocess/socket/sleep. `test_g6_every_record_writer_is_the_engine_or_a_declared_exception` allows only `register_job`, `next_seq`, and the two `_*_legacy` bodies.
|
||||
|
||||
`MAM_ENGINE_CAS=0` restores `_update_job_status_legacy` / `_pick_pending_legacy`. X-01..X-04 parametrize `"0"`/`"1"`.
|
||||
|
||||
`STATUS_TO_REQUEST` maps a merged `status=` onto the request that owns it, so a raw field cannot skip the table. Unknown requests are refused. `status` riding on `progress`/`merge` is refused (`test_r3_*`). `expected_attempt` with no record field fail-closes (`test_r2_*`).
|
||||
|
||||
Operator requests `redispatch` / `finalize` are gated on `OPERATOR_SOURCES = ("registry_cli", "operator")`. Redispatch must advance `iteration`. Wrapper `_operator_write` uses `_source="registry_cli"`. `publish_event` uses `_source="publish_event"` and cannot leave a terminal (`test_fa_*`).
|
||||
|
||||
---
|
||||
|
||||
## 3. Round 2 findings — independently confirmed
|
||||
|
||||
### F-C (blocking) — idempotent ACK is not a refusal
|
||||
|
||||
`TransitionResult.idempotent` is a distinct slot from `conflict`. Same-terminal re-receipt sets `idempotent=True`, `commit=False`, no second write (`engine.py:163-167`, `test_fc_engine_marks_an_idempotent_result`).
|
||||
|
||||
`_operator_write` (`registry.py:329-366`):
|
||||
- committed → rc 0
|
||||
- idempotent → rc 0 (the wrapper’s `status --set completed` on an already-completed record under `set -e`)
|
||||
- genuine conflict → stderr `refused: …`, rc 4
|
||||
|
||||
`test_fc_idempotent_finalize_is_success_not_refusal` covers both CAS modes. `test_fc_a_genuine_conflict_is_still_reported` keeps cancelled-after-completed at rc 4.
|
||||
|
||||
### F-D — vocabulary check is not skipped
|
||||
|
||||
`_operator_write` checks `VALID_STATUSES` **before** the CAS/legacy branch. Invalid status → rc 1, registry fingerprint unchanged, in both `MAM_ENGINE_CAS=0` and `1` (`test_fd_invalid_status_is_rejected_in_both_modes`). This is a usage error, not a CAS refusal (rc 4).
|
||||
|
||||
### R1 (blocking) — absence of evidence is not approval
|
||||
|
||||
`collect_observed_identity` (`policy.py:33-65`):
|
||||
- identifiers from the `collector_attribution` envelope beside the payload
|
||||
- native **herdr** payload identifiers are **not** collected
|
||||
- non-herdr (event/filesystem) payloads may name the job/attempt
|
||||
- every occurrence is kept (a later contradiction is not dropped)
|
||||
- `report_manifest.attempt_id` is collected separately
|
||||
|
||||
`decide` requires every `ATTRIBUTION_FIELD` to be present **and** agree, then requires a manifest attempt that matches dispatch (`policy.py:115-137`). `finalized_candidate.json` now carries the envelope. Tests: `test_r1_missing_attribution_evidence_holds` (all four fields), `test_r1_missing_report_manifest_holds`, `test_r1_identifiers_inside_a_native_payload_are_not_evidence`, plus the Round-1 contradiction tests.
|
||||
|
||||
### R4 — compare-and-write is one critical section
|
||||
|
||||
`update_logged_status` (`mqtt_common.py:580-617`):
|
||||
- per-job lock `.<name>.lock`, **outside** the registry transaction
|
||||
- read-compare-write under that lock; skip if `existing_seq >= incoming_seq`
|
||||
- `tempfile.mkstemp` per write (no shared `.tmp`)
|
||||
|
||||
`test_r4_overlapping_mirror_flushes_cannot_lose_the_newer_state` intercepts `Path.read_text` after the older flush has read, starts a seq=2 write, asserts the competitor **blocks**, and that `completed` seq=2 survives. `test_r4_each_mirror_write_uses_its_own_temp_file` asserts `mkstemp` in the function body and no leftover temps.
|
||||
|
||||
---
|
||||
|
||||
## 4. Shadow does not write (`P2A-03`)
|
||||
|
||||
`ShadowRunner` is not given `engine`. AST forbids `engine` / `update_job_status` / `transition(` in `shadow.py`. `test_g4_shadow_run_leaves_the_registry_byte_identical` hashes every file under the registry dir before/after `evaluate_all` of all 12 scenarios. Decisions go to `.mam/shadow/<job>.ndjson`. Attribution lives in `.mam/shadow/attribution/`, not the job record (Phase 1 schema freeze). `candidate_approved` is WOULD-COMPLETE log only. Auto-complete is still Phase 2b.
|
||||
|
||||
---
|
||||
|
||||
## 5. Watcher (`P2A-02`)
|
||||
|
||||
`herdr agent wait`: `--timeout` is milliseconds (`timeout_ms`); `--until` always explicit; exit 0 → stdout snapshot; exit 1 → **stderr** (`timeout` / `agent_not_found` healthy; other → `transport_error`). Timeouts do not become `Observation`. Backoff 1→2→4→8→16s; failure **drops** `cached_snapshot`. Tests never invoke a real herdr (`run=` injectable).
|
||||
|
||||
Default `until=("idle","done","blocked")` is intentional observation of `blocked` for `hold_blocked`. `interactive_ready` is captured but is **not** a completion gate (`policy.py` uses `agent_status in {idle,done}` only).
|
||||
|
||||
---
|
||||
|
||||
## 6. Policy vs fixtures (`P2A-04`)
|
||||
|
||||
12 scenarios (Phase 0 seven + `race_double_dispatch`, `attempt_reuse`, `partial_report`, `unknown_status`, `restart_midflight`) all match `expected_policy.decision` (`test_g7_*`). Debounce without manifest → `hold`. Missing execution anchor → `hold`. Policy is I/O-free (`test_policy_is_pure_no_io_no_clock`).
|
||||
|
||||
---
|
||||
|
||||
## Residuals (non-blocking)
|
||||
|
||||
1. **`pick_pending` CAS split lock** (`registry.py:128-144`): select under lock, then `transition` re-acquires. Concurrent pickers for the **same** session with two pending jobs can both select job A; the loser returns `None` even if job B is still pending (until the next call). Sequential two-job pick still works because the second call re-scans. Retry-on-failed-claim would close it. Not typical production (one picker per session). Safety (no double-claim) is preserved by `expected_status="pending"`.
|
||||
2. **`mqtt_common.py:401` docstring** still says ``lib_py.loop.engine``. The module is `lib_py.delegation.engine`.
|
||||
3. **`reject_stale` still reads `pane_process_generation` from the native herdr payload** (`policy.py:88-94`; fixtures `restarted_generation`, `race_double_dispatch`). After R1, that field is not evidence for *approval*, but the stale-rejection path still uses it. If a live collector puts generation only in `collector_attribution`, a stale generation becomes `hold` (missing/mismatch) rather than `reject_stale`. Both refuse completion. Vocabulary only.
|
||||
4. **`test_r1_identifiers_inside_a_native_payload_are_not_evidence`** plants only three of the four identifiers in the native payload (omits `pane_process_generation`). A regression that *did* count native ids would still hold on the missing fourth field. The production skip of herdr-payload ids is real; the test is weaker than its comment.
|
||||
5. Full **664**-test suite not re-run this turn (197 targeted including Phase 0/1). Brief’s “576” is the Phase 1 baseline; collection has grown with Phase 2a (88) plus Round-2 regressions.
|
||||
|
||||
No `[ESCALATE: PLANNER]`. Engine placement remains a freeze-rule correction. Round-2 fixes are surgical and tested.
|
||||
|
||||
---
|
||||
|
||||
[VERDICT: PASS]
|
||||
Reference in new issue
Block a user