Skip to content

Fix the built-in issues two workspace updates reported, and batch the report - #622

Open
gnguralnick wants to merge 68 commits into
mainfrom
gabriel/wondrous-stingray
Open

Fix the built-in issues two workspace updates reported, and batch the report#622
gnguralnick wants to merge 68 commits into
mainfrom
gabriel/wondrous-stingray

Conversation

@gnguralnick

@gnguralnick gnguralnick commented Sep 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes every issue in the bug report an agent filed while updating a workspace from minds-v0.6.1 to minds-v0.6.2 (Sentry event d1f5f23c0e624d5ba869d992feb4fc68), plus the one-report-per-pass change asked for alongside it, and one more from a later update (issue 7). All seven from the first report were confirmed against this repo's origin/main before anything was changed. Every fix that carries a test has a control run showing that test fails without it — issues 1, 2, 3, 4 and the reserved-name work. Issue 0 is prose only and has no test. Issue 6's tests cannot reach green on macOS at all (see Testing below), so its three fixes were verified only as far as this platform allows; the Linux CI run on this PR is what actually validates them.

0. A built-in defect had nowhere to go. Both upstream channels exist, but nothing said that finding a defect is itself a reason to use one, so the default was a local tk ticket — which cannot reach code that lives upstream. update-self now has a terminal step that submits such findings or names them in the results message with the submission offered, and AGENTS.md states the rule and names both channels.

/assist now sends one report per pass, covering every built-in issue found, instead of one per issue. Each POST pops its own modal over the user's workspace.

1. host-backup-now was OOM-killed on every run. Its in-flight check read the whole events log to look at the last 200 lines; that file reaches gigabytes because each event embeds the stdout of the restic command it reports. It now reads only the end. This also fixes a lie in the update flow: update-self takes a restore point by calling this, so on any workspace with a grown log every update reported no restore point while the snapshots were healthy. The log is capped at write time too, head and tail kept.

2. An update built the frontend into a tree with no node_modules. npm ci was gated on whether the npm manifest merged; the build ran whenever frontend source merged. 0.6.2 changed neither manifest, so the build died on tsc: not found and the update rolled back. Whether the manifest changed says nothing about whether the dependencies are installed; both are asked now, as the recovery path already did.

3. The build-app scaffolder carried a third, drifted copy of the reserved-name list, so names scaffolded cleanly and then failed to register. It now asks forward_port.py itself, which removes that carrier rather than syncing it.

4. The drift guard sampled names instead of comparing sets, so anything outside its fixed tuple could diverge unseen — and it restated the scaffold's rule in the test body rather than running it. It now compares the sets and calls _validate_name directly. Three findings from the same area: layout.py held a fourth copy that had drifted to two of eight names; github is now reserved (enabling GitHub sync would otherwise have failed the manifest suite); and app-vs-app program collisions are guarded, which the standalone-label rule never covered.

Also found while confirming 4: find_wiring_sections read only system/supervisord.conf, but every program now lives in a drop-in — so app footprints silently lost their wiring, and the browser app's scope computation raised outright. It now reads the drop-ins and attributes each block to the file it is written in.

5. fleet_test.py failed for every agent and passed in CI, because it set MNGR_AGENT_ID but fleet.py reads MINDS_CHAT_ID first. Fixed at the conftest rather than in the one test — layout_test.py had the same bug, found by running the suite the way an agent does.

6. The agy_shim reminder test. The report's diagnosis was wrong: both strings are live and come from different scripts, and seeing one where the other was expected means the shim believed no step was open. Three real defects behind it — the fixture created a step and never started it, the turn key was read with GNU-only stat -c, and the test asked the kernel for a fresh inode and hoped. All three fixed.

7. Every update after a workspace's first failed to start (a second report from the 0.6.2 -> 0.7.0 update, Sentry event e720d59408c244f6b275a37fd4da2d19). Destroying the previous pass's worker left its mngr/update-self branch behind, and the new worker is created on a branch of that same name, so mngr create died on fatal: a branch named 'mngr/update-self' already exists. Step 3b now clears the old branch first: it deletes it when HEAD already contains it (after a landed pass, and after a rolled-back one, since a rollback is a revert on top of the merge), and otherwise keeps it under an archive/update-self-<timestamp> name and tells the user in plain words. The check is git merge-base --is-ancestor, not git branch -d: once GitHub sync has pushed the branch, -d compares against the pushed copy and would delete an update that never landed. Prose only, using plain git so it works with any workspace's own launcher. Verified in a scratch repo across no branch, landed, rolled back, unmerged, and pushed-but-unlanded. The same report's host-backup-now exit 143 is issue 1.

8. Test stubs fell through to the real tools, which killed a user's terminal sessions (Sentry event 90bbaf47b0c44cfd87944395c8a4d19c). A workspace container mounts /tmp as a bare --tmpfs, which Docker makes noexec. A stub executable a test writes under pytest's tmp_path and puts on PATH was therefore skipped, and the real tmux, mngr or latchkey ran. One update's validation run killed the user's terminals this way and failed 110 tests. Two parts:

  • The docker provider now mounts /tmp:exec (.mngr/settings.toml), matching mngr's gen-2 start args in the paired PR. That reaches containers created from here on.
  • For containers that still have the old mount, a new test-only lib, system/libs/pytest_executable_tmp, adds a pytest plugin. It is registered through pytest11, so the root, chat and system_interface runs in the workspace venv all load it. At configure time it runs a one-line script under the temp root. If that fails, it moves temporary files to /var/tmp, which is on the container's rw root filesystem, and says so in the header. If the root was chosen explicitly, or nothing works, it stops the session before any test runs.

Verified on a real noexec mount (a macOS RAM disk). A test stubbing uname under tmp_path got the real uname with the plugin disabled, and its stub with the plugin loaded. I also confirmed read-only on the reporting workspace that /var/tmp is mounted without noexec. The redirect path has no automated test, because that needs a noexec mount. The vendored mngr suite runs from its own venv and is not covered.

Testing

uv run --no-sync pytest per root, on macOS, with MINDS_CHAT_ID and MNGR_AGENT_ID set (an agent's env):

root result
system/scripts 402 passed, 4 failed
system/libs 583 passed, 1 skipped
system/services 451 passed
.agents 686 passed, 8 skipped
system/apps/browser 295 passed, 3 skipped
root test_*.py 43 passed

The 4 failures are the agy_shim tests that cannot pass on macOS: the shim runs its hooks under PATH=/usr/bin:/bin, where /bin/bash is 3.2.57 and cannot parse the reminder script, plus BSD sed and the absent /proc. A pristine origin/main worktree on this machine fails the same four. CI is Linux and is the real check for those.

Changelog gate passes: entries for agents, app_manifest, browser, dev, host_backup.

Paired with imbue-ai/mngr-internal#1110, which fixes the navigation bug in the same report.

Gabriel Guralnick and others added 13 commits September 17, 2026 13:56
Gabriel Guralnick Sculptor
Ask the registry whether a name registers, instead of copying its rule
21a6e37 
Reported from a workspace update to minds-v0.6.2 (Sentry event
d1f5f23c0e624d5ba869d992feb4fc68), issues 3, 4 and 5.

Problem: the reserved-name set had four carriers. forward_port.py owns it and
is stdlib-only by contract; app_manifest and layout.py cannot import it, and
the build-app scaffolder chose not to. layout.py's copy held two of the eight
names and the scaffolder's held neither the six it was missing nor any guard,
so nine names scaffolded cleanly and then failed to register. The drift test
meant to catch exactly this sampled a fixed tuple of names, so any name outside
the sample was free to diverge -- and it modelled the scaffold's rule in the
test body rather than running it, so it would have gone stale anyway.

Fix: the scaffolder asks forward_port.validate_service_name and keeps only its
own built-in app names, which eliminates that carrier rather than syncing it.
The remaining two copies exist because they cannot import, so the guard now
compares the sets wholesale and the scaffold guard runs _validate_name itself.
layout.py's copy is brought back in line.

Also from the same report: reserve `github`, since enabling GitHub sync writes
a github-sync program an app named `github` would claim as a sidecar; guard
app-vs-app program collisions, which the standalone-label rule does not cover;
and read the supervisord drop-ins when computing an app's wiring, which had
been reading only the main config since the programs moved out of it -- the
browser app's scope computation was raising outright.

Finally, the browser and scripts suites now clear MINDS_CHAT_ID and
MNGR_AGENT_ID. Tests that set only MNGR_AGENT_ID read the runner's real chat id
for the other, so they passed in CI and failed for every agent.

Co-authored-by: Sculptor <sculptor@imbue.com>
Gabriel Guralnick Sculptor
Stop host-backup-now reading a gigabyte to look at 200 lines
5aec7af 
Reported from a workspace update to minds-v0.6.2 (Sentry event
d1f5f23c0e624d5ba869d992feb4fc68), issue 1.

Problem: `uv run host-backup-now` exited 143, killed by earlyoom, within
seconds of starting and with 2.9 GB free. Its first step asks whether a backup
is already in flight, and `_scan_for_inflight_tick_ids` answered that by doing
`events_path.read_text().splitlines()` and then using only the last 200 lines.
Every event embeds the stdout of the restic command it reports, so that file was
1.59 GB on the reporting workspace. `_read_new_events`, directly below it, had
always seeked to a byte offset and read only the delta; this was the one
unbounded read in the module.

The consequence reached past the command: `update-self` takes a restore point
by calling it and treats a non-zero exit as "no confirmed restore point", so
every update on such a workspace ran, and told the user, that it had no restore
point -- while the hourly snapshots were healthy throughout.

Fix: read at most 8 MB from the end of the file and drop the line the window
cut in half. A workspace whose events are large gets fewer than 200 of them,
which is the right trade: only a BACKUP_STARTED with no completion after it
matters, and the events a tick emits before completing are the small ones, so an
in-flight tick is always inside the window.

Also cap each event field at 16 KB in `write_event`, keeping head and tail with
a marker naming what was dropped. Nothing rotates this log and the `restic
backup --json` progress stream was adding megabytes a day; the opening lines and
the final summary are what an operator reads, and both survive.

Co-authored-by: Sculptor <sculptor@imbue.com>
Gabriel Guralnick Sculptor
Install the npm dependencies when they are missing, not when the mani…
7b80d72 
…fest moved

Reported from a workspace update to minds-v0.6.2 (Sentry event
d1f5f23c0e624d5ba869d992feb4fc68), issue 2.

Problem: the minds-v0.6.2 apply failed and auto-rolled-back with `npm run build
failed (exit 127): sh: 1: tsc: not found`. The `npm ci` and the `npm run build`
were gated on different conditions: the install on `plan.frontend_manifest` (did
package.json or package-lock.json merge?), the build on `plan.frontend` (did
frontend source merge?). Neither manifest changed on either side of this update,
so the install was correctly skipped by its own rule -- into a tree that had no
system/node_modules at all, which is where the build then looked for tsc and
vite. The rollback worked and left the workspace healthy, so this cost an update
rather than any data.

The gate was wrong in principle: whether the manifest *changed* says nothing
about whether the dependencies are *installed*. The recovery path already
reasoned this way -- `_is_recovery_npm_ci_needed` checks `"node_modules" not in
restored` -- so the forward path was the odd one out.

Fix: install when the manifest merged, or when a build is going to run against a
tree that has no node_modules. The bundle-copy shortcut still wins over both,
since installing a verified worker bundle needs no node_modules at all.

The apply fixtures gain a node_modules, which is what a live workspace has; the
two tests that want a tree without one now say so.

Co-authored-by: Sculptor <sculptor@imbue.com>
Gabriel Guralnick Sculptor
Read the turn key portably, and stop asking the kernel for a new inode
c8d049b 
Reported from a workspace update to minds-v0.6.2 (Sentry event
d1f5f23c0e624d5ba869d992feb4fc68), issue 6.

The report diagnosed this as a test expecting a string the shim no longer
emits. That is not what is happening: both strings are live and come from
different scripts. `Open task reminder` is the open-steps carryover
(agent_open_tickets_reminder.sh) and `[Step tracking reminder]` is the
require-a-step nudge (agent_require_steps_pretool.sh). Seeing the second where
the first was expected means the shim believed no step was open.

Three real defects behind that:

1. `_with_open_step` creates a step and never starts it. `tk create --step` only
   declares the record; `tk start` is what makes one in_progress, which is what
   "an open step" means to both policies. So the fixture built the exact state
   that makes the require-a-step nudge fire. It also discarded the tk result, so
   a failing setup step was invisible; both calls are now checked.

2. The turn key is read with `stat -c %i`, which is GNU-only. Off Linux the
   probe yields nothing, the carryover block never runs, and the turn is never
   stamped -- so the suite reads as a broken guard rather than an unportable
   probe. Both spellings are asked now.

3. `test_the_reminder_returns_on_the_next_turn` recreated the marker and hoped
   the kernel handed back a different inode, holding the freed number down with
   64 files. CI hit the reuse, the reminder correctly stayed quiet, and the test
   failed -- which is the flake recorded in minds-v0.6.1.md. It now renames a
   confirmed-new inode into place and fails loudly if it cannot obtain one.

Verified as far as macOS allows: the turn key now resolves and the turn is
stamped, and the inode helper is exercised directly. The final assertion still
cannot pass here because the shim runs the reminder under PATH=/usr/bin:/bin,
where macOS's /bin/bash is 3.2.57 and cannot parse the script at all. That, BSD
sed and the absent /proc are why the same four tests fail on a pristine
origin/main checkout on this machine; CI is Linux and is the real check.

Co-authored-by: Sculptor <sculptor@imbue.com>
Gabriel Guralnick Sculptor
Give a built-in defect somewhere to go, and send one report for all o…
8df385e 
…f them

Reported from a workspace update to minds-v0.6.2 (Sentry event
d1f5f23c0e624d5ba869d992feb4fc68), issue 0 -- the one the reporting user said
mattered most -- plus the batching change asked for alongside it.

Problem: an agent that finds a bug in built-in code has no route to the code
that needs changing. Both upstream channels exist -- `submit-upstream-changes`
for a fix, the `/assist` report POST for a diagnosis -- but nothing said that
finding a defect is itself a trigger to use either; both read as things the user
initiates. So the default was to file a local `tk` ticket and call it done,
which is a dead end: the defect is upstream's, and every workspace that hits it
rediscovers and reburies it.

That leak is widest in `update-self`, whose review gates are designed to surface
this exact class and deliberately do not fix it locally, since a fix to a file
byte-identical to the release only manufactures divergence the next update must
reconcile. The worker duly labels findings `submit-upstream-changes candidate`
-- and "candidate" is where it stopped.

Fix: `update-self` grows a terminal step that collects every such finding and
either submits it or names it in the results message with the submission
offered. AGENTS.md states the general rule, names both channels and what each is
for, and says that a local ticket is not a valid end state for a built-in
defect. CLAUDE.md includes AGENTS.md, so both harnesses get it.

Separately, `/assist` now sends ONE report per pass covering everything it
found, rather than one per issue. Each POST pops its own modal over the user's
workspace, so a report per issue is a queue of modals to read and submit one at
a time for a single session's work. Issues can be split apart upstream; they
cannot be un-popped here. The description template is numbered to match, and the
new `update-self` step says the same thing.

Co-authored-by: Sculptor <sculptor@imbue.com>
Gabriel Guralnick Sculptor @claude
Prove the bounded tail read without patching Path.open
c9c4b6a 
Problem: test_the_inflight_scan_never_reads_the_whole_events_log in
system/services/host_backup/src/host_backup/cli_test.py counted the bytes read
by assigning a wrapper over the stdlib `Path.open` and restoring it in a
finally. That mutates a shared stdlib class for the length of the test, leaves
it patched if the process dies in between, and is the pattern
PREVENT_MONKEYPATCH_SETATTR exists to discourage, hand-rolled so the ratchet's
regex cannot see it. Its comment also claimed the scan runs "without the file's
size ever being read", when _read_tail_lines seeks to SEEK_END precisely to
read the size.

Fix: assert on what the bound does instead. A BACKUP_STARTED with no completion
now sits at the top of the log, under the 8 MB of padding events; the log is 52
lines, so max_lines excludes nothing and only a window that never reached the
top can leave that tick out of the answer. Confirmed the assertion
discriminates: with an unbounded window the same fixture yields
{'in-flight', 'older-than-the-window'}, with the shipped default {'in-flight'}.
The size threshold now follows _TAIL_READ_MAX_BYTES rather than a literal.

Co-authored-by: Sculptor <sculptor@imbue.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Gabriel Guralnick Sculptor @claude
Stop the app-program collision guard reporting an app against itself
646dc4a 
Problem: test_no_app_claims_another_apps_program_as_a_sidecar in
system/test_app_manifests.py ranges `owner` and `claimed` over the same list
with no self-pair exclusion, so an app whose own `program` is `<name>-<role>`
is reported as colliding with itself. That manifest is legal -- `program`
defaults to `name` only when the manifest omits it, and the sibling standalone
test subtracts `app_programs` precisely because an app's program need not be
its name -- and an app's own `<name>-<role>` program IS its sidecar, which is
what scope.py's sidecar_prefix is for. The suite runs inside user workspaces
over every manifest in the tree, so a user-built app named `pr` with
`program = "pr-server"` fails it for being correct.

Fix: exclude the self-pair. Verified with real AppManifest objects: `pr` /
`pr-server` now yields [], while the case the guard exists for -- an app `pr`
beside an app `pr-review` -- is still reported. The changelog entry now says
the guard is about another app's programs and why self-matching is not one.

Co-authored-by: Sculptor <sculptor@imbue.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Gabriel Guralnick Sculptor @claude
Make the scaffold's forward_port loader private, annotated and cached
c92e782 
Problem: the loader extracted out of _read_and_validate_icon in
.agents/skills/build-app/scripts/scaffold_flask_lib.py so _validate_name could
share it was named `load_forward_port`, though nothing outside the module calls
it and every other helper in the file is underscore-prefixed; it was also the
only function in the file with no return annotation, and it exec'd the ~800-line
forward_port.py afresh per call -- twice per scaffold run now, and once per
candidate name in the drift test that drives _validate_name.

Fix: rename to _load_forward_port, annotate it -> ModuleType, and memoize it.
forward_port.py is constants and functions with its CLI behind a __main__
guard, so reusing the first module object changes nothing observable.

Co-authored-by: Sculptor <sculptor@imbue.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Gabriel Guralnick Sculptor
Say what the tail read actually bounds, and spell the drop-in paths a…
010a6ae 
…s posix

Two items the review pass recorded without fixing.

`_read_tail_lines`'s docstring opened "Never reads the whole file", which is not
true when the log is smaller than the ceiling -- and the whole point of the
sentence is the guarantee, so state the guarantee: at most `max_bytes`, however
large the file is.

`_supervisord_conf_paths` built its RepoRelativePath with `str(...)` where the
rest of scope.py uses `.as_posix()`. Identical on both platforms we support, but
the repo-relative path spelling is the sort of thing that should have one form.

Co-authored-by: Sculptor <sculptor@imbue.com>
Gabriel Guralnick Sculptor
Say where a footprint's wiring is actually read from
8bf578d 
Problem: system/libs/app_manifest/README.md still documented
find_wiring_sections and the footprint's `wiring` block as reading only
system/supervisord.conf, and its worked example printed
`"path": "system/supervisord.conf"`. This branch made the computation read the
main config plus every system/supervisord.conf.d/*.conf drop-in and attribute
each block to the file it is written in, and every program in the template
lives in a drop-in -- so the documented path is one no real footprint can
contain (`app-manifest footprint system/apps/browser/app.toml` emits
supervisord.conf.d/browser.conf and supervisord.conf.d/xvfb.conf).

Fix: the README names both carriers, the example carries a drop-in path, and
the `wiring` field description says there is one entry per file a block is
written in. The changelog entry says the README moved with the code.

Co-authored-by: Sculptor <sculptor@imbue.com>
Gabriel Guralnick Sculptor
Bring the layout migration's reserved-name copy under the drift guard
94b3f0f 
Problem: system/scripts/migrate_workspace_layouts.py carries a fourth copy of
the reserved app-name set with the same stated meaning as the other three, and
it was still at the pre-branch drifted value {"localhost", "auth"} -- exactly
where layout.py was before this branch synced it. The new drift guard compares
only three carriers and its docstring says "three places", so the copy this
branch was written to eliminate was left in place and documented as gone. The
script is live: bootstrap runs it at every boot and the update apply runs it
before the services restart.

Fix: sync the set and prefixes in migrate_workspace_layouts.py, point its
comment at what it is a copy of, and compare it in
test_every_carrier_of_the_reserved_name_set_holds_the_same_set alongside the
other three. The six added names have been unregisterable for longer than the
migration has existed, so no legacy layout store can name one and no mapping
changes. Changelog corrected to four carriers.

Co-authored-by: Sculptor <sculptor@imbue.com>
Gabriel Guralnick Sculptor
Test the event-field cap where events.py is tested
c89245d 
Problem: the two tests for the per-field cap write_event applies were added to
cli_test.py, whose module docstring scopes it to "the host-backup-now waiters
and exit-code contract". They call write_event and make_event and assert on
events.jsonl; nothing in them touches cli.py. Every other module in the package
has its own <module>_test.py, and events.py was the one with none.

Fix: move both into a new events_test.py. cli_test.py keeps the tail-read tests,
which do exercise its own _read_tail_lines and _scan_for_inflight_tick_ids.

Co-authored-by: Sculptor <sculptor@imbue.com>
Gabriel Guralnick Sculptor
Name the real two-entry case in the wiring docs
74953aa 
Problem: the sentence added in 8bf578d said an app "with a sidecar of its own
usually has two" wiring entries. That is wrong: build-app writes one drop-in per
app, so a `<name>-<role>` sidecar shares its app's file. Two entries come from a
program declared under `[wiring] programs` that lives in its own drop-in.

Fix: name the case that actually produces two -- browser.conf for program:browser
and xvfb.conf for the xvfb it declares.

Co-authored-by: Sculptor <sculptor@imbue.com>
@gnguralnick
gnguralnick marked this pull request as ready for review September 20, 2026 05:23
gnguralnick
gnguralnick commented Sep 20, 2026
View reviewed changes
Comment thread .agents/skills/update-self/SKILL.md Outdated
Comment thread AGENTS.md Outdated
Gabriel Guralnick and others added 6 commits September 19, 2026 22:59
Gabriel Guralnick
Merge remote-tracking branch 'origin/main' into gabriel/wondrous-stin…
e7a44b2 
…gray
Gabriel Guralnick @claude
Pin app_manifest's supervisord reader to the include glob
4d01848 
Problem: scope.py's new drop-in reader carries a comment saying
system/test_supervisord_layout.py pins the directory it names, but that
file pins exactly four readers by path and app_manifest.scope is not one
of them. It is the fail-open shape that file exists to catch, and the
worst case of it: an empty read is find_wiring_sections' ordinary answer
("this app is not registered yet"), so a glob that moved would take the
app's wiring back out of its footprint in silence -- the same bug this
branch was written to fix.

Fix: added app_manifest.scope as a reader in that parity test, imported
normally rather than by path (it is a library, not a standalone script),
with its repo-relative paths resolved against the repo root. Updated the
module docstring's reader list and the test's "all four" wording to
match. Confirmed the new assertion fails when the constant is pointed at
a directory the glob does not name.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Gabriel Guralnick @claude
Keep one agent-identity fixture in the scripts' conftest
2e13fbb 
Problem: the branch added an autouse _isolate_agent_identity that clears
MINDS_CHAT_ID and MNGR_AGENT_ID, sixty lines above a pre-existing autouse
_isolate_own_chat_id that already cleared MINDS_CHAT_ID. Two autouse
fixtures for one job, each hiding the other from a reader of the other.
The new one's docstring also gave as its reason a failure the old one
already prevented -- a test setting only MNGR_AGENT_ID reading the
runner's real chat id -- which is true of the browser app, whose conftest
had no such fixture, but could not happen here.

Fix: dropped _isolate_own_chat_id and left _isolate_agent_identity as the
single fixture, with a docstring giving the reason that holds: layout.py
falls back from MINDS_CHAT_ID to MNGR_AGENT_ID, so clearing the chat id
alone still leaves the derived address steered by whoever runs the suite.
Ran the scripts' suites with both variables set in the environment.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Gabriel Guralnick @claude
Say what the scripts' identity fixture actually changed
ba8fad3 
Problem: the system changelog claimed layout.py's tests "passed in CI and
failed for every agent that ran them". They did not: the scripts' conftest
already carried an autouse fixture clearing MINDS_CHAT_ID, and the tests
were measured passing at the pre-branch tree under an ambient chat id
distinct from the one they set. The identical claim for the browser app
does reproduce and its entry is left alone.

Fix: rewrote that paragraph as the hardening it is -- the chat id was
already hidden, the MNGR_AGENT_ID it falls back to was not, and both are
now cleared in one fixture rather than two.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Gabriel Guralnick @claude
Drop a false precedent from the npm-ci comment
5fab762 
Problem: the comment explaining the forward apply's new npm-ci condition
said "Both conditions are asked, as the recovery path already asks them
(_is_recovery_npm_ci_needed)", and the changelog repeated it. The
recovery path reads `plan.frontend_manifest and
_is_recovery_npm_ci_needed(...)` -- a conjunction gated on the manifest,
the shape this change replaces -- and that helper asks whether the
pre-apply node_modules copy could be put back, not whether node_modules
is present. Wording only: the forward install now happens before the
build, and take_snapshots records nothing for a target that did not
exist, so the rollback leaves that install standing.

Fix: kept the reasoning that stands on its own (manifest-changed and
dependencies-present are different questions, and the bundle-copy
shortcut still wins over both) and removed the appeal to a precedent
with a different shape.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Gabriel Guralnick @claude
Let the open-steps reminder parse under bash 3.2
ddbfaa0 
Problem: the branch makes the shim's turn-key probe portable so an agent
reading a red suite on a laptop can tell an unportable probe from a
broken guard -- but the reminder still never fired off Linux, and three
of the four reminder tests still failed on macOS. The probe now works;
the script it then runs does not parse. /bin/bash on macOS is 3.2, which
is what the shim hardcodes as REAL_BASH, and 3.2 scans for quotes inside
$( ) even within a heredoc, so the apostrophe in "if it's not already
in_progress" opens a quote that never closes. The shim runs that script
with 2>/dev/null and falls back to an empty reminder, so the parse error
was as silent as the stat one.

Fix: reworded that clause to "if it is not already in_progress".
`/bin/bash -n` now passes over it (and over every other script the shim
runs), and the three reminder tests pass on macOS. Extended the
changelog paragraph to name both causes rather than leave it claiming a
fix it had not finished.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@gnguralnick gnguralnick mentioned this pull request Sep 20, 2026
Draft
Gabriel Guralnick added 8 commits September 21, 2026 17:25
Gabriel Guralnick
Merge origin/main into gabriel/wondrous-stingray
4d26daa 
Two conflicts, both settled by what main did rather than by preference.

system/scripts/migrate_workspace_layouts.py: main deleted the layout
migration outright, along with its test and every live reference to it.
This branch had only synced that file's copy of the reserved-name set, so
with the file gone there is no fourth carrier left to sync. Took the
deletion, and put the drift guard in forward_port_test.py back to the
three carriers that still exist. Changelog says three again.

system/scripts/conftest.py: both sides added an autouse fixture hiding the
ambient agent identity. Main's clears the chat id; this branch's clears the
chat id AND the MNGR_AGENT_ID that layout.py falls back to, which is the
whole point of it. Kept this branch's, dropped main's -- two autouse
fixtures for one job, with the narrower one able to mask the gap.
Gabriel Guralnick
Clear the agent identity via layout's env-var constants
aaefbe6 
Problem: the merge kept this branch's `_isolate_agent_identity` autouse
fixture and dropped main's `_isolate_own_chat_id`, which took with it the
file's only use of `layout.ENV_MINDS_CHAT_ID`. The kept fixture spells both
variables as string literals, even though `layout.py` owns them as constants
(lines 92-93) and `layout_test.py` beside it uses those constants throughout.
A rename would move `layout.py` and `layout_test.py` together and leave this
autouse fixture silently isolating nothing. It was also defined above the
module-level `layout` load it should be reading from.

Fix: clear `layout.ENV_MINDS_CHAT_ID` and `layout.ENV_MNGR_AGENT_ID` instead
of the literals, and move the fixture below the script loads, where main's
equivalent sat.
Gabriel Guralnick
Scope the reserved-name drift guard to the carriers it can reach
a274728 
Problem: the guard's docstring promised "Every carrier belongs here: one
left out is one free to drift", but a fourth carrier exists that the test
cannot import -- SEED_APP_RESERVED_NAMES in the mngr repo's
apps/minds_evals/imbue/minds_evals/data_types.py, which names
validate_service_name here as its source and still holds the drifted
frozenset({"localhost", "auth"}), the exact two-name value this branch fixed
in layout.py. A reader who trusts the promise stops looking, and the one
carrier the guard hides is the one that is actually wrong. The changelog's
"compares every carrier's set" said the same thing.

Fix: scope both claims to the carriers this repo holds, and name the mngr-side
copy and where it has to be corrected. The assertions are unchanged; they were
already right for the three reachable carriers.
Gabriel Guralnick
Excuse a sidecar the claiming app's own package runs
1b7996f 
Problem: the merge brought in main's `terminal_pty` app, and this branch's
sidecar-collision guard reds on it -- "terminal would claim 'terminal-pty'
(app terminal-pty)". The overlap is deliberate: `system/apps/terminal_pty/`
holds nothing but the manifest of the ttyd origin, and the program that runs
and registers it is the `terminal-pty` console script of
`system/apps/terminal/pyproject.toml`, so `program:terminal-pty` really is the
terminal's sidecar. The guard matched on names alone and could not tell that
from the unrelated-app case it was written to catch.

Fix: a prefix match is a collision only when the claimed program is not run by
the claiming app's own package. `_entry_point_manifest_paths` already mapped a
program's command to the package declaring its console script; that lookup is
now `_script_entry_points`, and `_packages_running_program` asks it which
packages run a program. A program with no block, or one running something other
than an app's entry point, is run by no package here and excuses nothing.
Gabriel Guralnick
Say what is out of reach about the mngr-side carrier
c45bff2 
Problem: the drift guard's docstring called the mngr repo's
SEED_APP_RESERVED_NAMES "out of reach" and said there is nothing the guard can
assert about it, naming only the mngr-repo path. A copy of that exact file is
tracked in this repo at system/vendor/mngr/apps/minds_evals/imbue/minds_evals/
data_types.py, still holding the drifted two-name value, so a reader who goes
looking finds the carrier in the tree and the claim does not hold. The paragraph
was written to stop a reader trusting an overstated promise, and repeated the
flaw.

Fix: say what is actually true -- the in-tree copy is a generated snapshot, so
only a fix upstream reaches the carrier, and an assertion here would pin the
snapshot rather than the set it was taken from. The assertions are unchanged.
Gabriel Guralnick
Grant the sidecar excuse only where the runner is decidable
d938816 
Problem: the excuse `_packages_running_program` hands out credits EVERY app
package declaring a console script with the claimed program's name, not the one
that actually runs it. Only one of several declarers can have its script on
PATH, and the supervisord config does not say which, so a package that merely
declares the name is credited too -- and silences the collision the guard exists
to catch. Probed against synthetic app trees: `pr` and `pr-review` as unrelated
apps report "pr would claim 'pr-review' (app pr-review)", but adding one unused
`pr-review = "pr_app.nothing:main"` line to `pr`'s pyproject makes the guard go
quiet.

Fix: credit a package only when it is the sole app package declaring the script,
which is what makes its script the only one of the apps' on PATH; several
declarers leave the runner undecided here, so nobody is credited and the
collision stands. `_script_entry_points`'s docstring no longer claims a
declaration says a package runs anything. `terminal-pty` is declared by
`system/apps/terminal` alone and is still excused.
Gabriel Guralnick
Name the duplicated console script rather than a collision it fakes
269d1a3 
Problem: the sidecar excuse now credits only the sole declarer of a program's
console script, which rests the guard on a property nothing checks -- that no two
app packages declare the same script name. It is a real property:
build_workspace.sh installs every manifest-carrying app as its own
`uv tool install -e`, and _tool_env.sh points all of them at one
UV_TOOL_BIN_DIR, so two declarers leave a single file there for whichever
installed last. When it breaks, the only thing that reds is
test_no_app_claims_another_apps_program_as_a_sidecar, with the wrong diagnosis:
adding `terminal-pty = "browser.nothing:main"` to system/apps/browser reports
"terminal would claim 'terminal-pty' (app terminal-pty)", two app names that are
fine and whose renaming fixes nothing, never naming the stray declaration.

Fix: test_no_two_app_packages_declare_the_same_console_script asserts the
property directly and fails with the script and the packages sharing it
("terminal-pty: browser, terminal" on that probe), so the collision line is read
correctly instead of chased. The `[project.scripts]` scan moves into
_scripts_by_package so the new guard and _script_entry_points share one read.
The sidecar guard's logic is unchanged.
Gabriel Guralnick
Say in the signature that one package is decided, not several
b455230 
Problem: `_packages_running_program` kept the plural name and the `set[Path]`
return from when it credited EVERY declarer of a program's console script, but
it now returns the sole declarer or nobody -- its own docstring opens "The app
package that runs a supervisord program". At the call site,
`owner_package not in _packages_running_program(...)` reads as "owner_package is
among the packages that might run this", which is the rule that was deliberately
removed, and the set type admits states the function can no longer produce.

Fix: `_package_running_program`, returning `Path | None`, compared with `!=`.
The three states it withholds the excuse for are unchanged; re-running the
duplicate-declaration probe still withholds it.
Gabriel Guralnick added 11 commits September 22, 2026 11:29
Gabriel Guralnick
Drop install_mngr's stale noexec rationale for injecting uv
e65dd2b 
Problem: the Run protocol docstring said a stub uv on PATH could not run
because tmp_path sits on a noexec /tmp. The pytest-executable-tmp plugin now
keeps tmp_path where files can run, so the stated reason no longer holds.
Fix: keep only the contract line (injected so a test can watch what it was
handed).
Gabriel Guralnick
Parametrize the select_temp_root unit tests
7ea4629 
Problem: four select_temp_root tests repeated one call-and-assert shape with
different inputs, through an unannotated helper.
Fix: one parametrized test with an id per case; the four cases and their
expectations are unchanged.
Gabriel Guralnick
Redirect pytest temp files to /var/tmp, not ~/.cache
56cf138 
Problem: when the default temp root could not run files, pytest-executable-tmp
moved temporary files to ~/.cache/pytest-executable-tmp. pytest nests tmp_path
several levels under that root, so a unix socket a test binds there (such as
system_interface's fake_supervisor) came to about 112-123 bytes on a workspace,
past Linux's 108-byte AF_UNIX limit. With a root of the same length on macOS,
15 of liveness_test.py's 21 tests errored with "AF_UNIX path too long". The root
also sat on the persistent /home/user volume.
Fix: redirect to /var/tmp. It is short enough that the same tests pass under it
(checked on macOS, whose path limit is shorter), and it is off the backed-up
home volume. README and changelog updated to match.
Gabriel Guralnick
Say only older workspaces mount /tmp noexec
218f3f6 
Problem: the pytest-executable-tmp README, the three pyproject dev-dependency
comments and the chat, system_interface and lib changelog entries said in the
present tense that a workspace container mounts /tmp noexec. This branch mounts
/tmp:exec for new containers, so only containers created before the change
still have noexec.
Fix: reword each one to name those older containers, which are the ones the
plugin exists for.
Gabriel Guralnick
Keep the outer PYTEST_DEBUG_TEMPROOT out of test_plugin's inner session
ef39945 
Problem: _run_inner_session passed the outer environment through and removed
only PYTEST_ADDOPTS. With PYTEST_DEBUG_TEMPROOT set on the outer run, the inner
plugin treated it as an explicit root and tmp_path landed there, not under the
TMPDIR the test set. test_a_session_whose_temp_root_can_run_files_uses_it_unchanged
then failed (reproduced: 1 failed, 1 passed).
Fix: remove PYTEST_DEBUG_TEMPROOT from the inherited environment too, before
the per-test overrides are applied. Both tests now pass with the variable set.
Gabriel Guralnick
Drop test_plugin's vestigial HOME override
72fba79 
Problem: both inner-session tests in pytest_executable_tmp/test_plugin.py set HOME
to a tmp_path dir. That kept the old ~/.cache fallback out of the real home; the
fallback is now /var/tmp and the plugin never reads HOME, so the override was dead
and implied a dependency that does not exist.
Fix: remove the HOME entry from both tests' env overrides.
Gabriel Guralnick
Name PYTEST_DEBUG_TEMPROOT in the plugin's stop message
a35ffac 
Problem: pytest-executable-tmp's stop message (plugin.py, _no_usable_root_message)
said to point TMPDIR or --basetemp at an executable directory. PYTEST_DEBUG_TEMPROOT
is the other explicit root the plugin refuses to replace, and pytest reads it ahead
of TMPDIR, so a run stopped for that reason was pointed at a setting with no effect.
Fix: name --basetemp, PYTEST_DEBUG_TEMPROOT and TMPDIR, in pytest's precedence order.
Gabriel Guralnick
Drop the section banners from pytest-executable-tmp's ratchet file
852a843 
Problem: system/libs/pytest_executable_tmp/test_pytest_executable_tmp_ratchets.py,
new on this branch, carried four `# --- <section> ---` divider banners copied
from the mngr_cli_contract ratchet file.
Fix: removed them; the test function names already say what each check covers.
Gabriel Guralnick
List pytest-executable-tmp in the system/libs index
ba40f5b 
Problem: system/libs/README.md indexes every support library under
system/libs/, but the new pytest_executable_tmp lib was missing from it.
Fix: add a one-line entry for it, matching the other entries.
Gabriel Guralnick
Say the tmp plugin loads only from the workspace venv
83089c8 
Problem: system/changelog/gabriel-wondrous-stingray.md said every pytest run in
the workspace loads pytest-executable-tmp, but the vendored mngr suite runs
from its own venv and does not.
Fix: scope the sentence to the workspace's own venv and name the vendored mngr
suite as not covered, matching the lib's README.
Gabriel Guralnick
Say in the browser changelog that its Fortress test uses tmp_path
048f623 
Problem: system/apps/browser/changelog/gabriel-wondrous-stingray.md described
only the agent-identity fix, not e812546's move of the fake Fortress binary
from a scratch dir in the app checkout to tmp_path.
Fix: add one sentence to the entry's paragraph naming that change and the
plugin it relies on.
@gnguralnick gnguralnick mentioned this pull request Sep 22, 2026
Draft
Gabriel Guralnick added 18 commits September 22, 2026 15:29
Gabriel Guralnick
Merge remote-tracking branch 'origin/main' into gabriel/wondrous-stin…
c5bd1a5 
…gray

Conflicts, each resolved as a union of independent additions:
- update_self_test.py: this branch's installed node_modules and main's mngr pin
  in the apply fixture.
- test_supervisord_layout.py: main's reformatting plus this branch's
  app_manifest.scope reader assertion.
- uv.lock: main's lock, re-locked to add pytest-executable-tmp.

Main stopped vendoring mngr, so the reserved-name drift guard's docstring and
this branch's changelog no longer describe a vendored copy.
Gabriel Guralnick
Drop the vendored-mngr claim from the branch changelog
1105706 
Problem: system/changelog/gabriel-wondrous-stingray.md still said the mngr repo is one "this one only vendors". Main stopped vendoring mngr (it is now a pinned git dependency), and the merge that brought that in missed this sentence.
Fix: say the copy lives in the mngr repo, outside this tree.
Gabriel Guralnick
Count mngr as built-in in AGENTS.md's escalation rule
b09fcef 
Problem: AGENTS.md's escalation paragraph defined built-in code as vendored (system/vendor/), the initial template commit, or an update-self merge. That covered mngr while it was vendored; since main made mngr a pinned git dependency, an mngr defect matched none of the clauses, and the PR channel it named (a template PR) cannot carry an mngr fix.
Fix: name mngr (installed from the pinned commit) in the definition, and note that an mngr defect goes in a report, since its fix is an mngr PR rather than a template one -- matching the /assist and submit-upstream-changes skills.
Gabriel Guralnick
Send an update pass's mngr defects to the report, not a template PR
7ec0abe 
Problem: update-self step 5d (.agents/skills/update-self/SKILL.md) routed every built-in defect with a fix through submit-upstream-changes. That was written while mngr was vendored; main now installs mngr from a pinned git commit, so submit-upstream-changes redirects an mngr fix to references/mngr-changes.md, which ends in a local PR or ticket -- the "recorded locally" end state 5d forbids. AGENTS.md was already corrected for this; 5d disagreed with it.
Fix: scope the submit-upstream-changes route to template code and say an mngr defect always goes in the report, matching AGENTS.md.
Gabriel Guralnick
Count mngr as built-in in /assist's classification test
5621e3c 
Problem: AGENTS.md defines built-in code as mngr, vendored, initial-commit or
update-self-merged, and defers to /assist step 3 for the exact test. Step 3A --
the test step 6 reports against -- listed only the file-based kinds, so an mngr
defect matched nothing there, though the report template already classifies it
as built-in.

Fix: step 3A now names mngr's code as built-in, pointing at B for why it is not
a file in this repo.
Gabriel Guralnick
Name the two names layout.py held instead of a stale count
c83f4c5 
Problem: the system changelog said layout.py's reserved-name copy held 'two of
the eight names'. Merging main's reservation of `agent` made that count wrong
relative to this branch's base.

Fix: name the two names it held (`localhost` and `auth`), which no later
reservation can make stale.
Gabriel Guralnick
Name the reserved names the scaffolder missed instead of a wrong count
490e9d3 
Problem: .agents/changelog/gabriel-wondrous-stingray.md said the scaffolder's
drifted reserved-name copy let "nine names" scaffold and then fail to register.
It was six at the branch's original base and seven against the merged main
(agent, app, env, host, owner, share, vm), and the number moves with every
reservation main adds.

Fix: name two of them (`share` and `host`) instead of counting, as the system
changelog already does for layout.py's copy.
Gabriel Guralnick
Route an mngr defect to the report in the mngr-changes reference
6aa7279 
Problem: .agents/skills/submit-upstream-changes/references/mngr-changes.md (from
origin/main) told an agent to describe an mngr change in the workspace's own PR or
ticket, while this branch's AGENTS.md rule and update-self step 5d send an mngr
defect to the /assist report and say a ticket is not a valid end state for one.
Fix: say that a defect found in mngr goes in the /assist step 6 report, keeping the
existing guidance for a change the template's own work needs.
Gabriel Guralnick
List github and agent among the labels an app name may not take
7337592 
Problem: build-app's pre-flight name guidance and the workspace-app-model
contracts spec enumerate the standalone-program labels an app name may not be
as share, app, owner, vm, host, env. forward_port.py and every other carrier
now also reserve github (this branch) and agent, so an agent choosing a name
from the pre-flight list could pick one the scaffolder refuses.
Fix: add agent and github (the github-sync program GitHub sync adds) to both
enumerations.
Gabriel Guralnick
Say why github is in RESERVED_APP_NAMES
f9b5939 
Problem: the comment above app_manifest's RESERVED_APP_NAMES explains every
member but github, which the branch added: github-sync is missing from its
program list, and since that program is not in supervisord.conf the
conf-driven guard it names does not cover it either.
Fix: name github-sync and say it is written only when GitHub sync is enabled.
Gabriel Guralnick
Merge remote-tracking branch 'origin/main' into gabriel/wondrous-stin…
ccaec00 
…gray

results-message.md: main dropped an item and renumbered the list; kept main's
numbering with this branch's Step 3b caveat added to item 6.
Gabriel Guralnick
Merge remote-tracking branch 'origin/main' into gabriel/wondrous-stin…
f4bf03d 
…gray

uv.lock: main's lock, re-locked to add pytest-executable-tmp.
Gabriel Guralnick
Install npm dependencies before a rollback rebuilds into a tree witho…
47ea018 
…ut them
Gabriel Guralnick
Rotate the host-backup events log and follow it across a rotation
a3bcd42
Gabriel Guralnick
Treat a temp root the probe cannot write to as unable to run files
bcc6576
Gabriel Guralnick
Name only the explicit temp-root settings in the executable-tmp stop …
7bb973a 
…message
Gabriel Guralnick
Move the built-in-issue report instructions into a shared reference
179e6b6
Gabriel Guralnick
Run the CI pytest suites on a noexec /tmp
3d425c4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@gnguralnick