fix(test): Edge refresh 검증을 안정화한다

중복 Start cleanup과 짧은 register timeout이 clean release qualification을 비결정적으로 막지 않도록 fake-node handshake 계약을 통일한다.
This commit is contained in:
toki 2026-08-12 14:42:34 +09:00
parent 39fa1da55b
commit ba47290950
5 changed files with 302 additions and 41 deletions

View file

@ -1,44 +1,43 @@
<!-- task=m-iop-one-shot-agent-model-comparison/08+07_comparison_rerun plan=0 tag=TEST milestone-task=claude-standalone,gemini-standalone,gpt-standalone,gemini-hybrid,gpt-hybrid -->
<!-- task=m-iop-one-shot-agent-model-comparison/08+07_comparison_rerun plan=1 tag=REVIEW_TEST milestone-task=claude-standalone,gemini-standalone,gpt-standalone,gemini-hybrid,gpt-hybrid -->
# Code Review Reference - TEST
> **[IMPLEMENTING AGENT — READ FIRST]** Start only after child 07 PASS. Fill implementation evidence, leave active files in place, and report ready for review. Finalization is review-only.
# Code Review Reference - REVIEW_TEST
## Overview
date=2026-08-12
task=m-iop-one-shot-agent-model-comparison/08+07_comparison_rerun, plan=0, tag=TEST
task=m-iop-one-shot-agent-model-comparison/08+07_comparison_rerun, plan=1, tag=REVIEW_TEST
## Archive Evidence Snapshot
- Preserve `run-20260812T044800Z-412e05fc80df` and `run-20260812T050756Z-18293db0b83f`; do not reuse them.
- Dependency: archived PASS `07+06_caller_write_contracts/complete.log`.
- Prior logs: `plan_cloud_G09_0.log`, `code_review_cloud_G10_0.log`; verdict FAIL/R1.
- Old preflight-only id: `run-20260812T053517Z-6b4f4de67128`; never reuse.
## Implementation Item Completion
| Item | Status |
|---|---|
| TEST-1 runtime recovery | [ ] |
| TEST-2 ready=9 preflight | [ ] |
| TEST-3 one scored run | [ ] |
| REVIEW_TEST-1 bootstrap test fix | [ ] |
| REVIEW_TEST-2 release qualification/deploy | [ ] |
| REVIEW_TEST-3 ready preflight | [ ] |
| REVIEW_TEST-4 scored run | [ ] |
## Implementation Checklist
- [ ] [TEST-1] Prove child 07 PASS, inspect and restore the existing managed dev runtime without changing source.
- [ ] [TEST-2] Run a fresh public CLI preflight and record its emitted id with ready=9.
- [ ] [TEST-3] Run exactly one fresh public CLI scored matrix and record its run id and nine-cell terminal/web results.
- [ ] [REVIEW_TEST-1] Apply R1 test-only lifecycle/timeout fix and pass high-count local/remote regressions.
- [ ] [REVIEW_TEST-2] Complete clean release tests, four-binary rebuild/redeploy, and 4-node healthy verification.
- [ ] [REVIEW_TEST-3] Produce a fresh public ready=9 preflight id.
- [ ] [REVIEW_TEST-4] Execute exactly one public scored run and record nine-cell terminal/workspace results.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
## Review-Only Checklist
- [ ] Append verdict/routing signals after fresh verification.
- [ ] Verify no direct caller/provider invocation or scored retry occurred.
- [ ] Archive active files and write/move `complete.log` only on PASS.
- [ ] Preserve milestone-task metadata.
- [ ] Append final verdict and routing signals.
- [ ] Freshly verify R1, release identity, ready=9, one run, and web gates.
- [ ] Archive/finalize only after all requirements pass.
## Deviations from Plan
_Record actual deviations or state none._
_Record actual deviations or none._
## Key Design Decisions
@ -46,28 +45,29 @@ _Record actual decisions._
## Reviewer Checkpoints
- Runtime recovery used the existing managed deployment and did not expose secrets.
- Preflight is ready=9 before run.
- Exactly one new scored run id exists, with 9 success and all web gates passing.
- Test cleanup never invokes a second Runtime Start.
- All fake register calls share the established handshake timeout.
- Deployed source is clean release HEAD; four Nodes are connected/healthy.
- Fresh preflight is ready=9; exactly one fresh scored run succeeds 9/9.
## Verification Results
### Dependency and runtime recovery
### R1 high-count tests
_Paste exact commands/output with secret values omitted by command construction._
_Paste exact output._
### Release qualification and deployment
_Paste exact output/identity summary._
### Public preflight
Command: `python3 scripts/agent_comparison_benchmark.py preflight --manifest scripts/fixtures/agent-comparison-benchmark-iop-one-shot.json`
_Paste actual stdout/stderr and exit status._
_Paste exact output and id._
### Public scored run
Command: `python3 scripts/agent_comparison_benchmark.py run --manifest scripts/fixtures/agent-comparison-benchmark-iop-one-shot.json`
_Paste actual stdout/stderr and exit status._
_Paste exact output, id, status, and artifact gate summary._
## Section Ownership
Implementation status/checklist/evidence is implementation-owned. Review checklist and verdict are review-only.
Implementation status/checklist/evidence is implementation-owned. Review checklist/verdict is review-only.

View file

@ -0,0 +1,151 @@
<!-- task=m-iop-one-shot-agent-model-comparison/08+07_comparison_rerun plan=1 tag=REVIEW_TEST milestone-task=claude-standalone,gemini-standalone,gpt-standalone,gemini-hybrid,gpt-hybrid -->
# Bootstrap qualification fix and comparison rerun
## For the Implementing Agent
Implement the selected R1 fix exactly, run every verification, then resume the clean release deployment, ready=9 preflight, and exactly one scored run. Fill `CODE_REVIEW-cloud-G10.md`, keep active files in place, and do not create `complete.log` or archive files.
## Background
The first live-run plan correctly stopped twice: a preflight found the initial clean release lacked the feature, then the feature-bearing clean release exposed two failure modes in one bootstrap test. The reviewer closed both to stale test-only timeout/cleanup code; production runtime behavior is not changed.
## Archive Evidence Snapshot
- `plan_cloud_G09_0.log` / `code_review_cloud_G10_0.log`: FAIL with R1 after no scored run.
- Preflight-only evidence `run-20260812T053517Z-6b4f4de67128`: all cells `model_missing` on pre-feature clean release; preserve, do not reuse.
- Release branch `release/dev-936` now contains clean feature merge `b244b6e224a9d44c09a1e7af49c0788f597c89fe`.
## Finding Resolution Map
| ID | Evidence | Root Cause | Selected Fix | Mode | Changed precondition | Acceptance |
|---|---|---|---|---|---|---|
| R1 | remote full suite register timeout; package run duplicate-Start unexpectedly succeeded | obsolete 2s register bound plus cleanup duplicate Start cancels active lifetime | use `fakeNodeHandshakeTimeout` for all fake register calls and cleanup with one Stop | direct-fix | test no longer mutates lifecycle during cleanup and uses the established remote-safe bound | exact test x20, package x10, full suite, rebuild/deploy, ready=9, one scored run |
## Analysis
### Files Read
- `apps/edge/internal/bootstrap/runtime_refresh_node_test.go`
- `apps/edge/internal/bootstrap/runtime.go`
- `apps/edge/internal/transport/server.go`
- `apps/edge/internal/transport/integration_test.go`
- `agent-task/m-iop-one-shot-agent-model-comparison/08+07_comparison_rerun/code_review_cloud_G10_0.log`
- `agent-roadmap/sdd/knowledge-tool-optimization-extension/iop-one-shot-agent-model-comparison/SDD.md`
### SDD Criteria
SDD remains READY; milestone-task ids and S04-S08 are unchanged. The live evidence gate remains ready=9 followed by nine successful terminal/workspace results.
### Verification Context
Remote runner is `toki@toki-labs.com:/Users/toki/agent-work/iop-dev`, release `dev-936`. No scored run has started. Four Nodes were restored connected before the new release test failed. Token files remain process-local and values must not be printed.
### Test Coverage Gaps
R1 is covered by high-count exact/package runs. External endpoint and workspace output remain covered only by the resumed public preflight/run.
### Symbol References
None; no production or public symbol changes.
### Split Judgment
Keep the fix and resumed deployment in this follow-up because the fixed qualification is the ordered precondition for the same all-cell execution invariant. Child 07 remains satisfied by its archived `complete.log`; no new sibling dependency is introduced.
### Scope Rationale
Exclude production Runtime behavior, routes, model aliases, task content, scoring, and roadmap mutation. Only the broken test lifecycle/timeout, evidence pointer files, and active review may change.
### Final Routing
- evaluation_mode=`isolated-reassessment`; finalizer=`finalize-task-policy.sh pair`
- build closures all true; scores `2/2/2/2/2` => G10; route `grade-boundary`; `PLAN-cloud-G10.md`
- review closures all true; scores `2/2/2/2/2` => G10; route `official-review`; `CODE_REVIEW-cloud-G10.md`
- large_indivisible_context=false; risks=`temporal_state,boundary_contract,variant_product` (3); review_rework_count=1; evidence_integrity_failure=false; capability gap absent.
## Implementation Checklist
- [ ] [REVIEW_TEST-1] Apply R1 test-only lifecycle/timeout fix and pass high-count local/remote regressions.
- [ ] [REVIEW_TEST-2] Complete clean release tests, four-binary rebuild/redeploy, and 4-node healthy verification.
- [ ] [REVIEW_TEST-3] Produce a fresh public ready=9 preflight id.
- [ ] [REVIEW_TEST-4] Execute exactly one public scored run and record nine-cell terminal/workspace results.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
### [REVIEW_TEST-1] Close bootstrap test nondeterminism
**Problem:** `runtime_refresh_node_test.go:517-524` restarts Runtime in cleanup, and register requests in the file retain a shorter timeout than the declared fake-node handshake contract.
**Solution:** Replace the duplicate Start cleanup with the standard Stop-only form used by neighboring tests. Replace all four fake register `2*time.Second` bounds with `fakeNodeHandshakeTimeout`; do not add retries or production changes.
**Modified Files and Checklist:**
- [ ] `apps/edge/internal/bootstrap/runtime_refresh_node_test.go`: exact R1 test-only changes.
**Test Strategy:** Fresh high-count exact and package tests prove both observed modes are absent.
**Verification:** `go test -count=20 -run '^TestRefreshConfigApplyNoChangeSkipsNodePush$' ./apps/edge/internal/bootstrap` and `go test -count=10 ./apps/edge/internal/bootstrap` exit 0.
### [REVIEW_TEST-2] Release qualification and deployment
**Problem:** The current deployed d40e release is clean but lacks the benchmark feature; b244 source has not passed qualification or been deployed.
**Solution:** Commit/push R1, merge it into `release/dev-936`, sync the runner, run full pre/post-build suites, rebuild four binaries, config check/dry-run, deploy Edge and all four Nodes, then require connected/healthy state.
**Modified Files and Checklist:**
- [ ] `agent-task/m-iop-one-shot-agent-model-comparison/08+07_comparison_rerun/CODE_REVIEW-cloud-G10.md`: exact release/deployment evidence.
**Test Strategy:** Apply dev-runtime deploy contract; any failed required stage stops scored execution.
**Verification:** clean source/build identity matches release HEAD and CP reports 4 connected Nodes with healthy providers.
### [REVIEW_TEST-3] Fresh public preflight
**Problem:** The old preflight belongs to the pre-feature release.
**Solution:** With protected environment values, call public CLI preflight and store only emitted id.
**Modified Files and Checklist:**
- [ ] `agent-task/m-iop-one-shot-agent-model-comparison/08+07_comparison_rerun/preflight_id.log`: exact id.
**Test Strategy:** Require `ready=9 registration_required=0 implementation_gap=0`.
**Verification:** public preflight exits 0.
### [REVIEW_TEST-4] One scored run
**Problem:** No valid generated benchmark output exists.
**Solution:** Invoke public run exactly once after ready=9, store id, and inspect immutable evidence.
**Modified Files and Checklist:**
- [ ] `agent-task/m-iop-one-shot-agent-model-comparison/08+07_comparison_rerun/run_id.log`: exact id.
**Test Strategy:** Require 9 success and all web artifact gates PASS; do not retry on failure.
**Verification:** public run exits 0 with required counts.
## Dependencies and Execution Order
Child 07 PASS remains satisfied. Execute REVIEW_TEST-1 through REVIEW_TEST-4 strictly in order.
## Modified Files Summary
| File | Item |
|---|---|
| `apps/edge/internal/bootstrap/runtime_refresh_node_test.go` | REVIEW_TEST-1 |
| `agent-task/m-iop-one-shot-agent-model-comparison/08+07_comparison_rerun/CODE_REVIEW-cloud-G10.md` | REVIEW_TEST-2, REVIEW_TEST-3, REVIEW_TEST-4 |
| `agent-task/m-iop-one-shot-agent-model-comparison/08+07_comparison_rerun/preflight_id.log` | REVIEW_TEST-3 |
| `agent-task/m-iop-one-shot-agent-model-comparison/08+07_comparison_rerun/run_id.log` | REVIEW_TEST-4 |
## Final Verification
1. High-count commands in REVIEW_TEST-1 exit 0.
2. Full dev-runtime pre/post suites and build identity checks pass.
3. Public preflight exits 0 with ready=9.
4. Exactly one public run exits 0 with 9 success and all artifact gates.
After completing all code changes, fill implementation-owned sections in `CODE_REVIEW-*-G??.md`.

View file

@ -0,0 +1,115 @@
<!-- task=m-iop-one-shot-agent-model-comparison/08+07_comparison_rerun plan=0 tag=TEST milestone-task=claude-standalone,gemini-standalone,gpt-standalone,gemini-hybrid,gpt-hybrid -->
# Code Review Reference - TEST
> **[IMPLEMENTING AGENT — READ FIRST]** Start only after child 07 PASS. Fill implementation evidence, leave active files in place, and report ready for review. Finalization is review-only.
## Overview
date=2026-08-12
task=m-iop-one-shot-agent-model-comparison/08+07_comparison_rerun, plan=0, tag=TEST
## Archive Evidence Snapshot
- Preserve `run-20260812T044800Z-412e05fc80df` and `run-20260812T050756Z-18293db0b83f`; do not reuse them.
- Dependency: archived PASS `07+06_caller_write_contracts/complete.log`.
## Implementation Item Completion
| Item | Status |
|---|---|
| TEST-1 runtime recovery | [ ] |
| TEST-2 ready=9 preflight | [ ] |
| TEST-3 one scored run | [ ] |
## Implementation Checklist
- [ ] [TEST-1] Prove child 07 PASS, inspect and restore the existing managed dev runtime without changing source.
- [ ] [TEST-2] Run a fresh public CLI preflight and record its emitted id with ready=9.
- [ ] [TEST-3] Run exactly one fresh public CLI scored matrix and record its run id and nine-cell terminal/web results.
- [ ] Fill implementation-owned sections in CODE_REVIEW-*-G??.md with actual implementation notes and verification output.
## Review-Only Checklist
- [ ] Append verdict/routing signals after fresh verification.
- [ ] Verify no direct caller/provider invocation or scored retry occurred.
- [ ] Archive active files and write/move `complete.log` only on PASS.
- [ ] Preserve milestone-task metadata.
## Deviations from Plan
Clean release source was required before a scored run. The first rebuilt release lacked the feature source and public preflight `run-20260812T053517Z-6b4f4de67128` correctly stopped with 9 `model_missing` cells. After committing/merging the feature into `release/dev-936`, the mandatory pre-build suite exposed a new deterministic test defect, so no scored run was started.
## Key Design Decisions
Preserve both preflight-only runs. Do not bypass the failed pre-build suite or deploy `+dirty` source. Repair the exact bootstrap test lifecycle/timeout defect, rerun the full release qualification, then resume preflight and the single scored run.
## Reviewer Checkpoints
- Runtime recovery used the existing managed deployment and did not expose secrets.
- Preflight is ready=9 before run.
- Exactly one new scored run id exists, with 9 success and all web gates passing.
## Verification Results
### Dependency and runtime recovery
```text
release source before merge: d40e4494e767e3fec7796c670f58130a9a194e80
first rebuilt Edge source: d40e4494e767 (clean)
ports: 18082/18083/18084/19093/19101 listening
Control Plane: 4/4 nodes connected; 8 providers healthy/available; in_flight=0 queued=0
fresh release b244b6e224a9d44c09a1e7af49c0788f597c89fe pre-build test:
--- FAIL: TestRefreshConfigApplyNoChangeSkipsNodePush (2.02s)
runtime_refresh_node_test.go:575: register: request timeout for nonce 1
isolated probes:
3 x exact test: PASS
full bootstrap package:
--- FAIL: TestRefreshConfigApplyNoChangeSkipsNodePush (0.00s)
runtime_refresh_node_test.go:518: expected already started error
```
### Public preflight
Command: `python3 scripts/agent_comparison_benchmark.py preflight --manifest scripts/fixtures/agent-comparison-benchmark-iop-one-shot.json`
```text
error: preflight blocked run_id=run-20260812T053517Z-6b4f4de67128 status=registration_required ready=0 registration_required=9 implementation_gap=0
```
The immutable preflight JSON classifies every cell as `model_missing`. This came from the clean d40e release not yet containing the feature; it is not accepted as ready evidence.
### Public scored run
Command: `python3 scripts/agent_comparison_benchmark.py run --manifest scripts/fixtures/agent-comparison-benchmark-iop-one-shot.json`
Not run. The mandatory clean-release pre-build suite failed before the scored-run gate.
## Code Review Result
- Verdict: FAIL
- Required: 1
- Suggested: 0
- Nit: 0
- Dimension Assessment:
- Correctness: Fail — bootstrap qualification is nondeterministic under its own test lifecycle.
- Completeness: Fail — ready=9 and scored execution are not yet produced.
- Test coverage: Fail — the intended handshake timeout constant is not used by register calls and cleanup asserts an unsupported duplicate Start lifecycle.
- API contract: Pass — public preflight stopped without substitution.
- Code quality: Warn — one test contains a destructive duplicate Start in cleanup.
- Plan deviation: Pass — the new source defect was preserved and routed instead of bypassed.
- Verification trust: Pass — both distinct failure modes were reproduced on the declared remote release runner.
- Findings:
- R1 Required
- Evidence: `apps/edge/internal/bootstrap/runtime_refresh_node_test.go:517-524` calls `Start` again in cleanup; `Runtime.newLifetimeContext` cancels the live lifetime before the second transport bind. The same test's registration at `:569-575` still uses `2*time.Second` while `fakeNodeHandshakeTimeout=5*time.Second` exists for this remote/race stall class. Remote full suite produced register timeout; isolated package produced duplicate-Start success.
- Root Cause: stale test-only lifecycle and timeout code; the duplicate Start can cancel/rebind the active runtime, and the register handshake retains the obsolete short bound.
- Selected Fix: replace the cleanup with one `rt.Stop()` call and replace all fake-node register `2*time.Second` bounds in this file with `fakeNodeHandshakeTimeout`. Add no production lifecycle change. Accept with exact test `-count=20`, package `-count=10`, full dev-runtime suite, then rebuild/redeploy and resume ready=9 plus one scored run.
- Disposition: direct-fix.
- Routing Signals: review_rework_count=1, evidence_integrity_failure=false.
- Next Step: materialize REVIEW_TEST follow-up and continue automatically.
## Section Ownership
Implementation status/checklist/evidence is implementation-owned. Review checklist and verdict are review-only.

View file

@ -183,7 +183,7 @@ nodes:
regResp, err := toki.SendRequestTyped[*iop.RegisterRequest, *iop.RegisterResponse](
&fakeNode.Communicator,
&iop.RegisterRequest{Token: "tok-refresh"},
2*time.Second,
fakeNodeHandshakeTimeout,
)
if err != nil {
t.Fatalf("register: %v", err)
@ -402,7 +402,7 @@ nodes:
regResp, err := toki.SendRequestTyped[*iop.RegisterRequest, *iop.RegisterResponse](
&fakeNode.Communicator,
&iop.RegisterRequest{Token: "tok-conn"},
2*time.Second,
fakeNodeHandshakeTimeout,
)
if err != nil {
t.Fatalf("register: %v", err)
@ -514,13 +514,8 @@ nodes:
t.Fatalf("Start: %v", err)
}
defer func() {
if err := rt.Start(context.Background()); err == nil {
t.Fatal("expected already started error")
} else {
// runtime is running, stop it
if err := rt.Stop(); err != nil {
t.Fatalf("Stop: %v", err)
}
if err := rt.Stop(); err != nil {
t.Fatalf("Stop: %v", err)
}
}()
@ -569,7 +564,7 @@ nodes:
regResp, err := toki.SendRequestTyped[*iop.RegisterRequest, *iop.RegisterResponse](
&fakeNode.Communicator,
&iop.RegisterRequest{Token: "tok-noc-change"},
2*time.Second,
fakeNodeHandshakeTimeout,
)
if err != nil {
t.Fatalf("register: %v", err)
@ -878,7 +873,7 @@ nodes:
regResp, err := toki.SendRequestTyped[*iop.RegisterRequest, *iop.RegisterResponse](
&fakeNode.Communicator,
&iop.RegisterRequest{Token: "tok-rt"},
2*time.Second,
fakeNodeHandshakeTimeout,
)
if err != nil {
t.Fatalf("register: %v", err)