feat: complete, harden, and package native review workflows - #5972
feat: complete, harden, and package native review workflows#5972jedwards27 wants to merge 32 commits into
Conversation
Signed-off-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz>
Signed-off-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz>
Signed-off-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz>
…urneys * origin/main: fix(desktop): hide the offcanvas-collapsed sidebar so it stops painting over the community rail (block#5947) Polish mobile message threads and composer (block#5645) chore(release): release Buzz Desktop version 0.5.14 (block#5917) ci(release): remove desktop smoke gate (block#5914) Signed-off-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz>
Signed-off-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz>
Signed-off-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz>
Signed-off-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz>
Signed-off-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz> Signed-off-by: Jude Edwards <judeedwards@squareup.com>
Signed-off-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz>
Signed-off-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz>
d856366 to
682a6f0
Compare
Fail closed on non-standard fixture ports, use fixed loopback database coordinates, scrub repository-controlled subprocess environments, and remove generated review keys when fixture seeding fails. Co-authored-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Apply absolute maximum budgets to the worst candidate sample while retaining median-based relative regression comparisons. Cover the catastrophic-outlier case directly. Co-authored-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Create a unique run-owned simulator, scrub credentials from iOS review subprocesses, and delete only the owned device during cleanup. Co-authored-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Default harness commands to a credential-free environment and isolate cleanup HOME without reconstructing the host environment. Co-authored-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Signed-off-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz> (cherry picked from commit 3635687)
Signed-off-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz>
themiguelamador
left a comment
There was a problem hiding this comment.
Blocking findings:
- The documented native-review command/test path imports PyYAML, but the dependency is neither installed nor pinned; it fails from a clean checkout even after Hermit activation.
- The evidence redactor leaks common secret forms (
Authorization: Bearer …, JSON keys such as"token": "…") and copies receipt failure/cleanup strings without recursive redaction, so a bundle described as privacy-safe can disclose credentials. - The AVAssetWriter capture loop does not advance its frame clock while the writer is backpressured. Its sleep target then remains in the past and the driver busy-spins, perturbing the performance run it is measuring.
scrollaccepts a target locator but posts at the mouse's previous position, so journeys can scroll the wrong view.- iOS receipts omit the common schema fields (
flow, timestamps, isolation, steps, measurements, performance) and add an undeclareddevice, making them invalid against the checked-in receipt schema and producing incomplete publication provenance. - Journey/performance/media inputs accept malformed durations, duplicate samples, booleans, and non-finite numbers; some fail late, while duplicate receipts can satisfy the minimum cohort size without independent runs.
Fixed in Complear/buzz@b3562d00e (branch review/pr-5972-fix): pinned uv/PyYAML environment, comprehensive evidence redaction, backpressure-safe capture scheduling, locator-targeted scrolls, schema-compatible iOS receipts, and fail-closed input validation with regression coverage. Verification: native-review 50/50, Swift release build, desktop 4,954/4,954 plus lint/typecheck, mobile 1,417/1,417 plus analysis, shell/JSON/Python syntax checks.
* origin/main: (43 commits) perf(desktop): parallelize relay agent directory rebuild (block#6258) Refine the mobile emoji picker (block#5853) fix(desktop): exclude archived agents from nest, order regeneration (block#5905) Add font size and conversation density preferences (block#5644) fix(desktop): emit camelCase config-write payload fields (block#6062) fix(desktop): downscale large avatars for agent-share PNG body (block#6260) fix(desktop): preserve early relay auth challenges (block#3320) Polish mobile message actions (block#5873) Refine mobile pairing confirmation (block#6018) chore(scripts): add buzz-adopt-prod-agents.sh (block#6250) feat(managed-agents): close five Claude Code agent-config gaps (block#4557) chore(hooks): keep mobile analysis out of pre-commit (block#6236) fix(shared-ui): delay hover disclosures by default (block#5821) fix(desktop-chrome): preserve balanced layout when sidebar collapses (block#6000) Polish mobile timeline navigation (block#5874) chore(release): release Buzz Desktop version 0.5.17 (block#6234) fix(prompt): simplify pickup follow-through (block#6186) fix(mcp): scope todo usage (block#6216) fix(desktop): bound remote agent mention authorization (block#6224) fix: bump h2 for RUSTSEC-2026-0258 (block#6222) ... Signed-off-by: Princess Donut <3cb959c7eb65d61f634e61df318e450f18f82fa0e01849e7010b82666ead0587@buzz.block.builderlab.xyz> # Conflicts: # desktop/src/main.tsx # mobile/ios/Podfile.lock
Signed-off-by: Princess Donut <3cb959c7eb65d61f634e61df318e450f18f82fa0e01849e7010b82666ead0587@buzz.block.builderlab.xyz>
Signed-off-by: Princess Donut <3cb959c7eb65d61f634e61df318e450f18f82fa0e01849e7010b82666ead0587@buzz.block.builderlab.xyz>
|
Implemented the requested hardening in
Exact-head validation was performed independently twice on
GitHub currently reports the PR as mergeable at this SHA. DCO passes; other repository checks/review requirements remain pending. Any head change invalidates the exact-head evidence above. Please re-review |
|
Reviewing exact head
Exact-head evidence: Python native-review suite 50/50 passed; Swift release build passed; Python/JSON/shell syntax gates passed; source inspection covered the remediation areas. Fresh empty-cache launcher provisioning was inconclusive because the configured internal mirror failed TLS and the public-PyPI control was blocked by dependency-confusion policy, so I am not attributing that environmental failure to this PR. No exact-head native Desktop/iOS journey receipt was available. Live GitHub state immediately before this review was Verdict: changes remain required. GitHub does not permit this PR author's account to submit a second formal request-changes review on its own PR; this comment records the fresh exact-head team review, while the existing request-changes review remains open. |
|
:bot: Jude’s code review agent — changes remain required on exact head Reviewed
Exact-head checks otherwise passed: native-review Python 50/50, Swift release build, all production journey validations, Python/JSON/shell checks, clean tree, and matching live/local OIDs. Fresh-cache provisioning could not be re-established in this environment because the configured Block mirror failed TLS; that is recorded as unverified rather than a separate finding. No GUI/native journey was launched under the shared-machine safety rule. GitHub remains |
Redact complete credential-header values, enforce journey schema bounds at runtime, and add mutation-sensitive Swift coverage for capture scheduling and targeted scroll placement. Co-authored-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Co-authored-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Treat indented and structured-log Authorization values as complete credential fields instead of leaving the credential after an auth scheme. Co-authored-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Redact the remainder of any log line containing an Authorization header so structured and prefixed logs cannot leak Digest, Negotiate, custom, or future credential schemes. Co-authored-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
|
All six blocking findings have been addressed, and the repaired branch is now at exact head Remediation summary:
Fresh exact-head evidence at
Independent adversarial review also returned CLEAR at this exact SHA for both lanes: Swift/native-runtime integration and mutation sensitivity; and redaction/security plus runtime/schema behavior. No live GUI AVAssetWriter/CGEvent journey was run, so the native evidence here is release compilation plus causal unit/mutation coverage rather than a claim of end-to-end GUI execution. Please re-review the current head. |
|
:bot: Jude’s code review agent Verdict: changes remain required. Reviewed current PR base
Integrated exact-head validation:
No GUI was launched under the shared-machine safety rule, so this round does not claim live AVAssetWriter/CGEvent end-to-end evidence. That residual risk does not explain or mitigate the deterministic privacy and validation failures. Please fix both and request another exact-head review; any push invalidates this verdict. GitHub does not permit the PR author's account to submit a formal request-changes review on its own PR, so this comment records the integrated fresh review while the existing change request remains open. |
Redact complete Authorization assignment and folded-header values, and reject unsupported or duplicate keyboard modifiers before native actuation. Co-authored-by: Carl <5f365698229751c0461f57bb03a4e93134e6e936bd7039ebe7b737282a43c754@buzz.block.builderlab.xyz> Signed-off-by: Jude Edwards <judeedwards@squareup.com>
|
Addressed both remaining review findings at exact head
Exact-head validation at
Two fresh independent exact-head checks returned CLEAR. The original failing redaction probes and adversarial neighbors now fully redact while preserving the next unindented line; modifier probes accept empty/all-supported unique forms and reject duplicate/unknown/non-string forms before Swift execution. No GUI was launched for this validation-only delta. Please re-review the current head; any further push invalidates this evidence. |
|
@themiguelamador the requested fixes are published at |
|
:bot: Jude’s code review agent — fresh exact-head review Verdict: REQUEST CHANGES (submitted as a comment because the authenticated account is the PR author and GitHub forbids self-review) Blocking finding[P2] Redact complete credential-shaped values; the evidence bundle currently leaks their tails.
These strings reach shared evidence through both Integrated review evidenceFresh independent source/privacy and native/Swift lanes were completed at this exact head. The privacy lane found the blocker above; the native/Swift lane found no additional material contract, packaging, cleanup, or release-path defect. I independently reproduced the blocker and verified both lanes' exact-head receipts were created after the reviewed commit. At exact clean HEAD
Behavior/contracts traced: journey schema/runtime validation → Python driver protocol → Swift locator/action/AX/CGEvent/recording path; subprocess environment isolation; Desktop state/semantic probe; iOS run-owned simulator lifecycle; receipt/finding-bundle/video publication; cleanup aggregation; SwiftPM/release boundaries. Manual/native evidence: no GUI or simulator journey was launched in this fresh round because shared-machine policy requires explicit opt-in. Source contracts, schema/runtime checks, unit tests, and release compilation were exercised; live visual/lifecycle proof remains residual risk. Please fix the blocker and request a new exact-head review. Previous clearance does not carry forward. |
Signed-off-by: Jude Edwards <judeedwards@squareup.com>
|
Addressed the fresh review blocker at exact head
Added exact regression coverage for:
Exact clean-head validation at
No GUI or simulator was launched for this two-file redaction-only change. Please re-review the new exact head; prior clearance is intentionally invalidated. |
|
:bot: Jude’s code review agent — fresh exact-head review Verdict: REQUEST CHANGES (recorded as a comment because GitHub forbids the authenticated PR-author account from self-reviewing) Blocking finding[P2] Escaped quote boundaries still leak credential suffixes from structured logs. The quoted alternatives in These are realistic JSON/Python-like structured log strings. Both recursively copied receipt fields and focused log excerpts pass arbitrary strings through this path ( Integrated exact-head evidenceTwo fresh independent lanes were completed and reconciled. The integration lane was otherwise clear across evidence-bundle → receipt → publisher contracts, schemas, Swift, and release compilation. The privacy lane found the blocker above; I independently reproduced all three escaped-quote leaks and opened both fresh exact-head evidence logs. At clean exact head
Behavior/contracts traced: focused-log selection and redaction; recursive receipt redaction; evidence manifest; publisher inputs; journey/schema validation; Swift/native build boundary. Manual/native evidence: no GUI or simulator was launched under the shared-machine opt-in policy. This deterministic source-level privacy failure is independently reproduced and does not depend on native execution. Please fix the escaped-quote boundary and request a new exact-head review. Previous clearance is void. |
Signed-off-by: Jude Edwards <judeedwards@squareup.com>
|
Addressed the remaining redaction issue in Quoted credential values now consume escaped characters before finding their closing delimiter, so escaped quotes (including multiple preceding backslashes and Python-style single quotes) cannot leave a secret suffix in the published excerpt. Regression coverage verifies complete redaction while preserving the following safe field/line. Exact-head validation on
The branch and PR head were verified at that SHA after push. Ready for re-review. |
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: COMMENT — code review clear; exact-head Desktop E2E execution still required before merge clearance
Reviewed: bbd20fae75ecc3bd7a83cc12a65379fac22a2b79..036e0535f46e77ba29cb5c35924d1fa1c7d0b93f (live PR head reverified)
Risk: critical privacy boundary. Finding bundles publish log/receipt material, so incomplete credential redaction can disclose secrets.
Behavior/contracts traced: the new escape-aware quoted-value matcher at tools/native-review/evidence_bundle.py:19-23 through redact_log, focused_log, recursive redact_value, and finding_bundle; regression rows at tools/native-review/tests/test_evidence_bundle.py:133-140; full native-review journey/schema and Swift driver boundaries.
Findings: no unresolved code finding. Independent adversarial probes covered valid JSON values with 0–8 backslashes before embedded quotes, escaped Python-style single quotes, malformed/truncated values, folded continuations, focused excerpts, recursive receipt strings/secret keys, and end-to-end bundle generation. Every planted secret was removed while safe neighboring fields/lines remained. Reverting the escape-aware matcher made all four new regression rows fail, then restoring it returned green.
Exact-head validation: clean SHA fences around all local runs.
- native-review Python package: 54/54
- all three production Desktop journeys: runtime loader + Draft 2020-12 schema validation
- Swift package: 3/3
- Swift production build
- Python compile, schema metaschema checks, launcher shell syntax, and
git diff --check - exact-head CI: 14 successful jobs plus DCO, including Desktop Core/build, Mobile, unit/lint/security, Windows/Linux builds, relay/backend E2E, and Desktop Release Candidate
CI blocker / residual risk: run https://github.com/block/buzz/actions/runs/32303054986 completed cancelled. Both Desktop integration shards were cancelled during pnpm exec playwright install-deps chromium before build/relay/seed/tests ran; smoke shards 1/3/4 were cancelled in provisioning and shard 2 was cancelled shortly after entering the smoke step. Aggregate Desktop jobs failed only because those shards did not succeed. This is provisioning/timeout behavior, not a failed assertion, and the PR does not alter the workflow; nevertheless, those exact-head E2E suites are not proven. Re-run this SHA until all smoke/integration shards execute and pass before treating the PR as merge-clear.
Manual/native evidence: no GUI or simulator was launched under the shared-machine safety policy. This source-level redaction fix does not require visual proof; no broader native lifecycle claim is made.
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent — REQUEST CHANGES
The Desktop side of the integrated stack works at this head, but the iOS rig does not launch from a clean checkout.
Blocking — the committed CocoaPods lockfile is not canonical, so flutter drive aborts before the app launches. mobile/ios/Podfile.lock:23,56,87,116 contains four blank lines introduced while resolving merge commit b149b803e501553d676415b9ee7a792fa61b395c. On macOS with the repository toolchain (Flutter 3.41.7, CocoaPods 1.17.0), pod install writes Pods/Manifest.lock without those lines. Xcode's [CP] Check Pods Manifest.lock performs a byte comparison and rejects the build with The sandbox is not in sync with the Podfile.lock. Consequently tools/native-review/ios_review.py:153-156 can never reach the integration journey from a clean checkout. I reproduced this twice at exact clean head 036e0535f46e77ba29cb5c35924d1fa1c7d0b93f, including after deleting mobile/ios/Pods and .symlinks; both receipts failed with cleanup passing and the run-owned simulator removed. Running pod install directly leaves the tracked lockfile hash unchanged but produces a manifest differing only at those four blank lines (cmp nonzero), which isolates the defect to the committed lockfile rather than stale Pods state.
Please regenerate/normalize mobile/ios/Podfile.lock and rerun just native-review-ios 'iPhone 17 Pro' at the new clean head. Add a CI check that runs pod install and asserts Podfile.lock equals Pods/Manifest.lock (or an equivalent canonicality check), because the current Mobile CI is green while the native rig is dead on arrival.
Validation at the pinned head:
python3 -m unittest discover -s tools/native-review/tests -p 'test_*.py'— 54/54 pass.swift test --package-path tools/native-review/swift— 3/3 pass.just native-review-doctor— all checks pass, including Accessibility and Screen Recording.- All three real macOS journeys pass against the isolated
buzz-harnessrelay with clean provenance, finalized H.264/yuv420pvideo-share.mp4, and cleanup passing: tooltip dwell, multiline composer, and search shortcut dismissal. just native-review-ios 'iPhone 17 Pro'— fails twice at Xcode's Pod manifest check; each failed receipt finalized screenshot/video/log evidence and reports cleanup passed. Simulator inventory hash before/after is identical.- Finding-bundle CLI retry was blocked by a transient internal PyPI TLS failure; its 54-test Python suite passed, but I did not count the failed network attempt as live workflow proof.
Residual risk: performance cohort comparison was covered by the unit suite but not a fresh 3× baseline/3× candidate native cohort; publication is exercised separately with the canonical Desktop artifact.
Signed-off-by: Jude Edwards <judeedwards@squareup.com>
Signed-off-by: Jude Edwards <judeedwards@squareup.com>
Signed-off-by: Jude Edwards <judeedwards@squareup.com>
Signed-off-by: Jude Edwards <judeedwards@squareup.com>
|
:bot: Jude’s code review agent Verdict: REQUEST CHANGES Reviewed: [P2] Make the documented clean-environment test command install its pinned dependency
python3 -m unittest discover -s tools/native-review/tests -p 'test_*.py'That exact command still fails from a new dependency-empty Python venv. The production launchers correctly self-provision Please make the public test recipe use an isolated pinned invocation—preferably one canonical wrapper/Just recipe shared by README and CI—and verify that exact recipe from an empty Python environment. Integrated exact-head evidenceAll three fresh lanes independently reproduced the documentation failure. The remediation delta is otherwise clear:
At verdict time, 16 checks had succeeded with no completed failure; Desktop Core, three smoke shards, Windows Rust, and two Desktop integration shards remained running. They are not counted as passed. No GUI or simulator was launched in this round. The previously reported exact-head native Desktop journeys and iOS failure receipts are not re-cited as proof for this newer head. GitHub forbids the authenticated PR-author account from submitting a formal request-changes review on its own PR, so this comment records the integrated verdict. Any new head invalidates it. |
Signed-off-by: Jude Edwards <judeedwards@squareup.com>
|
Addressed the clean-environment test-command finding at exact head
Exact-head validation:
Live PR/local/remote head OIDs match. DCO is green; the new CI run is still in progress. The earlier base-head Desktop shard had two tooltip-semantic failures and one unrelated video-control flake; this four-file test-command delta does not touch Desktop behavior, but the new exact-head CI still has to earn green rather than inheriting it by folklore. |
Signed-off-by: Jude Edwards <judeedwards@squareup.com>
|
Follow-up: the first canonical wrapper exposed a real CI portability defect on |
|
:bot: Jude's code review agent Verdict: REQUEST CHANGES Reviewed: Risk: medium. This delta is tooling-only, but its contract is contributor reproducibility for a native test harness that handles reviewer state and evidence. Blocking finding
Please either make the suite support the baseline interpreter, or pin/provision a supported Python and declare/check the minimum with an actionable error. Add a clean-shell or minimum-version regression leg so CI cannot silently regain this dependency on runner PATH. Validation at exact clean head
The prior missing-PyYAML finding is fixed: README, Justfile, and CI now converge on one wrapper, the dependency is pinned, cleanup works, and failures propagate. The remaining blocker is interpreter reproducibility. No GUI or simulator was launched. Broader CI still had Desktop Core, four smoke shards, relay E2E, Mobile, and Windows running at review time; no completed failure was observed, and those jobs were not counted as passed. |
Summary
finding-bundlecommand for optional clipping, focused/redacted logs, minimal receipt provenance, and hashed manifestStacked on #5972 because the native-review harness has not landed on
mainyet.Validation
At exact clean head
2b74739e0f7869c559612dcedb4b61b7094c7aab:python3 -m unittest discover -s tools/native-review/tests -p 'test_*.py'— 29/29 passjust native-review-ios 'iPhone 17 Pro'— passes on iOS 26.5; receipt clean and cleanup passesvideo-share.mp4is H.264/yuv420p, 994×2160, and accepted by the live Buzz upload endpointReceipt:
test-results/native-review/2b74739e0f78/ios_pairing/ios-20260815T120507-595304/receipt.json