Production persistence, restart durability, CI efficiency and repository-truth convergence - #29
Merged
quantdale merged 14 commits intoAug 26, 2026
Conversation
…mium caching - rust.yml: replace per-run 'cargo install cargo-audit --version 0.22.1 --locked' (minutes of dependency-tree compilation every Windows run) with the official prebuilt release via taiki-e/install-action, pinned to a full commit SHA with tool version still exactly cargo-audit@0.22.1. Audit gate semantics unchanged; installation failure still fails the job visibly. - e2e.yml + simulation.yml: cache ms-playwright Chromium keyed on the package-lock hash; the explicit install step remains as the cold-miss fallback so a lockfile bump repopulates the correct browser build.
Session bootstrap re-enumerated the OS for data it already held: hardware::detect() built a fresh sysinfo System (~2.1ms) and Disks (~0.7ms) per call, twice per launch, on top of CollectorState::new() already holding both - 4 redundant System + 1 redundant Disks enumerations per session start AND per supervisor recovery. - New CpuIdentity (vendor + brand) extracted once from the already- refreshed System; detect_with_cpu() lets every caller that holds a refreshed System or prior profile skip the re-probe. - disk_infos_from() projects an already-enumerated Disks list; state.rs builds its degraded-startup profile from held state only. - Regression pins: CpuIdentity brand semantics match the removed legacy detection exactly; disk_infos_from projection round-trips. - examples/startup_probe.rs: headless session-bootstrap timing probe (mechanism medians: System refresh 2069us, Disks 646us; profile discovery phase 4ms -> <1ms; total bootstrap dominated by the required ~200ms rate-baseline priming, honestly unchanged). - build_snapshot tick path: per-250ms HashMap<String,String> vendor cache under the history lock -> zero-allocation (&str,&'static str) linear scan (GPU counts are single digits). - Release profile: lto=true -> "thin" after measuring the warm final-crate phase CI re-pays per build: fat 4m45s vs thin+cu=1 3m15s (-32%), exe 9.12MB -> 9.42MB; full table in Cargo.toml.
…ontract - MetricChart wrapped in React.memo with hoisted identity-stable yDomain constants (MetricCard DEFAULT_Y_DOMAIN, renderCardContent Y_DOMAIN_AUTO): the card tree rebuilds every ~250ms scalar tick, but chart props are all primitive or identity-stable now. Measured on the Vite mock harness (7 charts, 12s window): chart-body renders 686 -> 182 (-73%), long-task main-thread time 3.57s -> ~1.3s (-62%); live scalars and 1 Hz growth unchanged. Guard test MetricChart.test.tsx pins the memo contract and commit-skipping semantics. - mockBackend: duplicated Nvidia-telemetry literal (x2) consolidated into nvidiaStatsFor. - journeys: duplicated recovery-probe install (x2) -> installRecoveryProbe. - useMetrics hook test: duplicated beforeEach reset -> freshIpcMock. - Removed dead code: drawSessionLength + unused persona field sessionLengthSecs (+ fixture), shouldQuarantine (policy stays documented on FLAKE_BUDGET), assertChartGrowth helper. - sim journey fix found by validation runs: customization-roundtrip failed deterministically for seeds whose RNG rolls a misdrag at the reorder step. dragCard models seeded mis-drags that cancel; the journey asserted "reorder applied" unconditionally, so any misdraw became a seed-stable gate failure under CI's random seeds. dragCard now returns 'applied' | 'cancelled' and the journey asserts the matching postcondition while still round-tripping persistence from the actual resulting order.
…; record adopted remediation/perf campaign - Record the 2026-08-26 deep-audit remediation pass and performance campaign in progress.md Completed (schema doc 4->5 fix, probe path, consolidations, dead-code removal, CI efficiency, measured UI/bootstrap/ LTO wins, misdrag journey fix). - AGENTS.md: startup_probe command documented. - .cursorrules: schema 5 + lifecycle-1 pointer; examples/ probe paths; thin-LTO note pointing at the measured table. - Open the new OpenSpec change: proposal/design/tasks + deltas for user-simulation-platform (real-lane sidebar relaunch certification), ci-pipeline-efficiency-and-coverage (audit install path, browser caching), project-documentation-accuracy (automation-capability truth, schema-version consistency).
…ix destructive sidebar persistence merge Workstream B — sidebar-relaunch-persistence (real lane only): drives the built exe over CDP; records rendered [data-sb-id] order after native discovery settles, keyboard-drags two items, asserts the write lands in the run-isolated real settings.json, then kills and RESPAWNS the process and asserts: first-process exit evidence, fresh CDP port, in-memory history rebuilt empty, native supervisor re-bootstrapped (schema_version=1), store non-destruction across the boundary, order-preserving rendered restore, coherent unrelated settings, and advancing live metrics. SortableSidebarCard exposes data-sb-id as the stable semantic seam (pinned by a HardwareSidebar test). Workstream C — restart-soak-durability (real lane only, 3 bounded cycles): rotating window/viewMode mutations persist -> clean shutdown -> relaunch -> restore-from-disk + UI match + native status + metric advance each cycle; strict JSON validity of the isolated store asserted mid-soak. Selected only on packaged/dispatch lanes via SIM_JOURNEYS. Production fix found BY the journey on real hardware: discovery varies between processes (GPU Engine counters materialize lazily; disk source differs pre/post WMI). The dashboard deliberately keeps ghost ids in cardOrder, but the sidebar auto-save effect persisted the discovery- filtered subset — permanently rewriting the user's arrangement. persistSidebarCardOrder now mirrors the dashboard's non-destructive merge for the STORE while rendering keeps filtering to present devices. Platform hardening: - Runner enforces an orphan-process guard on every real-lane run; shared assertNoOrphanProcesses helper reused by verify:packaged. - RealAppDriver exposes cdpPort/appExePath so journeys can prove a genuine second process (reused port would mean no new target). - New engine steps: readSidebarIds, waitForSidebarCards, invokeCollectorStatus, invokeHistoryPointCount, readRealStoreFile — all resolving the CURRENT driver page (pre-restart bindings die with the first process). - dragSidebarCard split so journeys can reorder without toggling the sidebar closed. Truth convergence (Workstream A): AGENTS.md three-lane capability statement replaces the stale 'WebView2 can't be automated' claim; progress.md reflects post-PR #28 state; .cursorrules gains the simulation-platform pointer (backlog item); exploratory register names the certifying journey and keeps the free-roam pointer caveat honestly.
…ecks against discovery churn - handleDragEnd extracted to mergeDraggedSidebarOrder: dragging while a device is transiently absent appends saved-but-unrendered ids instead of shrinking the store (unit-pinned x2). - Journey persisted-sidebar-order now polls store-vs-LIVE-DOM consistency instead of comparing a one-shot DOM snapshot: real-hardware WMI/PDH enrichment can append newly discovered ids milliseconds after the drag, which raced the previous equality check (observed in run logs). - Post-relaunch subset check uses the settled saved order reference.
Three diagnostic real-lane runs surfaced and fixed three journey defects: - store-vs-DOM consistency now polls until the drag write lands under ghost-aware semantics (store may be a superset; restricted-to-rendered must equal rendered exactly); - post-relaunch emptiness is sampled IMMEDIATELY at live with the old process's kill-time depth as reference (old=4 -> new@live=0 observed); - status freshness claim corrected: timestamp_ms is wall-clock epoch, not session age — generation/schema/state carry the re-bootstrap proof. New waitForSidebarSettled step requires two identical consecutive renders so WMI/PDH enrichment appends cannot race order assertions. Local real-lane evidence: sidebar-relaunch-persistence PASS 16/16; restart-soak-durability PASS 25/25 (3 mutate/persist/shutdown/relaunch cycles) against src-tauri/target/release/sys-monitor-tauri.exe.
verify:full exit 0; verify:packaged PASS; full sim:real matrix green (sidebar-relaunch-persistence 16/16, restart-soak-durability 25/25); verify:version consistent at 0.1.4; openspec strict 17/17; git diff --check clean. CI cost table filled from hosted log of run 32925221386; hosted-run cells to be completed after push.
…econciliation Sidebar + soak journeys marked done with local evidence; dual-GPU physical limitation and discovery-variance behavior restated; pointer drag kept registered.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b9e88f828
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # codegen-units stays 1: cu=16 saves another ~50s per build but grows the | ||
| # shipped binary another ~800KB with no runtime benefit — flip only if release | ||
| # build minutes ever outweigh artifact size for this app. | ||
| lto = "thin" |
There was a problem hiding this comment.
Release builds now omit the previous strip = true, so production executables retain symbols and the size comparison attributed solely to thin LTO also changes a second build variable. This directly contradicts the updated .cursorrules release profile and the surrounding artifact-size rationale; restore stripping or explicitly reconcile the documented profile and remeasure the comparison.
AGENTS.md reference: AGENTS.md:L5-L7
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in 30d3dba: strip = true restored to [profile.release]; .cursorrules again matches source verbatim. The measured size column in the Cargo.toml table predates stripping, so shipped artifacts are now SMALLER than the documented numbers while the wall-time comparison (the part CI re-pays) is unaffected - an honest note was added next to the table.
Comment on lines
+104
to
+109
| median( | ||
| &mut state_costs | ||
| .iter() | ||
| .zip(profile_costs.iter()) | ||
| .map(|(a, b)| a + b) | ||
| .collect::<Vec<_>>() |
There was a problem hiding this comment.
Calling median above sorts state_costs and profile_costs independently, so this zip pairs phase timings by rank rather than by the iteration in which they occurred. On runs where the phases vary inversely, the reported TOTAL is the sum of the two phase medians rather than the median end-to-end startup cost, which can hide or invent regressions in the new startup probe; record each iteration's combined duration before either vector is sorted.
AGENTS.md reference: AGENTS.md:L43-L45
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in 30d3dba: per-iteration totals are now captured into a separate vector BEFORE median() sorts either phase vector, so TOTAL reports the median of true end-to-end iteration costs rather than a rank-paired artifact.
Comment on lines
+729
to
+733
| const orderPreserved = restoredDom.every((id) => { | ||
| const at = refOrder.indexOf(id, cursor); | ||
| if (at < 0) return false; | ||
| cursor = at + 1; | ||
| return true; |
There was a problem hiding this comment.
When the second discovery pass omits any saved GPU or disk, restoredDom.every(...) accepts any order-preserving subset, so the journey passes even though this change's delta spec requires the post-relaunch [data-sb-id] order to exactly equal the persisted pre-restart order. The real lane therefore does not certify the contract it introduces; either wait for and require the same discovered ID set, or amend the OpenSpec requirement to explicitly define the subset semantics.
AGENTS.md reference: AGENTS.md:L84-L92
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Spec amended in 30d3dba (openspec delta user-simulation-platform): the restore contract now explicitly defines three branches - append-only store non-destruction, order-preserving rendered subset under discovery variance, and EXACT equality when the relaunch discovers the same device set. The journey now asserts exact equality on that branch too (locally exercised on a stripped release build: sidebar-relaunch-persistence PASS 17/17); strictness is not weakened - it is defined where variance is real hardware behavior, evidenced in evidence.md sections 3 and 9.
Hosted packaged-lane run 32952589280 surfaced two issues:
1. first-launch-onboarding died at bring-up on the FIRST launch of a
cold hosted runner ('Target page, context or browser has been
closed' between CDP readiness and page attach; no app stderr; all
later launches fine). RealAppDriver.launch now retries once with a
fresh process/port before failing — genuine spawn defects still fail.
2. On the hosted hardware the relaunch discovered MORE devices than the
first process (extra ephemeral disks) and the sidebar effect appended
them to the saved order, tripping exact-equality. The honest
non-destruction invariant is append-only: every pre-restart id must
survive with relative order intact; new discoveries may append.
restored-store-non-destructive now asserts exactly that.
Local re-verification: vitest 248 green; first-launch-onboarding real
lane PASS 15/15.
…; surface teardown diagnostics Hosted run 32954583100 proved the retry path end-to-end but exposed its elevated-host gap: WebView2 >=150 on elevated runners takes the debug switches from the machine-wide HKLM policy written before the FIRST spawn, so respawning with a new env port left the policy pointing at the stale endpoint — attempt 2 polled a port nobody opened (60s timeout, 'fetch failed'). The retry now rewrites this.env with the new port and re-applies applyHklmArgsFallback() before respawning. Also: runner teardown diagnostics were collapsed to a constant string when they failed an otherwise-passing journey (sidebar 16/16 then opaque 'harness cleanup/isolation diagnostics failed'); the joined diagnostic text now lands in the failure message. Locally green: first-launch-onboarding + sidebar-relaunch-persistence + restart-soak-durability real lanes.
…tore-contract spec) All three P2 findings verified against source and fixed: 1. Cargo.toml release profile lost 'strip = true' in the adopted thin-LTO change while .cursorrules still documents it — restored, with an honest note that the measured size column predates stripping (shipped artifacts are smaller) and the wall-time comparison is unaffected. 2. startup_probe TOTAL median zipped state/profile costs AFTER median() had sorted each vector independently — pairing by rank instead of by iteration, misstating end-to-end startup cost. Totals are now captured before any sorting. 3. The sidebar delta spec said 'restored exactly' while the journey accepted order-preserving subsets under legitimate discovery variance. Spec now defines all three branches explicitly (append-only store non-destruction; order-preserving rendered subset; EXACT equality when discovery sets match), and the journey asserts exact equality whenever the relaunch discovers the same device set (locally exercised: PASS 17/17 on a stripped release build).
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| root.render(React.createElement(HardwareSidebar, { open: true, profile: p, memTotalGb: null })); | ||
| }); | ||
|
|
||
| const ids = Array.from(container.querySelectorAll('[data-sb-id]')).map((n) => n.getAttribute('data-sb-id')); |
There was a problem hiding this comment.
🔥 The Roast: getAttribute returns string | null, but you're feeding it straight into an expect().toEqual(string[]) comparison. With strict: true in tsconfig, tsc --noEmit will reject this faster than a GPUs-who-shall-not-be-named rejection.
🩹 The Fix: Filter out the nulls or assert non-null:
Suggested change
| const ids = Array.from(container.querySelectorAll('[data-sb-id]')).map((n) => n.getAttribute('data-sb-id')); | |
| const ids = Array.from(container.querySelectorAll('[data-sb-id]')).map((n) => n.getAttribute('data-sb-id')!).filter((x): x is string => x !== null); |
📏 Severity: warning
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| unobserve(): void {} | ||
| disconnect(): void {} | ||
| } | ||
| const originalRO = globalThis.ResizeObserver; |
There was a problem hiding this comment.
🔥 The Roast: You stubbed globalThis.ResizeObserver and only unpatched it inside the test body's finally. afterEach is empty for globals. One more it in this file and you've got cross-test pollution — like leaving a burner on because "the current recipe finished fine".
🩹 The Fix: Move save/restore into beforeEach/afterEach so any future test inherits a clean global:
Suggested change
| const originalRO = globalThis.ResizeObserver; | |
| let originalRO: typeof ResizeObserver | undefined; | |
| beforeEach(() => { originalRO = globalThis.ResizeObserver; }); | |
| afterEach(() => { globalThis.ResizeObserver = originalRO; }); |
📏 Severity: nitpick
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review Roast 🔥Verdict: No Issues Found | Recommendation: Merge Oh wait, this PR is actually clean. I had my flamethrower warmed up and everything. 📊 Overall: Like finding a unicorn in production — I didn't think clean PRs existed anymore, but here we are. Files Reviewed (12 files)
Previous Review Summaries (2 snapshots, latest commit a72a8d0)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit a72a8d0)Verdict: No Issues Found | Recommendation: Merge Oh wait, this PR is actually clean. I need to sit down. I had my flamethrower warmed up and everything. 📊 Overall: Like finding a unicorn in production — I didn't think clean PRs existed anymore, but here we are. Files Reviewed (10 files)
Previous review (commit 1b90b8d)Verdict: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)
🏆 Best part: The Rust backend is clean as a whistle — 💀 Worst part: The TypeScript type error in 📊 Overall: Like a first pancake — the shape is almost right but there's a TypeScript type error and a test-hygiene nit holding it back from the plate. Files Reviewed (39 files)
Reviewed by free · Input: 494.9K · Output: 46.6K · Cached: 2.2M |
…RO stub restore - HardwareSidebar test: filter getAttribute nulls with a type guard instead of feeding (string|null)[] into toEqual. - MetricChart guard: ResizeObserver stub save/restore moved from the test body's finally into beforeEach/afterEach so future tests inherit a clean global regardless of assertion failures. Vitest 248 green; tsc clean. Test-only delta: packaged-app and installer inputs unchanged, so the in-flight packaged/release dispatch evidence at 30d3dba remains valid for the shipped artifact.
…ning - campaign completion report CAMPAIGN REPORT (session report; PR #29) Starting SHA: 718e503 (origin/main, post-PR #28) Final code SHA: a72a8d0 (branch tip at hosted-green verification) Archive head: this commit (docs/spec/archive only) ROOT CAUSES DISCOVERED 1. Destructive sidebar persistence (production bug, found by the new real-lane journey on real hardware): hardware discovery legitimately varies between processes (Windows materializes GPU Engine counters lazily: 3 GPUs vs 0 on consecutive launches of this workstation; disk enumeration source differs pre/post WMI enrichment). The dashboard keeps ghost ids in cardOrder by design, but the sidebar auto-save effect persisted the discovery-filtered subset, permanently rewriting the user's arrangement. Fixed via persistSidebarCardOrder + mergeDraggedSidebarOrder (non-destructive store, ghost-preserving drags); unit-pinned x7. 2. Cold-start CDP bring-up flake on hosted runners (first launch lost its WebView2 target between endpoint readiness and page attach). RealAppDriver launch now retries once with a fresh process/port AND rewrites the HKLM debug-policy with the new port (elevated hosts take switches from the machine-wide policy, not env) - the retry itself was proven and corrected across three hosted runs before going green. 3. cargo-audit compiled from source on every Windows CI run: measured 5m14.5s install step inside a 10m56s Rust job (~48%) from hosted logs. 4. Repository truth drift: AGENTS.md claimed WebView2 cannot be automated while shipping that automation; progress.md described the completed, merged PR #28 as in progress. IMPLEMENTATION CHANGES - Workstream B: sidebar-relaunch-persistence journey (real lane): data-sb-id semantic ids; settled-discovery waits; keyboard reorder; isolated-store write check; true kill+respawn with distinct-port/exit/empty-history/ supervisor-rebootstrap proof; append-only non-destruction + order-preserving restore + exact equality when discovery matches; advancing metrics; runner-wide orphan guard + developer-store self-test. - Workstream C: restart-soak-durability (bounded 3-cycle soak, packaged/ dispatch lanes) after coverage-overlap evaluation. - Workstream A: AGENTS.md three-lane statement; progress.md headers; .cursorrules sim-platform pointer; exploratory register reconciliation; schema docs verified against constants (5/5, 1/1). - Workstream D: pinned prebuilt taiki-e/install-action (full-SHA) for cargo-audit@0.22.1 (adopted work verified, evidence recorded); Playwright Chromium caching keyed on lockfile hash. - Workstream E: focused audits of settings/persistence, driver, lifecycle, CI supply chain (43/43 full-SHA action pins); findings above fixed. REGRESSION TESTS ADDED MetricChart memo contract; HardwareSidebar data-sb-id order pin; persistSidebarCardOrder x3; mergeDraggedSidebarOrder x2 (248 total vitest); two real-lane journeys (17/17 + 25/25 locally, both green hosted). CI CHANGES / EFFICIENCY rust.yml audit install ~3.3s (was 5m14.5s); whole Rust job 10m56s -> 6m11s (-43%) at identical audit semantics; e2e/simulation Chromium caching. Audit remains mandatory; action failure fails the job before the gate. SECURITY IMPLICATIONS No unsigned binaries; immutable full-commit-SHA action pins throughout; machine-wide HKLM debug policy written pre-spawn and unconditionally removed on every close path (aggregated failures); sim isolation (SYSMON_SIM_APP_DATA absolute-path store override) re-verified; no gate weakened. REAL-LANE PERSISTENCE RESULT Hosted windows-latest run 32959735876 @30d3dba: all six journeys PASS - sidebar-relaunch-persistence 17/17 (exact-equality branch exercised), restart-soak-durability 25/25; first-launch/customization green. LOCAL VALIDATION (final heads) verify:full exit 0; verify:packaged PASS; full sim:real matrix PASS; verify:version consistent (0.1.4); openspec strict green; git diff --check clean; cargo fmt/clippy -D warnings clean. HOSTED VALIDATION Final-head PR checks ALL PASS (rust/frontend/exe/e2e/mock-sim/lint/Kilo/Snyk, runs 32960383402/.328/.341); packaged dispatch 32959735876 success; release qualification 32959741283 success (MSI+NSIS install/run/uninstall, hashed manifest). OPENSPEC STATE Change created -> tasks evidenced -> deltas synced to main specs -> strict validation green (16 specs) -> archived as 2026-08-26-production-persistence-and-operational-hardening. REMAINING LIMITATIONS (truthful) - Dual identical-GPU runtime mapping physically unqualified (no qualifying hardware here; deterministic fixtures cover identity logic). - Physical hotplug/lid/power remain registered exploratory-only. - Free-roam pointer-drag on the real lane stays registered (keyboard drag is the certified interaction). - Backend discovery may differ between sessions (documented behavior; the sidebar now tolerates it without data loss). - actions/download-artifact emits a Node-20 deprecation warning on Node 24 runners (pre-existing, warning-only; future maintenance). PR/MERGE RESULT: PR #29 merged after every required final-head check was green and all review threads were answered with fixes + evidence.
quantdale
deleted the
agent/monitorers-production-persistence-ci-hardening
branch
August 27, 2026 08:08
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Post-feature hardening campaign (OpenSpec: \production-persistence-and-operational-hardening):
estart-soak-durability, 3 cycles, packaged/dispatch lane): mutate/persist/shutdown/relaunch/verify with strict JSON validity and per-cycle native status checks.
Validation
Local final head: verify:full pass, verify:packaged pass, full sim:real matrix pass (sidebar 16/16, soak 25/25), verify:version consistent at 0.1.4, openspec strict 17/17, git diff --check clean.
Test plan