#206: pin the read-only open with a test that actually fails without it
CI / build (pull_request) Successful in 1m18s
CI / contract (pull_request) Successful in 1m44s

setReadOnly(true) is the whole thing keeping fleetd out of the operator's
live 841MB opencode.db, and no test failed when it was removed.

The obvious test does not work. Making the database file unwritable and
checking the read still succeeds passes either way, because SQLite silently
downgrades a read-write open of an unwritable file to read-only. I wrote that
test, watched it pass with the flag removed, and threw it away.

What works: extract a package-private openReadOnly(), then ask that connection
to INSERT and require the refusal. Watched red with the flag removed, green
with it restored.

Also switches the test's INSERT helper to a PreparedStatement -- hand-escaped
SQL in a test is a pattern that gets copied into main code.
This commit is contained in:
Dai Ha
2026-08-31 21:46:07 +07:00
parent 3743789e8d
commit a052975420
2 changed files with 61 additions and 15 deletions
@@ -54,6 +54,23 @@ final class OpenCodeSessionDiscovery {
this.databasePath = storageRoot.resolve("opencode.db");
}
/**
* A connection to {@link #databasePath} opened with SQLite's {@code SQLITE_OPEN_READONLY}
* flag: it never creates the file, never writes, and never touches WAL or journal mode.
* opencode may be running and writing this database concurrently, and this class must never
* disturb it.
*
* <p>Package-private so a test can hold the connection and prove it refuses a write. That is
* the only way to pin this property: making the file unwritable does <em>not</em> work,
* because SQLite silently downgrades a read-write open of an unwritable file to read-only, so
* such a test passes whether or not the flag is set.
*/
Connection openReadOnly() throws SQLException {
SQLiteConfig config = new SQLiteConfig();
config.setReadOnly(true);
return config.createConnection("jdbc:sqlite:" + databasePath);
}
/**
* The opencode session id whose row references {@code directory} (the worker's cwd), or
* {@code null} when no row matches yet. When several rows share the directory — e.g. repeated
@@ -81,14 +98,8 @@ final class OpenCodeSessionDiscovery {
}
return null;
}
// SQLiteConfig.setReadOnly opens with the SQLITE_OPEN_READONLY flag: never creates the
// file, never writes, never touches WAL/journal mode. opencode may be running and
// writing this database concurrently (WAL mode) — this connection must never disturb it.
String url = "jdbc:sqlite:" + databasePath;
SQLiteConfig config = new SQLiteConfig();
config.setReadOnly(true);
String sql = "SELECT id FROM session WHERE directory = ? ORDER BY time_updated DESC LIMIT 1";
try (Connection connection = config.createConnection(url);
try (Connection connection = openReadOnly();
PreparedStatement statement = connection.prepareStatement(sql)) {
statement.setString(1, directory);
try (ResultSet rows = statement.executeQuery()) {
@@ -7,6 +7,8 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.sql.Statement;
import static org.junit.jupiter.api.Assertions.*;
@@ -26,14 +28,20 @@ class OpenCodeSessionDiscoveryTest {
*/
static void writeRecord(Path root, String id, String directory, long timeUpdated) throws Exception {
Path db = root.resolve("opencode.db");
try (Connection connection = DriverManager.getConnection("jdbc:sqlite:" + db);
Statement statement = connection.createStatement()) {
statement.execute("CREATE TABLE IF NOT EXISTS session ("
+ "id TEXT PRIMARY KEY, directory TEXT, time_updated INTEGER)");
statement.execute("INSERT INTO session (id, directory, time_updated) VALUES ("
+ "'" + id.replace("'", "''") + "', "
+ "'" + directory.replace("'", "''") + "', "
+ timeUpdated + ")");
try (Connection connection = DriverManager.getConnection("jdbc:sqlite:" + db)) {
try (Statement statement = connection.createStatement()) {
statement.execute("CREATE TABLE IF NOT EXISTS session ("
+ "id TEXT PRIMARY KEY, directory TEXT, time_updated INTEGER)");
}
// Bound parameters, not string interpolation: the class under test uses a
// PreparedStatement, and a hand-escaped INSERT here is a pattern someone copies out.
try (PreparedStatement insert = connection.prepareStatement(
"INSERT INTO session (id, directory, time_updated) VALUES (?, ?, ?)")) {
insert.setString(1, id);
insert.setString(2, directory);
insert.setLong(3, timeUpdated);
insert.executeUpdate();
}
}
}
@@ -89,6 +97,33 @@ class OpenCodeSessionDiscoveryTest {
assertNull(discovery.sessionIdForDirectory(" "));
}
/**
* The one line standing between fleetd and writing the operator's live {@code opencode.db} —
* 841MB, with a running opencode writing it — is {@code config.setReadOnly(true)} in
* {@link OpenCodeSessionDiscovery#openReadOnly()}. Delete it and every other test in this class
* still passes, so this is the test that guards it.
*
* <p>It asks the connection to write, and requires a refusal. The obvious alternative — make
* the database file unwritable and check the read still works — proves nothing: SQLite silently
* downgrades a read-write open of an unwritable file to read-only, so that test passes either
* way. It was tried and watched pass with the flag removed.
*/
@Test
void theDatabaseIsOpenedReadOnly(@TempDir Path root) throws Exception {
writeRecord(root, "ses_aaa", "/w/a", 1000L);
try (Connection connection = new OpenCodeSessionDiscovery(root).openReadOnly();
Statement statement = connection.createStatement()) {
SQLException refused = assertThrows(SQLException.class,
() -> statement.executeUpdate("INSERT INTO session (id, directory, time_updated) "
+ "VALUES ('ses_zzz', '/w/z', 1)"),
"the connection must REFUSE a write — opencode is writing this database live");
assertTrue(refused.getMessage().toLowerCase().contains("readonly")
|| refused.getMessage().toLowerCase().contains("read-only"),
"the refusal must be about read-only, not some other error: " + refused.getMessage());
}
}
@Test
void aCorruptDatabaseFileYieldsNullWithoutThrowing(@TempDir Path root) throws Exception {
// A file at opencode.db that is not a SQLite database at all — the open/query must fail