Skip to content

fix(scrub): settle BrainBar exit and name refusal gates - #1063

Merged
EtanHey merged 3 commits into
mainfrom
wt/scrub-quiesce
Oct 4, 2026
Merged

EtanHey merged 3 commits into
mainfrom
wt/scrub-quiesce

Conversation

@EtanHey

@EtanHey EtanHey commented Oct 4, 2026 •

Copy link
Copy Markdown
Owner

Guarded at-rest scrub collapsed different shutdown failures into quiesce-failed, hiding which service or gate refused the write. It also checked BrainBar processes immediately after asynchronous launchd bootout. This change stops the daemon before the UI (the daemon UI-watchdog can fall back to openBundle), allows up to 30 seconds for process exit, then retains both loaded-service/process/lsof checks before opening a writer and after the survey. A timeout, revived job, unknown state, or probe failure still refuses the write and restores only successfully booted-out services, respecting deliberate pauses.

Refusal JSON now includes an allowlisted, value-free detail: bootout/state/loaded plus the fixed service label, process name, or gate name. It survives the scrub error wrappers without retaining exception values.

The CLI already exits 1 on refusal in current, installed, and pinned one-off code. The observed zero was a shell receipt bug: /bin/sh expands echo "$(date ...) exit=$?" using the successful date substitution's status. Reproduced with false. A reusable scripts/scrub-at-rest-oneoff.sh LOG_PATH ABSOLUTE_PYTHON [options] captures the CLI status immediately, logs it, and exits with that status; the lead can use it when scheduling the next quiet-window run.

Read-only diagnosis: all 13 configured service labels match the GUI domain; the enrichment job is absent, while resident BrainBar/UI/hotlane jobs use KeepAlive. Historical restoration messages prove bootout reached the final enrichment service without an earlier bootout failure. Old JSON omitted the subgate, and examined lifecycle/unified logs did not identify it: an unknown state on that final service check, a reloaded job, or a still-running process cannot be distinguished retrospectively. The ordering and asynchronous-exit defects are reproduced with synthetic tests; they are not claimed as a proven historical subgate or live remediation.

Validation: RED 15 diagnostic/settling failures plus the separate UI-revival ordering failure. Final focused suite: 208 passed; Ruff and shell syntax passed. Full normal pre-push at c276533: 6,222 passed, 11 skipped, 70 deselected, 2 expected failures, 108 warnings; registration 3 (1 warning), isolated routing/eval 40, Bun 1, FTS shell passed. Final head b8619ee adds only explicit fixture-exhaustion handling and check=False on subprocess tests that assert refusal statuses; runtime/source and shell wrapper are byte-identical to that full-gate base. The supported changed-files ranges from each preceding gated commit reran the whole changed test file (152 passed) plus registration 3, isolated routing/eval 40, Bun 1 and FTS shell gates.

Local CodeRabbit reviewed all five production/test/script files with zero findings before the final fixture-only follow-up. The fixture-exhaustion follow-up's local review was rate-limited; builder inspected the exact helper delta using the red-team/blue-team prompts, and fresh tests/gates passed. No independent pair-review claim. All three provider modes cover refusal/apply exit status, real Python CLI entry points, safe diagnostics, timeout, restoration, and process-exit synchronization. The shell wrapper preserves 0/1/75. Tests use synthetic temporary databases only. No canonical scrub, database read/open/copy, real service quiescence, BrainBar stop, merge, install, or release.

Hosted DeepSource Python remains failing: four protected-member style notices received explicit builder pushback for shared internal safety helpers; two subprocess notices are fixed in b8619ee. This is not a passing CI or lead-approved waiver claim. Lead-routed review must resolve the remaining check.

Bot policy: lead-routed Opus review after CI, plus CodeRabbit. Ready PR for lead-owned review and later canonical scheduling, unmerged.

— brainlayerCodex (worker) · codex/gpt-6.1-sol

EtanHey and others added 2 commits October 4, 2026 13:08
Co-Authored-By: brainlayerCodex running gpt-6.1-sol <noreply@openai.com>
Co-Authored-By: brainlayerCodex running gpt-6.1-sol <noreply@openai.com>
@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 36 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 280745ba-d5c6-4144-acb2-0ef653c43b52
📥 Commits

Reviewing files that changed from the base of the PR and between 072f02f and b8619ee.

📒 Files selected for processing (5)
  • scripts/scrub-at-rest-oneoff.sh
  • src/brainlayer/cli/__init__.py
  • src/brainlayer/maintenance.py
  • src/brainlayer/scrub_at_rest.py
  • tests/test_scrub_at_rest_command.py
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cursor

cursor Bot commented Oct 4, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: f21248b9-fe6c-4423-b051-3ca7b2e253e8)

@deepsource-io

deepsource-io Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in 072f02f...b8619ee on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
Python Oct 4, 2026 10:32a.m. Review ↗
Swift Oct 4, 2026 10:32a.m. Review ↗
JavaScript Oct 4, 2026 10:32a.m. Review ↗
Shell Oct 4, 2026 10:32a.m. Review ↗
Secrets Oct 4, 2026 10:32a.m. Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

@github-actions

github-actions Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

BrainLayer ratchet

Every Value below was measured by this run. A row this machine cannot measure says n/a — <reason> instead of a number; baselines in Notes name their own machine, method and date and were not measured here.

Row Status Value (measured by this run) Method Notes
commit provenance 🟢 GREEN measured b8619ee90c64 == PR head · checkout 2142f96ce8d8 commit graph + live PR head · in-process · runner Which commit this whole table is about. On a pull_request event the checkout is GitHub's synthetic merge ref, whose sha is not on the PR — #759's table printed 13fa724278bf while that PR's head was 4632f979 — so this row names the PR-head parent instead, the sha a reviewer can actually see. The comparison sha is read live from repos/{owner}/{repo}/pulls/{n} when the table is collected, not taken from the event payload, because the payload cannot know the run has been overtaken. Residual window, stated rather than papered over: a push landing between that read and the comment being posted is not caught here — the run for that push refreshes the table.
baseline attestation 🟢 GREEN baseline f421d1a7c5e6 matches the main attestation (run 36914446318 · main 072f02f7cf10 · 2026-10-01T19:28:05Z) main attestation artifact via Actions API · in-process · runner What every comparison is measured AGAINST, and who says so. The baseline fields of tests/fixtures/sprint_gate/corpus.json (queries, latency_baseline_ms, thresholds) are compared to the ratchet-attestation artifact of the latest successful push or (no-input) workflow_dispatch run of ratchet-attest.yml on main, fetched through the Actions API — a PR run cannot write to another run's artifacts. A field that differs is RED unless that main run measured the new value. The calibrated socket collector can license p50/p95; every absent measured path stays locked, so missing collection never passes as permission for a hand edit. Boundary: the comparator is this PR's checkout of ci_ratchet_table.py, diff-reviewable, not tamper-proof.
provenance 🟢 GREEN stamped 2142f96ce8d8 == HEAD, tree clean wheel stamp · in-process · runner Sha half of #749 keg-mode provenance: a keg built from this wheel can answer __build_sha__. The helper-age and served-process predicates need a running BrainBar and are measured only by scripts/sprint_gate.py on an installed Mac. The sha here is the checkout's — the merge ref on a PR — because that is what publish.yml stamps at release time; the PR-head sha this table describes is the one in commit provenance above.
fallback replay debt ⚪ n/a n/a — no fallback queue on this machine: the pending memories live in ~/Gits/*/docs.local/decisions, and docs.local/ is gitignored, so a runner checkout has no copy of them to count docs.local walk · machine with the fallback queue intended_brain_store: true with no chunk_id means a memory reached disk and never reached the DB, so it answers no brain_search. Budget: 0. Any pending or unparseable file is a finding, never a band -- 122 of these sat from 2026-06-28 to 2026-09-05 because nothing counted them where a reader would look. Measured by walking the tree, so it is only ever measured on a machine that HAS the tree.
mapped bytes ⚪ n/a n/a — no BrainBar daemon at /tmp/brainbar.sock: this row needs the daemon, its hybrid helper and the indexed corpus running together, and no GitHub-hosted runner has them (macOS included) — only a self-hosted Darwin/arm64 runner on an installed Mac would socket · installed Mac Baseline 26.2 GB — installed Mac, socket, 2026-09-03, after R2 drained 15,070 → 0. Up from 16.8 GB because the drain left more vectors mapped under the same cap: the change is the drain, not a leak. Not measured by this run.
search p50/p95 ⚪ n/a n/a — no BrainBar daemon at /tmp/brainbar.sock: this row needs the daemon, its hybrid helper and the indexed corpus running together, and no GitHub-hosted runner has them (macOS included) — only a self-hosted Darwin/arm64 runner on an installed Mac would socket · installed Mac Margin p50: margin unmeasured — 0 of the 5 attested green main runs it needs; no verdict is rendered from fewer. Margin p95: margin unmeasured — 0 of the 5 attested green main runs it needs; no verdict is rendered from fewer. Calibrated on MacBook-Pro.local at 2026-09-01T08:42:22Z under active_sprint_load (tests/fixtures/sprint_gate/corpus.json). Not measured by this run.
idle CPU ⚪ n/a n/a — no BrainBar daemon at /tmp/brainbar.sock: this row needs the daemon, its hybrid helper and the indexed corpus running together, and no GitHub-hosted runner has them (macOS included) — only a self-hosted Darwin/arm64 runner on an installed Mac would ps sampling · installed Mac Ceiling: average CPU < 30% over a 60 s window (resource_budget in scripts/sprint_gate.py), ratified and kept as a hard budget. Margin daemon: margin unmeasured — 0 of the 5 attested green main runs it needs; no verdict is rendered from fewer. Margin helper: margin unmeasured — 0 of the 5 attested green main runs it needs; no verdict is rendered from fewer. Margin watcher: margin unmeasured — 0 of the 5 attested green main runs it needs; no verdict is rendered from fewer. Needs the BrainBar daemon, helper and watcher actually running. Not measured by this run.
signature_valid ⚪ n/a n/a — the macOS signature-parity job is trigger-gated and did not run on this PR: it touches no release or signing path (pyproject.toml, scripts/release-*, scripts/brainlayer-version-check.sh, publish.yml, ratchet.yml) and carries no ratchet:signatures label — a GitHub macOS runner bills at ~10× Linux minutes and rebuilds the keg venv from source codesign · installed keg scripts/release-verify-signatures.sh <keg> codesign-verifies every *.so/*.dylib under libexec/venv. The macOS parity job installs the published tap formula (etanhey/layers/brainlayer), so this row measures the release path — formula, published sdist and Homebrew's relocation — and not this PR's tree. Release-time baseline for the same keg on a different machine: 442 valid / 0 invalid — installed Mac (M4 Max), brew --prefix brainlayer 1.5.11, 2026-09-03.

🟢 GREEN measured, within budget · 🔴 RED measured, out of budget — a finding to clear before merge · ⚪ n/a not measurable on this machine, never guessed.

No RED rows.

Measured on Linux/x86_64 · measured b8619ee90c64 · PR head b8619ee90c64 · checkout 2142f96ce8d8 · run · updated 2026-10-04 10:33:02 UTC

@EtanHey EtanHey added the size:M Tight-loop PR size: 151-400 hand-written lines added label Oct 4, 2026
_QUIESCE_DETAILS = frozenset(
{"quiesce-services", "brainbar-process-probe", "lsof-writers", "process:BrainBar", "process:BrainBarDaemon"}
| {
f"{step}:{maintenance._launchd_label(service)}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Access to a protected member _launchd_label of a client class


Accessing a protected member (a member prefixed with _) of a class from outside that class is not recommended, since the creator of that class did not intend this member to be exposed. If accesing this attribute outside of the class is absolutely needed, refactor it such that it becomes part of the public interface of the class.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WAIVED as an intentional package-internal call: maintenance is an internal module, not a public client class. Scrub already shares its quiesce, pause, backup, resume, and lock helpers. Reusing the authoritative label map and writer gates preserves that design and avoids divergent safety checks. Expanding/renaming the API belongs in a separate cleanup; this patch retains the existing boundary. Lead-routed pair review still applies.

— brainlayerCodex (worker) · codex/gpt-6.1-sol

raise ScrubAtRestError("writer service remains loaded", reason="quiesce-failed")
for service in LIVE_SERVICES:
try:
loaded = maintenance._service_is_loaded(service)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Access to a protected member _service_is_loaded of a client class


Accessing a protected member (a member prefixed with _) of a class from outside that class is not recommended, since the creator of that class did not intend this member to be exposed. If accesing this attribute outside of the class is absolutely needed, refactor it such that it becomes part of the public interface of the class.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WAIVED as an intentional package-internal call: maintenance is an internal module, not a public client class. Scrub already shares its quiesce, pause, backup, resume, and lock helpers. Reusing the authoritative label map and writer gates preserves that design and avoids divergent safety checks. Expanding/renaming the API belongs in a separate cleanup; this patch retains the existing boundary. Lead-routed pair review still applies.

— brainlayerCodex (worker) · codex/gpt-6.1-sol

raise ScrubAtRestError(
"writer service remains loaded",
reason="quiesce-failed",
detail=f"loaded:{maintenance._launchd_label(service)}",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Access to a protected member _launchd_label of a client class


Accessing a protected member (a member prefixed with _) of a class from outside that class is not recommended, since the creator of that class did not intend this member to be exposed. If accesing this attribute outside of the class is absolutely needed, refactor it such that it becomes part of the public interface of the class.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WAIVED as an intentional package-internal call: maintenance is an internal module, not a public client class. Scrub already shares its quiesce, pause, backup, resume, and lock helpers. Reusing the authoritative label map and writer gates preserves that design and avoids divergent safety checks. Expanding/renaming the API belongs in a separate cleanup; this patch retains the existing boundary. Lead-routed pair review still applies.

— brainlayerCodex (worker) · codex/gpt-6.1-sol

config.expected_writer_patterns = ()
maintenance._check_lsof_clean(config)
try:
maintenance._check_lsof_clean(config)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Access to a protected member _check_lsof_clean of a client class


Accessing a protected member (a member prefixed with _) of a class from outside that class is not recommended, since the creator of that class did not intend this member to be exposed. If accesing this attribute outside of the class is absolutely needed, refactor it such that it becomes part of the public interface of the class.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WAIVED as an intentional package-internal call: maintenance is an internal module, not a public client class. Scrub already shares its quiesce, pause, backup, resume, and lock helpers. Reusing the authoritative label map and writer gates preserves that design and avoids divergent safety checks. Expanding/renaming the API belongs in a separate cleanup; this patch retains the existing boundary. Lead-routed pair review still applies.

— brainlayerCodex (worker) · codex/gpt-6.1-sol

interpreter.write_text(f"#!/bin/sh\necho '{{\"synthetic\":true}}'\nexit {code}\n")
interpreter.chmod(0o700)
log = tmp_path / "oneoff.log"
result = subprocess.run(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'subprocess.run' used without explicitly defining the value for 'check'.


subprocess.run uses a default of check=False, which means that a nonzero exit code will be
ignored by default, instead of raising an exception.

You can ignore this issue if this behaviour is intended.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FIXED in b8619ee: check=False is now explicit because these tests deliberately exercise nonzero exits and assert the captured returncode. The changed test file passes all 152 tests; no assertion or exit behavior was weakened.

— brainlayerCodex (worker) · codex/gpt-6.1-sol

from pathlib import Path

env = {**os.environ, "PYTHONPATH": str(Path(__file__).resolve().parents[1] / "src")}
result = subprocess.run(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'subprocess.run' used without explicitly defining the value for 'check'.


subprocess.run uses a default of check=False, which means that a nonzero exit code will be
ignored by default, instead of raising an exception.

You can ignore this issue if this behaviour is intended.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FIXED in b8619ee: check=False is now explicit because these tests deliberately exercise nonzero exits and assert the captured returncode. The changed test file passes all 152 tests; no assertion or exit behavior was weakened.

— brainlayerCodex (worker) · codex/gpt-6.1-sol

@EtanHey

EtanHey commented Oct 4, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

— brainlayerCodex (worker) · codex/gpt-6.1-sol

@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Co-Authored-By: brainlayerCodex running gpt-6.1-sol <noreply@openai.com>
@cursor

cursor Bot commented Oct 4, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: 47860c4c-8f4b-4f36-936a-77977198c992)

@EtanHey
EtanHey merged commit e59cf87 into main Oct 4, 2026
23 of 24 checks passed
@EtanHey

EtanHey commented Oct 4, 2026

Copy link
Copy Markdown
Owner Author

Merged at e59cf871 from head b8619ee9. Codex brainlayerCodex-a9424d3e implemented; Opus pair review PASS (152/152; stubbed timeout + resume-order checks).

  • Fix: the BrainBar daemon stops before the UI (it closes the daemon's openBundle UI-watchdog relaunch path); a bounded ≤30 s async-exit wait; an allowlisted value-free refusal detail; the CLI exits 1 on any refusal.
  • Note: the earlier exit=0 in the 10-02 scrub logs was the lead's wrapper bug ($(date) overwrote $?), not the CLI.
  • Follow-ups (filed): tests for resume order and for resume-on-wait-timeout; the process match catches dev .build/debug/BrainBar; pre-existing _quiesce_services never resumes a job whose bootout returned non-zero but unloaded anyway, and resume-failed reports only a count.

— brainlayerClaude-90982d09 (lead)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M Tight-loop PR size: 151-400 hand-written lines added

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant