Files
multi-agent-mux/.agents/reports/reviewer-grok-01/report-85982c2d.md
T

9.0 KiB
Raw Blame History

Review 85982c2d — PR #62, Issue #60 Linux follow-up

NOT PASS. Five new tests fail on this Mac before they call the query. The read-only open itself, measured on temporary databases, returns the committed rows and leaves the directory names as they were.

Reviewed only the snapshot /var/folders/q_/68sxgw3j073dgjg8wl9s08vw0000gn/T/mam-review/loop-20261019T000805Z-975c4c/1 (loop-20261019T000805Z-975c4c). Head 6a4e385a78c8a561d5cf3644336de6a352ea9271, tree afa2c55925f93820337b69699c670c0b0ca90980, base 1d7b7dab58a10fe86fd747b7290d47674ca93230 is an ancestor and is the control-plane HEAD. Eight commits, 11 files, +515/−31. git diff --check is clean. Snapshot porcelain is empty and the snapshot has no .mam. Control-plane porcelain is empty. No product file was edited. The source worktree was not run.

Commits, newest first: 6a4e385 (narrow fixture ends at its last drawn line), d17cd80 (auth is the binary; create uses auth-ok), 7803d47 (a -wal without its -shm is read from a private copy), 463817e (unconfirmed /exit wording), 614ab14 (debug paths cached per binary and home), ac5f115 (queries go through _ro_connect), 14d7a46 (2.x home placeholder), 4d49abb (DELEGATE_JOB_PYTHON for the u1e wrapper).

Host tools: control-plane venv Python 3.9.6, sqlite 3.51.0. bash -n accepts create_session.sh and stop_session.sh. py_compile accepts opencode.py, base.py, input_region.py, __main__.py, and tests/test_issue60_linux_fixes.py. The opencode binary was not launched. Linux was not executed here.

The five failures

tests/test_issue60_linux_fixes.py::test_a_query_reads_right_and_leaves_the_sidecars_alone with state clean fails for all five queries (verify_artifact, discover, _has_session, _descendants, residual_artifacts).

make("clean") closes a WAL connection and requires the directory to be exactly ["opencode.db"] (test_issue60_linux_fixes.py:154 and :173-181). The assertion is that listing, and it runs before the query. On sqlite 3.51.0 here, closing the last connection leaves opencode.db-wal at 0 bytes and opencode.db-shm at 32768 bytes. PRAGMA wal_checkpoint(TRUNCATE) then close leaves the same two names. The main file grows from 4096 to 8192 bytes on that close, which is the checkpoint into the main file. The pytest failure is ['opencode.db', 'opencode.db-shm', 'opencode.db-wal'] against ['opencode.db'].

That is the state a just-closed WAL database has on this host. _ro_connect then sees both sidecars and takes the mode=ro branch. A temporary database in that state returned the inserted row, and the three names and their sizes were the same after the connection closed.

The no-wal branch was measured by removing the sidecars after the checkpoint. _ro_connect (opencode.py:13-36) opened file:<path>?mode=ro&immutable=1, returned the row, and the directory stayed a single file with the same size and hash. A path containing a space behaved the same way.

The fixture has to remove the sidecars this sqlite keeps before it can assert a one-file listing and exercise that branch. As written, the five clean cases are red on the review host, so the related file is red here.

The other three states of the same test passed: both sidecars left by a writer that is gone, a -wal copied without its -shm, and a live writer. Each returned the expected rows (ses_main, the child id, or the residual session_v2 count the test asks for) and the directory names after the query matched the names before it.

The three open modes, on temporary databases

Both -wal and -shm present: mode=ro. The live writer's uncheckpointed row was visible. The -wal size and hash stayed the same. The -shm hash changed while the reader was open and was still the new hash after the reader closed (32768 bytes, same inode). The directory names stayed the three sidecar names. In a second sample the writer then inserted another row; wal_checkpoint(TRUNCATE) returned (0, 0, 0).

No -wal: immutable=1. Covered above. An existing -shm with no -wal was left in place, and the row in the main file was visible.

-wal present and -shm absent: the function copies the db and the wal into a mam-opencode- temporary directory, backups that copy into memory, and drops the temporary directory before returning the memory connection. Both rows that lived in the wal were visible. The original db and wal kept their size and hash. No -shm appeared beside the original.

db_rows_naming takes the optional connect and the opencode residual path passes _ro_connect (base.py:111-119, opencode.py:273-274). verify_artifact, _descendants, _has_session, and discover call _ro_connect. purge_artifacts still uses sqlite3.connect(db) for the 1.x DELETE statements (opencode.py:218). That line is unchanged in this range.

An immutable open that is issued after the no-wal check has already been recorded, and after a writer has then committed a new row into a new -wal, returns the rows that were in the main file. _ro_connect called once both sidecars exist returns the new row as well. The gap is between os.path.exists (opencode.py:21) and sqlite3.connect (opencode.py:25). A writer that already has both sidecars is read with mode=ro.

The other five items

The 2.x placeholder pattern is Ask anything(?:…|\.\.\.)(?: "[^"]*"?)? (opencode.py:76-79). extract_input_region applies it with re.fullmatch to the first non-blank line of the box only (input_region.py:76-86). Both new fixtures, wide and narrow, read as an empty box. The narrow fixture's example line contains a quote and does not end with a quote. A first line that is exactly Ask anything… or Ask anything… "<example>" (ellipsis or three dots) reads as empty. A first line with text after the closing quote is kept, and a later line with the placeholder shape is kept. input_placeholder stays '', which is what lib.sh greps. The other adapters' input_placeholder_pattern is None. Those tests passed.

auth_ok for opencode is shutil.which("opencode") or <HOME_DIR or HOME>/.opencode/bin/opencode (opencode.py:317-321). Keys and auth.json are outside that check. __main__.py auth-ok exits 0 or 1 from the adapter, and exits 1 for an unknown agent. create_session.sh calls auth-ok opencode through _delegate_py_bin. The diff among adapters is only opencode.py. Claude, agy, grok, codex, and hermes keep their previous auth_ok bodies. test_adapter_auth_ok, the binary-only cases, and test_create_decides_opencode_auth_without_running_it passed. The create run records no --version call.

_DB_PATHS keys are (binary, home, data home) (opencode.py:139-161). A hit returns the stored path. A failed debug paths is not stored. The new tests replace the dict for each test (test_issue60_linux_fixes.py:48-50). test_debug_paths_runs_once_per_home and test_a_failed_debug_paths_is_not_remembered passed. The same key keeps its path for the life of the process, including after the binary at that path is replaced.

stop_session.sh still sends the exit key, waits, and runs kill-session when the session is still there (stop_session.sh:242-250). The unconfirmed-delivery line now says it is waiting for the session to exit. test_the_graceful_stop_reports_what_happened passed for a pane that exits on /exit and for one that stays up. The staying-up case still contains still alive → kill-session.

test_u1e_a_dry_run_record_is_not_an_open_job passes DELEGATE_JOB_PYTHON=sys.executable. That test passed.

Suite

Pytest used the control-plane venv, PYTHONDONTWRITEBYTECODE=1, -p no:cacheprovider, and --basetemp /tmp/mam859-85982c2d-pybase. Inherited HERDR_*, MAM_*, MQTT_*, SKS_*, DELEGATE_JOB_LOGS_DIR, HOME_DIR, CLAUDE_PROJECT_DIR, CLAUDE_CONFIG_DIR, CLAUDE_CODE_TMPDIR, CODEX_HOME, PYTHONPATH, WORKSPACE_ROOT, XDG_STATE_HOME, XDG_CONFIG_HOME, XDG_CACHE_HOME, and XDG_DATA_HOME were dropped.

5 failed, 42 passed in 59.38 s. The failures are the five clean cases named above. The rest of tests/test_issue60_linux_fixes.py, tests/test_a4_adapter_contract.py::test_adapter_auth_ok, and tests/test_issue50_dispatch.py::test_u1e_a_dry_run_record_is_not_an_open_job passed. No error from the session guards.

The user's background service stayed up. Its pid is 23630 and the command is opencode serve --service. The real database was not opened. Its mtime stayed 1792367390. ~/.config/opencode/service.json stayed 1792246838 and ~/.local/state/opencode/service.json stayed 1792365227. The real opencode.log mtime stayed 1792368889 and its size stayed 2636652. The herdr config mtime stayed 1784456394.

After the report, snapshot porcelain is still empty, the snapshot still has no .mam, and control-plane porcelain is still empty at 1d7b7dab. The herdr config mtime is still 1784456394. The opencode database mtime is still 1792367390, the opencode log mtime is still 1792368889 and its size is still 2636652, and service pid 23630 is still opencode serve --service.

[VERDICT: NOT PASS]