docs(admin-config-tool): close task 6.3, mark spec implementation-complete

Task 6.3's own "before deciding" gate (re-run the flaky primary-push
E2E 20-25x standalone with real stderr capture) was already satisfied
by bug-1485's 2026-08-01 follow-up: 40/40 clean runs, zero captured
crash evidence, after replacing the DEVNULL stderr redirect with real
per-node log capture. Decision reached there (mergeable as-is, bug-1485
downgraded to low-priority/environment-flake) was never reflected back
into tasks.md/spec.json. Marks 6.3 and its parent task-group 6 [x], and
reconciles spec.json's stale "tasks-generated" phase (21/21 tasks were
already complete) to "implementation-complete" with a validate_impl
block recording the known limitations (bug-1485 stays open/low-priority,
single-node ackedCount==0 exit-1 behavior, no CI live-E2E run yet).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-08-16 11:13:37 +00:00
parent c6f20ba553
commit 570e464dda
Signed by: rshen
SSH key fingerprint: SHA256:8Vf2pZ3h6rJa7EhGK+WaAQ9Mta/apZvteW6g9U/WKKk
2 changed files with 15 additions and 5 deletions

View file

@ -1,9 +1,9 @@
{
"feature_name": "admin-config-tool",
"created_at": "2026-07-06T01:30:00Z",
"updated_at": "2026-07-30T02:01:00Z",
"updated_at": "2026-08-16T00:00:00Z",
"language": "en",
"phase": "tasks-generated",
"phase": "implementation-complete",
"approvals": {
"requirements": {
"generated": true,
@ -30,5 +30,15 @@
"implementation": 23,
"validation": 24
}
},
"validate_impl": {
"decision": "GO",
"validated_at": "2026-08-16T00:00:00Z",
"note": "Corrected stale phase metadata (was 'tasks-generated' though 20/21 tasks were already [x] complete). Task 6.3's remaining blocker -- bug-1485's ~24/25 flaky primary-push failure -- was already followed up (2026-08-01): DEVNULL stderr redirect replaced with real per-node log capture, test re-run 40x standalone with zero reproductions and zero captured crash evidence. Decision reached: mergeable as-is, bug-1485 downgraded to low-priority/environment-flake rather than a blocker. Task 6.3 now marked [x] complete.",
"known_limitations": [
"bug-1485: primary-push E2E test (test_e2e_live_ha_push.py) had a ~24/25 flaky failure rate in one investigation round (elected primary observed <defunct> shortly after promotion) that could not be reproduced in 40 clean standalone runs; left OPEN as a low-priority environment-flake, not closed, since absence of reproduction here is not proof of absence. Real stderr capture is now permanently in place to diagnose any recurrence.",
"6.2's own finding stands: a genuinely single-node config push always exits 1 (ackedCount == 0, no standbys to ACK) even on full server-side success -- spec-consistent per cli.py/design.md's exit-0-iff-acked_count>0 postcondition, not a regression, but a future task may want a distinct single-node success signal.",
"push_config()'s TLS/redirect/framing code has had live E2E coverage only against the single-node (6.2) and 3-node (6.3) topologies exercised in this spec's own tests -- no production/CI live run yet."
]
}
}

View file

@ -141,7 +141,7 @@
- _Boundary: xceadmin.push, xceadmin.discovery_
- _Depends: 3.3_
- [ ] 6. Fix reply envelope decode and add live E2E push coverage
- [x] 6. Fix reply envelope decode and add live E2E push coverage
- [x] 6.1 Unwrap the `Request` reply envelope before decoding as `RequestReply`
- Live-verified 2026-07-30 against a real deployed single-node controller: `config push` crashes every time with `capnp.lib.capnp.KjException: ... non-list pointer where text was expected`. Root cause: `xce_reply()` (`src/lib/request.c`) wraps every reply — including `config push`'s — in an outer `Request{opcode: OPCode_reply, body: union{replymsg @4: RequestReply}}` envelope (`src/schema/controller.capnp:645-651`) before writing it to the wire. Two call sites decode the raw wire bytes directly as a bare `RequestReply`, skipping the outer envelope, which misaligns every pointer offset: `xceadmin.discovery._redirect_target()` (crashes first, on every reply) and `xceadmin.push.push_config()`'s own `mod.RequestReply.from_bytes(reply_bytes)` (same latent bug, currently unreached only because discovery crashes first — fixing discovery alone is not sufficient). The controller side is not at fault: server logs confirm the same request's dispatch, commit, and activation all succeed for the exact bytes that then crash the client on decode.
- Both decode sites unwrap `Request.body.replymsg` before treating the bytes as a `RequestReply` — introduce one shared decode helper (e.g. in `xceadmin.discovery`, which `xceadmin.push` already imports from) rather than duplicating the unwrap logic at each call site
@ -159,7 +159,7 @@
- _Boundary: xceadmin (test-only, cross-cutting with the built controller binary)_
- _Depends: 6.1_
- [ ] 6.3 Add a live 3-node HA E2E test for `config push` (success path + redirect)
- [x] 6.3 Add a live 3-node HA E2E test for `config push` (success path + redirect)
- Task 6.2 proved a single-node deployment always returns `ackedCount == 0` and exits 1 even on full server-side success (no standbys exist to ACK) — the `acked_count > 0` exit-0 success path, and Requirement 3's redirect-following, have never been exercised against a real controller, only against mocks (task 5.6) or a topology structurally unable to reach them (task 6.2). This task closes both gaps in one live 3-node deployment.
- Deploys a real 3-node HA `xce_controller` cluster (reuse `deploy-ha.sh`'s existing 3-node pattern, e.g. `localhost:19991/19992/19993`, staggered start for clean election per this repo's existing convention), waits for a primary to be elected
- Pushing to the primary directly: real `xceadmin config push` exits 0, reports `ackedCount > 0` (a real positive count from real standby ACKs, not the single-node `0` case), and the controller logs confirm commit/activate on the primary and replication/ack on at least one standby
@ -168,7 +168,7 @@
- _Requirements: 2.1, 2.5, 2.6, 3.1, 3.2, 3.3_
- _Boundary: xceadmin (test-only, cross-cutting with the built controller binary)_
- _Depends: 6.2_
- _Blocked: standby-redirect half is DONE and correct (round-1 reviewer independently re-verified: OP_upgradeConfig has no server-side redirect wiring today, matches design.md's own pre-existing "Out of Boundary" gap; test correctly locks in real current rejection behavior). Primary-push success-path test is flaky (~24/25 live fails in round-2, see bug-1485) — elected primary observed as a `<defunct>` zombie shortly after promotion, but a dedicated debug round could NOT reproduce this in 26+ clean live runs on an idle host and could not capture any crash evidence (stderr was DEVNULL'd in the original failing runs). Leading theory: round-2's failures were environment contamination (stale venv, concurrent build/test load) rather than a deterministic product bug, but this is unconfirmed either way. Uncommitted work (test_e2e_live_ha_push.py, _live_e2e_common.py, test_e2e_live_push.py refactor) is left in the worktree, not discarded — re-run the primary-push test 20-25x standalone on an otherwise-idle host with real stderr capture (not DEVNULL) before deciding whether it's mergeable as-is, needs the settle-window-to-real-poll fix noted in bug-1485, or points to a real controller-side race requiring a separate task._
- _Closed: standby-redirect half is DONE and correct (round-1 reviewer independently re-verified: OP_upgradeConfig has no server-side redirect wiring today, matches design.md's own pre-existing "Out of Boundary" gap; test correctly locks in real current rejection behavior). Primary-push success-path test was flaky (~24/25 live fails in round-2, see bug-1485) — elected primary observed as a `<defunct>` zombie shortly after promotion. Per this note's own next step, `test_e2e_live_ha_push.py`'s DEVNULL stderr redirect was replaced with real per-node log capture (commit `a5560256`) and the test re-run 40 times standalone (25 idle-sandbox + 15 with a concurrent competing build): 40/40 PASSED, 0/120 captured stderr logs non-empty — no crash, no ASAN report, in any run. Could not reproduce the original failure rate under matching conditions; leading hypothesis is that the original ~24/25 rate was itself caused by OTHER concurrent sandbox sessions contending for resources during that investigation, not a deterministic controller bug reachable in isolation. Decision: **mergeable as-is** — bug-1485 stays open, downgraded to low-priority/environment-flake (absence of reproduction here is not proof of absence), with the now-permanent stderr-capture infrastructure ready to catch a real ASAN report or signal if it recurs in a genuinely contended sandbox or in CI._
## Implementation Notes