Skip to content

Audit hardening: authorization, run recovery, and accessible review UI - #12

Open
FlowerYes wants to merge 4 commits into
mainfrom
codex/sept8-audit-hardening
Open

Audit hardening: authorization, run recovery, and accessible review UI#12
FlowerYes wants to merge 4 commits into
mainfrom
codex/sept8-audit-hardening

Conversation

@FlowerYes

Copy link
Copy Markdown
Member

Summary

This PR packages the September 8 audit into four reviewable commits. It fixes reproducible defects in authorization/replay, task approval and synthesis recovery, specialist prompt context, and browser review state. It also includes the regression tests, rendered UI evidence, and a source-reconciled evaluation guide.

Local result: 1,329 tests passed, 1 live-provider test skipped; 46 new regression cases. This does not claim a completed live-provider deployment, production certification, or a win against competing products.

Baseline: 4606ccf on main (v0.4.0). No dependency upgrades or database migrations are included. The pre-existing local AGENTS.md change is deliberately excluded.

Suggested review order

Commit Review focus Main files
b7abffe Authorization must remain valid through registration, replay, delivery, and outage cleanup. services/organizations.py, services/rooms.py, realtime/websocket.py, realtime/fanout.py; three regression files
8734667 Atomic task/run settlement, synthesis cancellation/recovery, and equivalent specialist context across harnesses. db/repositories.py, services/{agent_tasks,steps,branches,runs,_shared}.py, nexus_bridge/agent_bridge.py; three regression files
aabb4ef Asynchronous actions retain their original context; live refresh preserves deliberate review; mobile and keyboard controls remain usable. web/index.html, web/app.css, browser modules, tests/e2e/test_operation_context.py
1dc845b Acceptance gates, reproducibility, corrected documentation, and audit evidence. Body-cap/performance tests, README.md, docs/{BACKLOG,EVALUATION}.md, dated audit report and screenshots

The PR contains 60 changed files, including 24 screenshots and seven new regression-test files. Start with the behavioral commits; the images document desktop/mobile and both themes, not additional application code.

Full audit, architecture/provider trace, current competitor sources, and remaining roadmap

1. Authorization and realtime correctness

Problems

  • Workspace removal committed membership changes but left existing subscriptions able to receive future room events until periodic revalidation.
  • Room creation could authorize workspace membership before a concurrent removal, then use that stale decision inside its write.
  • Membership could change after initial WebSocket authorization, during subscription registration/backfill, or before a queued event was delivered.
  • Redis reconnect backoff used an unbounded exponent and eventually raised OverflowError after repeated failures.

Changes

  • Room creation repeats the workspace-membership check inside the writer transaction.
  • Workspace removal revokes all affected subscriptions, notifies clients, and broadcasts canonical events before advisory presence cleanup. Presence outages do not interrupt the remaining rooms' cleanup. Room removal/leave follow the same availability separation.
  • WebSocket registration and event delivery recheck durable read authorization. Unexpected authorization-read failures close with 1011; sender completion also tears down a silent receiver. All subscriptions are removed before advisory presence operations.
  • Redis backoff doubles a bounded delay and resets on recovery. The regression exercises recovery after 1,100 failures.

Fresh review found two regressions in an earlier version of these repairs—presence failure interrupting multiroom cleanup, and a failed authorization read leaving a silent socket subscribed. Both now have failure-injection tests and were repaired before this PR.

Reviewer hotspot: examine commit-versus-delivery boundaries and multiroom cleanup. Do not replace the durable checks with an unchecked cache to recover throughput.

2. Task, synthesis, and provider lifecycle

Task/approval settlement

An A2A task previously became failed when its execution paused for human approval without returning an output. A later approval could finish the execution while the task remained failed.

Task state now follows the matching current execution through authorization-required, working, completion, rejection, expiry, and cancellation. Task state, answer, and canonical events settle in the same transaction as the run. Cancellation refuses late output; recovery preserves an already completed answer. An older execution cannot settle a newer task attempt.

Synthesis recovery

Synthesis previously persisted RUNNING/idempotency before awaiting the model but could remain RUNNING indefinitely after cancellation or a crash. The start event was delayed until termination.

  • Start state, idempotency key, and canonical start event are now committed together before the provider call.
  • An immutable created_at establishes a fixed, nonconfigurable five-minute deadline. Provider waiting is bounded, and publication checks both status and deadline inside its transaction.
  • Cancellation shields terminal cleanup from repeated cancellation. Startup/periodic recovery fails expired records but preserves another worker's fresh request.
  • Conditional failure settlement prevents duplicate terminal transitions; a late provider result cannot publish after recovery.
  • A completed key replays its original result. A failed key retains its failure; retry with a new key.

Specialist context

The direct model-provider harness omitted specialist identity/instructions. It now uses the same specialist prompt assembly as the default bridge, without duplication. A paired-harness regression compares outgoing prompts, frozen branch context, and exact persisted provider-input provenance.

Reviewer hotspots: atomic event creation, cancellation during failure handling, stale-worker publication, matching execution ownership, and the five-minute deadline's suitability for existing deployments.

3. Browser behavior and UI

  • Capture room/session/branch identity across launch and publication awaits; channel navigation cannot redirect a partially completed launch into another channel.
  • Single-flight launch/publication prevents duplicate submissions. Publication immediately communicates busy state, retains that state through refresh, and selects the returned artifact.
  • Meta results are bound to their originating request/channel/session and are cleared on channel change/revocation.
  • Reconcile output cards by identity so live refresh and selection changes preserve focus and expanded provenance.
  • Store synthesis-title drafts per branch; calculate branch counts and source evidence using room-wide output data rather than only the selected branch.
  • Replace dimmed/struck-through excluded text with readable content and explicit Included/Excluded/Needs review labels.
  • Show the full branch question, visible synthesis field labels, and actionable publication guidance.
  • Provide 44×44px mobile header controls; move secondary actions into the channel menu. Keyboard menu traversal skips hidden/disabled items on desktop. Notifications are native buttons.
  • Remove height/margin animations from message-action reveals.

The existing visual identity and navigation model are preserved. This is a targeted usability/correctness repair, not a wholesale redesign.

Representative UI evidence — click to expand

Desktop review, light

Desktop review

Mobile publication, dark

Mobile publication

Additional evidence: desktop dark, 320px menu, 320px dark review.

All specialist content shown is explicitly simulated. These images are not evidence of reasoning quality.

Verification and reproducibility

Local environment: Python 3.12 on macOS, pinned constraints.txt, Chromium. Real local HTTP/WebSocket/browser checks ran outside the restricted socket sandbox. Production code was unchanged when packaging the verified tree into commits.

Check Result
Full pytest suite 1,329 passed, 1 skipped, 285.51 seconds
New browser regression suite 12 passed; publication additionally checked with a different artifact type so retaining the seeded selection cannot pass accidentally
New regression cases overall 46
Ruff lint / format Pass
Strict mypy Pass, 61 source files
Existing trace-anchor checker 94 anchors resolved
Audit report local links 24 resolved
pip check No broken requirements
pip-audit -r constraints.txt No known vulnerabilities found
File-backed selection acknowledgement p95 1.343 ms, 100 writes; all distinct contiguous events and final selection survive reopen

The performance test now asserts p95 below 250 ms instead of merely printing it. The macOS body-cap regression measures lifetime peak RSS through a test-only child probe; it still requires rejection of a 64 MiB chunked body with HTTP 413, under 15 seconds and under 32 MiB peak growth. Windows native measurement is preserved.

python3 -m venv .venv
source .venv/bin/activate
pip install -c constraints.txt -e '.[dev,e2e,redis]'
python -m playwright install chromium

PYTHONPATH=src python -m pytest
PYTHONPATH=src python -m pytest tests/e2e/test_operation_context.py
PYTHONPATH=src python -m pytest -s tests/performance/test_ack_latency.py
ruff check .
ruff format --check .
PYTHONPATH=src mypy src
python scripts/check_anchors.py
python -m pip check
pipx run pip-audit -r constraints.txt

The one skipped test requires OPENAI_API_KEY. There are 28 existing transport/cookie deprecation warnings, not suppressed. No external NEXUS runtime, real model endpoint, container build, or alternate supported Python interpreter was validated in this local audit. CI status should be reviewed independently before merging.

Compatibility, operational risks, and rollback

  • No schema migration, dependency change, new public route, credential, or production dataset is included.
  • Canonical synthesis start events now become visible before completion; consumers must not assume the old delayed ordering or hard-code event counts.
  • Synthesis now has a five-minute maximum lifetime; recovery may mark expired work failed. Reverting application code does not undo already committed events or published artifacts. Use normal versioned deployment/backup procedures; do not delete history to roll back.
  • Authorization safety has a measured replay cost: an isolated five-client test with 5,000 events each took 11.0–11.7 seconds/socket, versus 0.82–1.14 seconds with handshake-only checks. This isolates file-backed query/serialization work and excludes network time; it is not a production load benchmark.
  • Provider cancellation/late-publication fencing does not establish that upstream work or billing stopped.
  • The mobile sticky publication form still occupies roughly 300px of an 844px viewport. More compact reading/review layout remains follow-up work.
  • The optional UI detector ran in degraded regex mode; screenshots, executable tests, and targeted computed contrast checks supplement it. This is not a complete WCAG certification.

Explicitly out of scope / remaining product gaps

The audit report compares current primary-source evidence for QM, Buzz, Grok Bot, Hermes, Commonly, Patchwork, and Agor, but does not claim a hands-on competitive win. Remaining work includes independent skill grants, full provider-call accounting, original-source evidence beyond model text, operator-light email onboarding, artifact authoring/version comparison, and a live three-to-five-human benchmark. Existing workspace-directory and manual-decision UI are acknowledged rather than incorrectly listed as absent.

Reviewer checklist

  • Check transactional membership revalidation and revocation during replay/queued delivery.
  • Check presence failure cannot interrupt canonical delivery or multiroom cleanup.
  • Check task/run/answer/event atomicity and stale execution guards.
  • Check synthesis deadline, repeated cancellation, restart recovery, idempotent replay, and late results.
  • Check harness context/provenance parity without duplicate instructions.
  • Exercise channel navigation while a launch, publication, or Meta request is pending.
  • Exercise output review with keyboard focus/provenance open during refresh and selection changes.
  • Review desktop/mobile menu navigation and publication form tradeoffs.
  • Review CI results and the stated live-provider/runtime validation limits.
  • Confirm the measured replay tradeoff and remaining product gaps are acceptable for this release.

@FlowerYes
FlowerYes requested review from Yasser-Ameur and Yasser-Ameur-Dev and removed request for Yasser-Ameur September 10, 2026 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant