|
| 1 | +# SOW-0039 - Assert Rust timeout ABI test layouts |
| 2 | + |
| 3 | +## Status |
| 4 | + |
| 5 | +Status: completed |
| 6 | + |
| 7 | +Sub-state: layout enforcement validated; three AI review threads explained and resolved; publishing with implementation. |
| 8 | + |
| 9 | +## Requirements |
| 10 | + |
| 11 | +### Purpose |
| 12 | +Address the three reviewed AI findings and resolve each with a fix or evidence. |
| 13 | + |
| 14 | +### User Request |
| 15 | +The user explicitly requests fixing code/docs or explaining incorrect findings and resolving all comments. |
| 16 | + |
| 17 | +### Assistant Understanding |
| 18 | +Facts: both ARM Rust ABI modes currently pass and print different sizes, but the runner does not assert them. The GNU riscv32 claim assumes a time64 alias absent from pinned libc. The Netdata private-field finding applies a Go collector skill to a Rust unit test; the public integration test retains normal spinning. |
| 19 | +Inferences: enforcing the existing diagnostic in the mode-owning runner closes the validation gap without changing shared fixtures or transports. |
| 20 | +Unknowns: no unresolved implementation decisions. |
| 21 | + |
| 22 | +### Acceptance Criteria |
| 23 | +- The runner checks pointer, tv_sec and timespec sizes for both intended modes. |
| 24 | +- Actual ARM execution passes on Rust MSRV and stable; wrong/missing layouts and test failures are rejected. |
| 25 | +- Every existing AI review thread receives an evidence-based reply and is resolved. |
| 26 | + |
| 27 | +## Analysis |
| 28 | + |
| 29 | +Sources checked: current/paused SOW-0015/0021/0027 and pending SOW-0031/0032/0035/0037; completed SOW-0036 and SOW-0038; empty local specs directory; project vendoring skill; docs/code-organization.md; docs/level1-posix-shm.md; Rust public fixture and ABI runner; C ABI runner and Go timeout construction. |
| 30 | +Current state: no active overlapping execution. This is extra validation, not a failed previously proven timeout repair, so no regression reopening is required. |
| 31 | +Risks: parsing a diagnostic introduces a small coupling; exact whole-line matching and negative cases make it explicit. |
| 32 | + |
| 33 | +## Pre-Implementation Gate |
| 34 | + |
| 35 | +Status: ready |
| 36 | + |
| 37 | +Problem / root-cause model: mode selection is not independently checked against compiled field sizes. Both runs could silently test one ABI if a dependency changes its configuration behavior. |
| 38 | +Evidence reviewed: public fixture prints pointer width, actual tv_sec size (labelled time_t), and timespec size; libc build.rs tracks the mode environment; real ARM runs show 4/4/8 and 4/8/16. |
| 39 | +Affected contracts and surfaces: standalone test runner and validation guidance only; no C/Rust/Go API, protocol or production transport changes. |
| 40 | +Existing patterns to reuse: low-priority helper, mktemp/EXIT cleanup, pipefail and the existing public-fixture diagnostic. |
| 41 | +Risk and blast radius: runner failure behavior only; preserve cargo exit status with pipefail and stream output with tee. |
| 42 | +Sensitive data handling plan: public code paths, commit IDs and sanitized test summaries only in SOWs, specs, docs, project skills, instructions and code comments; no credentials, identities, endpoints or raw private logs. |
| 43 | +Implementation plan: capture each invocation into one temporary log; require the exact expected ABI line per mode; explain asserted layouts in public docs and integrator guidance. |
| 44 | +Validation plan: execute ARM time32/time64 with MSRV 1.91.0 and stable; inject wrong, missing and failing command outputs using a temporary command harness; ShellCheck, bash syntax, diff check and SOW audit. |
| 45 | +Artifact impact plan: |
| 46 | +- AGENTS.md: no workflow change. |
| 47 | +- Runtime project skills: no vendoring occurs; source-owned runner only. |
| 48 | +- Specs: validation section of authoritative SHM doc changes, no duplicate local spec. |
| 49 | +- End-user/operator docs: explain layout enforcement. |
| 50 | +- End-user/operator skills: mention asserted compiled layout in integrator validation guidance. |
| 51 | +- SOW lifecycle: new validation-hardening SOW, completed with its implementation in one commit; existing work remains paused/pending. |
| 52 | +Open-source reference evidence: rust-lang/libc @ 42620ffc4109dc32e02f1cae9e63a3f4311b4b71, src/unix/linux_like/linux/gnu/b32/riscv32/mod.rs:667 and src/unix/linux_like/linux/musl/b32/riscv32/mod.rs:644 confirm distinct syscall constants. |
| 53 | +Open decisions: none; user authorized fixes, explanations and thread resolution. |
| 54 | + |
| 55 | +## Implications And Decisions |
| 56 | + |
| 57 | +Keep the shared fixture unchanged and assert its actual compiled diagnostic in the runner that owns mode selection. No downstream copy is required. The two false-positive findings receive source-linked explanations. |
| 58 | + |
| 59 | +## Plan |
| 60 | + |
| 61 | +1. Explain and resolve the two inaccurate findings. |
| 62 | +2. Implement, validate, document and commit the layout gate. |
| 63 | +3. Push, reply with the fix and resolve the remaining thread; refresh all selected comments. |
| 64 | + |
| 65 | +## Execution Log |
| 66 | + |
| 67 | +### 2026-09-27 |
| 68 | + |
| 69 | +- Verified both false positives and replied/resolved them individually using the Netdata project PR review workflow. |
| 70 | +- Implementation gate recorded before edits. Added runner layout enforcement and synchronized validation documentation. |
| 71 | +- Source pre-push CI: 33 success, 3 configured skips, 1 neutral, no failures/running checks. Netdata retains one pre-compilation ARM container exec-format infrastructure failure and three running checks; no new downstream changes. |
| 72 | + |
| 73 | +## Validation |
| 74 | + |
| 75 | +Acceptance criteria evidence: |
| 76 | +- Runner asserts the expected pointer/seconds/timespec representation independently of libc configuration. |
| 77 | +- All three original threads received substantive replies and resolveReviewThread returned true individually: plugin-ipc PR17 discussions 4115703293 and 4115703296; Netdata PR24049 discussion 4115730229. |
| 78 | + |
| 79 | +Tests or equivalent validation: |
| 80 | +- Actual ARM/QEMU runner passes time32 (4/4/8 bytes) and time64 (4/8/16) under Rust 1.91.0 and stable 1.98.1. Installed the missing MSRV ARM standard library before rerunning. |
| 81 | +- Temporary negative command harness: old runner accepts duplicated legacy ABI; new runner rejects it and missing ABI output with exit 1; preserves Cargo exit 7; correct modes pass and temporary logs are removed in every case. |
| 82 | +- ShellCheck, bash syntax and git diff --check pass. SOW audit run before commit. |
| 83 | + |
| 84 | +Real-use evidence: |
| 85 | +- Production Rust SHM receives in the public fixture wait at least 100 ms and 1100 ms on both actual emulated ABIs and wake on delayed messages for finite/infinite/max budgets. |
| 86 | + |
| 87 | +Reviewer findings: |
| 88 | +- Qodo ABI assertion request implemented in the mode-owning runner, avoiding changes to the shared fixture. |
| 89 | +- Qodo GNU riscv32 claim rejected against pinned upstream constants: claimed time64 alias exists only for musl, which the branch handles. No full GNU riscv32 runtime support is asserted. |
| 90 | +- Qodo spin-test claim rejected: normal public construction is already covered, while the unit test deliberately isolates blocking; cited Go collector skill is outside Rust transport scope. |
| 91 | +- Direct self-review of the runner/docs working diff is sufficient: no production behavior changes or material unresolved interactions; negative tests cover failure propagation, stale output and cleanup. No repeated external review is needed for this scoped safeguard. |
| 92 | + |
| 93 | +Same-failure scan: |
| 94 | +- Searched ABI diagnostics/mode switches in tests, Rust integration tests and Runtime Safety CI. This is the single Rust mode-owning runner. C time64 CI already uses NIPC_TEST_REQUIRE_TIME64_32 static assertions; no matching missing check in the selected scope. |
| 95 | + |
| 96 | +Sensitive data gate: |
| 97 | +- Durable changes contain public paths, ABI sizes and sanitized validation evidence only; no credentials, identities, private endpoints or incident logs. |
| 98 | + |
| 99 | +Artifact maintenance gate: |
| 100 | +- AGENTS.md: unchanged because responsibilities/workflow are unchanged. |
| 101 | +- Runtime project skills: unchanged; no Netdata vendoring or new integration procedure. |
| 102 | +- Specs: authoritative docs/level1-posix-shm.md validation section updated; empty local specs directory needs no duplicate of public guidance. |
| 103 | +- End-user/operator docs: updated the expected ABI and failure behavior. |
| 104 | +- End-user/operator skills: docs/netipc-integrator-skill.md now requires both layout and behavior checks. |
| 105 | +- SOW lifecycle: completed file moved to done and committed with its runner/docs changes; paused/pending SOWs untouched. |
| 106 | + |
| 107 | +Specs update: validation contract clarified in public SHM documentation; runtime protocol unchanged. |
| 108 | +Project skills update: vendoring gate unaffected because runner/docs changes do not alter Netdata's vendored sources. |
| 109 | +End-user/operator docs update: expected representations and rejection behavior documented. |
| 110 | +End-user/operator skills update: compiled-layout verification added to integration checks. |
| 111 | +Lessons: do not infer ABI coverage solely from a configuration toggle; verify the actual compiled representation. |
| 112 | +Follow-up mapping: layout check implemented; two inaccurate findings rejected with evidence; independent 32-bit build coverage remains tracked in SOW-0037. No new deferred implementation. |
| 113 | + |
| 114 | +## Outcome |
| 115 | + |
| 116 | +Rust cross-ABI validation now fails when the selected layout was not actually compiled. All three original AI findings have been addressed and resolved; remote CI on the new commit is not claimed here. |
| 117 | + |
| 118 | +## Lessons Extracted |
| 119 | + |
| 120 | +Cross-ABI test configuration must be checked against the representation actually compiled. |
| 121 | + |
| 122 | +## Followup |
| 123 | + |
| 124 | +No additional source transport work is included. Existing independent 32-bit build gaps remain in SOW-0037. |
| 125 | + |
| 126 | +## Regression Log |
| 127 | + |
| 128 | +No regression of a previously proven timeout result; this adds an explicit validation safeguard. |
| 129 | + |
0 commit comments