[AbysmalBiscuit]

Agent summary

Problem

On the release PR, the rules cache test concurrent_refreshes_leave_one_whole_index failed on Windows with database is locked. The cache uses SQLite's rollback journal, so a refresh's write lock blocks every reader. Two refreshes back to back kept a reader waiting longer than its 2 s busy timeout. Hooks read this cache, so a hook can fail the same way while refreshes pile up.

In the same partition, launch_non_blocking_returns_before_readiness_then_status_flips failed with "server never started listening" and left no record of why.

Change

• The cache switches to WAL journaling, so a reader reads the last committed index and never waits on a refresh. a_refresh_in_progress_does_not_hold_up_a_reader holds a refresh open and reads through it.
• The launch test prints the server's log when it fails, so the next failure shows whether Python failed to bind or never started.

Proof

1. test a_refresh_in_progress_does_not_hold_up_a_reader, failing on the test-only commit and passing with WAL
2. test launch_non_blocking_returns_before_readiness_then_status_flips prints the log in its failure message

---

🤖 Generated with Claude Opus 5.5 via Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-16T16:23:47.563431Z	27dc60b	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

⚡ Grok Review

Reviewed commit: 27dc60b991

❌ Patch is incorrect: 1 finding.

The WAL switch leaves the rules cache on SQLite's default locking, so a refresh still blocks readers and the Windows timeout failure remains.

Enable WAL locking so refreshes stop blocking readers

crates/devkit-rules/src/cache.rs#L88-L96

RulesCache::open still uses rusqlite's default rollback-journal locking. begin_write takes a RESERVED lock that blocks readers for the whole refresh, so concurrent_refreshes_leave_one_whole_index can still fail with database is locked after the 2 s busy timeout, and hooks can fail the same way. PRAGMA journal_mode=WAL only changes the journal; it does not switch the connection to WAL locking. Call Connection::pragma_update(None, "journal_mode", "WAL") or execute_batch("PRAGMA journal_mode=WAL;") before the schema statements so readers see the last committed index while a refresh writes.

        let conn = Connection::open(&path).context("opening the rules cache")?;
        conn.pragma_update(None, "journal_mode", "WAL")
            .context("switching the rules cache to WAL")?;
        conn.execute_batch(
            "CREATE TABLE IF NOT EXISTS meta (key TEXT PRIMARY KEY, value TEXT NOT NULL);
             CREATE TABLE IF NOT EXISTS rules (
                 id INTEGER PRIMARY KEY,
                 body TEXT NOT NULL
             );",
        )
        .context("creating the rules cache schema")?;

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 27dc60b991

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Superseded by the review of f95e1bf

[github-actions]

⚡ Grok Review

Reviewed commit: f95e1bfd5a

✅ No issues found.

The WAL switch and the launch-test log both leave existing callers and tests working: readers still see one committed index, and the assertion only adds the server log to a failure message.

[github-actions]

⚡ Grok Review

Reviewed commit: ae229eddd3

❌ Patch is incorrect: 1 finding.

The rules cache now writes WAL while every hook still opens it read-only, which cannot use a WAL file. That breaks every cached read after the first refresh.

Open the rules cache read-write so WAL reads work

crates/devkit-rules/src/cache.rs#L184-L187

After the first refresh, write sets journal_mode=WAL and leaves -wal/-shm beside the cache. Every hook and command then calls RuleCache::read and meta, which open the file with SQLITE_OPEN_READ_ONLY. SQLite cannot open a WAL database that way: it must create or write the -shm file, so the open fails with a disk I/O error ("unable to open database file") or SQLITE_CANTOPEN. CachedSource::read returns that error, so a write hook injects no rules and devkit rules query/stats/context fail even though a committed index is on disk. The new test never hits this path: it opens a second read-write connection, which can read WAL. Open the cache read-write (or SQLITE_OPEN_READWRITE plus SQLITE_OPEN_URI with mode=ro is not enough), then set PRAGMA query_only=ON so a reader still cannot change rows. On Windows, also give the directory write permission and do not delete -wal/-shm while a reader is open.

        let conn = Connection::open_with_flags(
            &self.path,
            OpenFlags::SQLITE_OPEN_READ_WRITE | OpenFlags::SQLITE_OPEN_NO_MUTEX,
        )
        .with_context(|| self.reading())?;
        conn.pragma_update(None, "query_only", "ON")
            .with_context(|| self.reading())?;

[AbysmalBiscuit]

On the Grok review of 27dc60b: that was the test-only commit, before WAL was switched on. The suggested code names a RulesCache::open that does not exist. Setting journal_mode to WAL is itself what moves SQLite to WAL locking, and the fix commit (now ae229ed) does that in RuleCache::write. Grok's review of the fix commit reported no issues, and the review was already dismissed.

False positive. Since SQLite 3.22, a read-only connection opens a WAL database whenever it can create the -shm file, and the state directory is writable. RuleCache::read and meta open read-only after a WAL write in round_trips_the_index, a_refresh_in_progress_does_not_hold_up_a_reader and the cached_source tests, and all of them pass on ubuntu, macOS and Windows in run 38936335255.