The status that nothing ever wrote (Greenhouse engine dev log)

I’m building Greenhouse, a desktop app that enforces a creative-process cadence - capture ideas, let them sit, rotate between projects, never delete anything. The rules engine is Rust, and an earlier session had already shipped it: 22 passing tests, clean build, issue closed.

So when the next issue came up - “define the per-item state model” - it looked done before I started. The model was right there in state.rs. Five status values, a derived cooldown, a maturity flag. Every field the issue asked for.

I almost closed it as a duplicate. Instead I read the code.

Investigation

Item status was an enum:

pub enum ItemStatus {
    FreshIdea,
    MatureIdea,
    Active,
    Vaulted,
    Released,
}

MatureIdea is the interesting one. An idea starts fresh and becomes “mature” after a timer, at which point you’re meant to decide: work on it, or vault it. A status value seems reasonable.

Except nothing ever set it.

I grepped the whole crate. The promotion path went FreshIdea → Active directly. Nothing transitioned anything to MatureIdea. And the daily dashboard, the thing that surfaces “ideas due for a decision,” queried only fresh ideas:

let all_ideas = db.list_items_by_status(&ItemStatus::FreshIdea)?;

Here’s the trap. If any future code did set an item to MatureIdea - and the enum invites it, because it’s sitting right there - that item would vanish. The active worklist excludes it. The ideas-due query doesn’t pull it. It’d be in the database, with a valid status, and invisible everywhere.

Maturity was modeled twice: once as a derived boolean computed from a timestamp, once as a stored status. The two never had to agree, so eventually they wouldn’t.

Then I found the second one. The function that reads a status back out of SQLite:

fn str_to_status(s: &str) -> ItemStatus {
    match s {
        "fresh_idea" => ItemStatus::FreshIdea,
        // ...
        _ => ItemStatus::FreshIdea,  // anything else
    }
}

Any unrecognized string becomes a fresh idea. In most apps that’s a lazy-but-harmless default. In a no-deletion app it’s a quiet disaster: a corrupted or half-migrated Released row silently turns back into a fresh idea and re-enters the capture pipeline. The one promise the product makes - nothing is lost or altered behind your back - undone by a fallthrough arm.

The fix

Pick one model. Maturity is derived, full stop. The engine is a state calculator, not a gatekeeper - it answers “what’s true right now” from timestamps. So a stored MatureIdea had no reason to exist. I removed it. A mature idea is now just a fresh idea whose computed maturity_reached is true.

And the deserializer stops guessing:

fn str_to_status(s: &str) -> rusqlite::Result<ItemStatus> {
    match s {
        "fresh_idea" => Ok(ItemStatus::FreshIdea),
        // ...
        other => Err(/* unknown item status: ... */),
    }
}

Unknown status fails loud. For a no-deletion product, loud is the correct behavior.

Postscript: the off-by-one-second that wasn’t consistent

With the model cleaned up, I moved on to the timer issues - cooldown and maturity - expecting more of the same “already built, just verify” work. They were built and tested. But comparing them side by side, they disagreed on one tiny thing: the boundary.

Cooldown asked: is this still locked?

let is_cooling = now < until;   // available the instant we hit `until`

Maturity asked: has enough time passed?

now >= created_at + idea_maturity_secs   // mature the instant we hit the threshold

One was strict, one was inclusive. At the exact second the timer expires, cooldown said “available now” while maturity said “mature now” - opposite conventions for the same idea, “a duration has elapsed since an event.” Both specs said the same word: after. Available after the cooldown. Mature after the timer.

So I made them agree on the strict reading. The state flips strictly after the full duration elapses; at the exact boundary tick, you’re still in the “before” state for one more second:

let is_cooling = now <= until;                       // cooling through the boundary
now > created_at + idea_maturity_secs                // mature strictly after

One second on a 7-day timer changes nothing a user will ever notice. But a rules engine’s whole job is predictable timing, and two timers using different boundary math is the kind of thing that bites later - someone writes “the cooldown and maturity rules are the same shape,” and they’re quietly not. Now they are. Each change came with a test pinning the exact boundary, so the next person can’t drift them apart by accident.

A stat that has to earn its place

The touch mechanism - the “I worked on this” action that starts a cooldown and optionally takes a handoff note - was already built. But its issue had a quieter line I’d skipped past: track whether projects with handoff notes get finished more often, and surface that gently.

The data was already there. Every touch stores its note. So I could compare: of the projects that reached a conclusion, did the ones with notes finish more often than the ones without?

The easy version writes itself - count finishes in each group, divide, show the percentage. The trap is showing it too early. With two finished projects, “100% of note-takers finish!” is not a finding, it’s noise wearing a lab coat. And this is a tool about being honest with yourself; a fake insight is worse than none.

So the stat gates itself:

pub fn has_meaningful_sample(&self) -> bool {
    self.concluded_with_notes >= MIN_SAMPLE_PER_GROUP
        && self.concluded_without_notes >= MIN_SAMPLE_PER_GROUP
}

And the dashboard only carries it when it’s earned:

let insight = insight::handoff_insight(db)?;
let handoff_insight = insight.has_meaningful_sample().then_some(insight);

Below the threshold, the field is None and the UI shows nothing. “Surface gently” turned out to mean “mostly don’t surface at all.” The feature’s main job is knowing when to stay quiet.

One more small decision: what counts as a “project that concluded”? I went with terminal states only - Released (finished) or Vaulted (shelved). An Active project hasn’t chosen yet, so counting it as “not finished” would drag every rate toward zero and make notes look useless. Outcomes only get compared once they’re actually outcomes.

The sort that ran backwards

Greenhouse’s daily dashboard shows an “active worklist” - the projects you can pick up today. The whole point of the app is rotation: it nudges you toward work you’ve been neglecting and, through the cooldown, away from whatever you just touched. So the worklist should put your most-neglected project at the top.

The code sorted it like this:

ORDER BY last_touched DESC NULLS LAST

That’s most-recently-worked first, never-touched last. Exactly backwards. The project you’ve ignored for a month sits at the bottom; the one you just finished cooling off from sits at the top. The comment three lines up even said “neglected items rise” - the code was doing the opposite of its own description.

Nothing caught it because the only test checked which items were in the list, never their order:

assert_eq!(state.available_worklist.len(), 1);
assert_eq!(state.available_worklist[0].id, "available");

One item in, one item out - order is invisible with a sample size of one. Sorts are the classic place this hides: the list looks right, it’s full of the right things, and it’s upside down.

The fix is one line, plus a test that actually pins the sequence:

ORDER BY last_touched ASC NULLS FIRST
-- → [never-touched, longest-neglected, ..., most-recently-worked]

assert_eq!(order, ["never", "old", "mid", "recent"]);

If a sort matters, assert the order, not just the contents.

The sort that wasn’t sorting

That backwards sort had a sequel, and it’s the same line of SQL wearing a different disguise.

Greenhouse runs a daily “Rescue or Keep” review: once a day it surfaces one vaulted project that’s sat long enough to reconsider. The rule is “show me the one that’s been dormant longest.” The code grabbed all vaulted items and took the first mature one:

let vaulted_items = db.list_items_by_status(&ItemStatus::Vaulted)?;
let vault_review_item = vaulted_items
    .iter()
    .map(|item| derive_item_state(item, config, now))
    .find(|s| s.maturity_reached);

“First mature one” only means something if the list is ordered. And list_items_by_status orders by the same column as before - last_touched. For active projects that’s meaningful. For vaulted ones it’s nothing: a project in the vault isn’t being touched, so its last_touched is NULL and stays NULL.

So every vaulted row sorts on the identical value, NULL, lands in one NULLS LAST bucket, and SQLite hands them back in whatever order it pleases. .find() picks the first mature item off an arbitrary pile. The backwards sort at least sorted - this one had no sort at all. A key that’s constant across your rows is the same as no key.

It looked stable on my machine with a few rows. Same trap as last time, one notch quieter: “arbitrary” doesn’t promise “wrong every run,” it promises “no promise.” Add a row or run on a different SQLite build and the daily nudge silently points somewhere else.

The value I wanted was already in the schema - vaulted_at, the moment the item entered the vault. Longest-dormant is just the smallest one. So vaulted items got their own query instead of borrowing the worklist’s:

ORDER BY vaulted_at ASC NULLS LAST

Oldest vaulting first; the .find(maturity_reached) on top now yields the longest-dormant mature item by construction. The test inserts three mature items out of order plus a too-recent one and pins which one surfaces - because, same as the backwards sort, an order bug is invisible until a test asserts the order.

Same query, third lesson pulled off it: ORDER BY on a column that’s NULL for exactly the rows you’re selecting isn’t sorted, it’s “whatever the engine returns” - and it’ll look deterministic right up until it isn’t.

The constraint that wasn’t constraining

One more, and it’s my favorite kind: a safety rail that looks installed but isn’t bolted to anything.

The schema declares that every touch belongs to a real item:

CREATE TABLE touches (
    id        TEXT PRIMARY KEY,
    item_id   TEXT NOT NULL REFERENCES items(id),
    ...
);

That REFERENCES items(id) reads like a guarantee: you can’t log a touch for an item that doesn’t exist. Except SQLite ships with foreign keys off by default. The enforcement only turns on if you run PRAGMA foreign_keys = ON on the connection - and nothing did. So the constraint was documentation, not a rule. You could insert a touch pointing at a ghost item and the database would happily take it.

It hadn’t bitten yet because the one code path that writes touches checks the item exists first. But that’s defense resting entirely on application code remembering to be careful - exactly what the database constraint is supposed to make unnecessary.

The fix is one line, set on every connection at open:

self.conn.execute_batch("PRAGMA foreign_keys = ON;")?;

And a test that tries to insert an orphan and expects a rejection:

let result = db.insert_touch("t1", "ghost-item", 1000, Some("note"));
assert!(matches!(result, Err(crate::EngineError::Db(_))));

The declaration was right. The enforcement was missing. Writing it down isn’t the same as turning it on.

Half a decision

A repo review turned up something none of the earlier finds caught, because it doesn’t fail on any single run - it only fails on a run that stops in the middle. Greenhouse’s “vault an item” action is two SQL statements:

db.update_item_status(item_id, &ItemStatus::Vaulted)?;
db.set_vaulted_at(item_id, now)?;

Each one commits the instant it runs - there’s no transaction around the pair. If the process dies between them (a crash, a killed app, SQLITE_BUSY from something else touching the file), the first statement survives and the second doesn’t. The item is now Vaulted with vaulted_at = NULL.

That NULL isn’t cosmetic. The vault-review query orders vaulted items by vaulted_at to find “the one that’s been dormant longest” - the same ordering bug this arc already covered once, except this time the column itself is missing, not just unsorted. A NULL sorts last and never reads as mature, so the item is vaulted forever and never comes back up for Rescue-or-Keep. Shelved, silently, for good. In an app whose entire premise is “nothing is ever truly gone,” that’s the one outcome the design isn’t supposed to allow.

Second half of the same story: even on a clean run where both statements succeed, nothing told project.md that anything had happened. The mirror file sitting in the project’s folder kept reading **Status:** Active after the item was vaulted - the database said one thing, the file on disk said another, and the mirror’s own doc comment promises “always reflects current state.” It didn’t.

The DB wrapper holds its connection by value behind &self, and every write method is already fn foo(&self, ...). rusqlite has a transaction API for exactly that shape - unchecked_transaction(&self), not the checked transaction(&mut self) - so wrapping the pair cost nothing beyond a small helper:

pub fn with_tx<T>(&self, f: impl FnOnce() -> crate::Result<T>) -> crate::Result<T> {
    let tx = self.conn.unchecked_transaction()?;
    let result = f()?;
    tx.commit()?;
    Ok(result)
}

Both statements now run inside one closure passed to with_tx. A crash mid-way rolls back instead of leaving half a decision recorded.

The mirror regen has a sharper trap hiding in it. The obvious fix is “commit the transaction, then regenerate project.md, and return an error if that fails too.” But if that error propagates and the caller retries the whole vault_item call, the retry re-runs set_vaulted_at(now) with a later timestamp - silently pushing the dormancy clock forward on an item that was already vaulted. A safety mechanism would be resetting its own timer because a file write hiccuped. So the mirror regen is deliberately best-effort: log the failure, don’t fail the call. The database is the source of truth; the markdown file is a reconstructible view of it, not the other way around.

Same treatment went to the two other multi-write spots in the engine - logging a touch, and promoting an idea to an active project (three to five statements: status, stage, folder name, and an optional touch) - plus a UNIQUE index on the column that assigns each item its folder, which until now was only ever checked by application code before inserting, never enforced by the database itself.

Every earlier bug in this arc was “the code says one thing and does another” in a single, deterministic run - you could find it by reading closely once. This one only exists in the gap between two statements, and only some of the time. Tests can’t crash-inject a process mid-transaction to prove it, so what I could actually verify was the mechanism: a transaction that returns Err rolls back, one that returns Ok commits, and a real timestamp gets recorded when it succeeds. Some fixes you verify by testing the failure. Some you verify by testing that the failure mode no longer has a seam to fit through.

The rate computed in two languages

The same review that found the half-committed vault write also found a smaller, quieter version of the same family of bug - not “the code contradicts itself,” but “the code and the comments describing it contradict each other,” three separate times.

Here’s the sharpest one. Greenhouse tracks a soft insight: do projects with a handoff note get finished more often than ones without? The Rust side computes this and exposes a method:

pub fn notes_help(&self) -> bool {
    match (self.finish_rate_with_notes(), self.finish_rate_without_notes()) {
        (Some(with), Some(without)) => with > without,
        _ => false,
    }
}

Except impl methods don’t cross IPC. Only plain fields serialize. So the four raw counts made it to the frontend, and the boolean everyone actually wanted didn’t - which meant the dashboard needed the verdict, and someone had to write it again:

export function notesHelp(insight: HandoffInsight): boolean {
  const withRate = rate(insight.finished_with_notes, insight.concluded_with_notes);
  const withoutRate = rate(insight.finished_without_notes, insight.concluded_without_notes);
  if (withRate === null || withoutRate === null) return false;
  return withRate > withoutRate;
}

Same comparison, same edge cases, two files, two languages. It was correct on the day it was written - both sides were even careful about the same null-guard. But “correct today” was never the risk. The risk was the day someone changes the Rust rule - adds a minimum-difference threshold, changes the tie-break - and only edits one of the two places, because nothing forces them to remember the other one exists.

This one’s fixable at the root instead of just patched: make Rust compute it once and ship the answer.

pub struct HandoffInsight {
    // ...four counts...
    pub notes_help: bool,   // computed once, serialized, done
}

derive_handoff_insight sets it after tallying; the TypeScript function and its private rate() helper are deleted outright, and the dashboard reads daily.handoff_insight.notes_help like any other field. There’s no longer a second implementation to drift - not “kept in sync,” genuinely single-sourced.

Two smaller cousins

Same review turned up two more instances of “the description and the behavior disagree”:

config.yaml has always carried a seed_quota - the PRD’s own words for it are “a dashboard nudge toward a starting batch.” The low-inventory warning that’s supposed to read that number instead hardcoded LOW_INVENTORY_THRESHOLD = 3. Editing your config file to raise your quota to 10 did nothing; the nudge still fired at 3 regardless. Wiring it to the real config value is a one-line fix - but the fact that a config value with a written-down purpose sat there unread for four issues’ worth of engine work is exactly the “declared but not enforced” shape from a few sections back, just wearing a settings file instead of a PRAGMA.

And a doc comment on the promote action claimed it “starts cooling immediately.” It doesn’t, and hasn’t since an earlier issue deliberately flipped that default - there was even a passing test proving the opposite. The comment just never got told.

Not every one of these needed the same fix. The config knob got wired up. The comment got corrected. But a third field - vault_review_cadence_days, also parsed, also unused - I left alone on purpose: making it actually do something would mean inventing a way to remember “when was this last shown,” which the app doesn’t track anywhere, and approximating it with today’s date would silently skip a nudge in a product whose entire model is never blocking, never skipping, just gently reminding. Sometimes the honest fix for an unused field is a comment explaining why it’s still unused, not a feature nobody asked for.

A settings file that could brick the app before the window opened

The same review found a third one, and this one’s the sharpest of the batch, because it’s not about the code lying to itself - it’s about a code path that only exists to handle “the file on disk isn’t what we expect,” and it didn’t actually handle it.

Greenhouse persists one small file outside the vault: settings.json, holding the path to wherever the user’s vault lives. It’s read once, at startup, before the window opens:

pub fn load(config_dir: &Path) -> crate::Result<Self> {
    let path = config_dir.join(SETTINGS_FILE);
    if !path.exists() {
        return Ok(Self::default());
    }
    let content = std::fs::read_to_string(&path)?;
    let settings = serde_json::from_str(&content)?;
    Ok(settings)
}

Missing file is handled - a fresh install has no settings.json, and that’s fine, it means “show onboarding.” But a malformed file isn’t the same as a missing one, and the code treated them as if they were interchangeable failure modes when only one of them actually was. A truncated write from a crash mid-save, or a curious hand-edit, produces a file that exists but doesn’t parse - and that ? propagates the parse error straight out.

Here’s the part that made it sharp instead of just annoying: this function is called from inside Tauri’s .setup() hook.

.setup(|tauri_app| {
    app::load_startup_settings(tauri_app.handle())?;
    Ok(())
})

A ? inside a command handler just rejects that one call - the rest of the app keeps running, the user sees an error banner. A ? inside .setup() is a different animal entirely: it aborts startup. Not “the dashboard fails to load.” Not “onboarding shows an error.” The window never opens. The only way back in was finding settings.json on disk and deleting it by hand, outside the app - a debugging step nothing in the UI could tell you to take, because the UI never got the chance to exist.

Same shape of question as always: which failures are “this specific thing is broken” versus “we can’t trust the input, start over”? A missing file already answered that question - default and move on. A corrupt file deserves the same answer, for the same reason: the app has no way to distinguish “the user’s disk has a real problem” from “a write got interrupted,” and defaulting is safe either way, since the user just re-onboards into the vault they already have.

What doesn’t get the same treatment: the file-read step above the parse. An IO error there - permissions, a missing volume, an actual disk fault - is a different class of problem, one the user needs to see, not one the app should silently paper over. So the catch is scoped narrowly, to the parse step only:

let content = std::fs::read_to_string(&path)?;   // IO error: still propagates
match serde_json::from_str(&content) {
    Ok(settings) => Ok(settings),
    Err(e) => {
        eprintln!("Settings::load: corrupt {SETTINGS_FILE} ({e}), falling back to defaults");
        Ok(Self::default())                       // parse error: falls back
    }
}

The review asked a fair question in the other direction: does the equivalent function for the vault’s config file - config.yaml, not settings.json - have the same bug? It doesn’t, but not because anyone thought about it up front. Config::load is never called from .setup() at all, only from individual command handlers, so its parse errors were already just rejected promises, already just dashboard errors. The fix there wasn’t a fix, it was a test that pins down that this stays true - the kind of thing that’s obviously correct today and silently stops being correct the day someone wires config loading into startup for a good reason and forgets this constraint existed.

“Handle the missing case” and “handle the corrupt case” look like the same problem until you ask what happens next - and in this one, “what happens next” was the whole app failing to start.

The command that would open anything the webview asked it to

The review’s last finding in this cluster wasn’t a subtle logic bug. It was the opposite: a capability sitting in plain sight, doing exactly what it was written to do, and what it was written to do was too much.

Earlier in this arc, I routed folder-opening through a Rust command instead of the opener plugin’s JS API, specifically to get around an empty scope allowlist that was silently denying every open call. The fix worked. What I didn’t sit with at the time: the scope allowlist wasn’t just in the way, it was the only thing checking that the path being opened was one the app actually meant to open.

#[tauri::command]
pub fn open_folder(app: AppHandle, path: String) -> Result<(), String> {
    open_dir(&app, path)
}

That’s the whole command. A string comes in from the webview, and it goes straight to the OS’s default-handler launcher - no check that it’s inside the vault, no check that it’s even a real path on this machine. open_path is open(1) with extra steps: give it a .app bundle, a script with the right extension, a file:// URL to something unexpected, and the OS decides what “opening” it means. This is, quantitatively, the single biggest capability the app hands to its own webview - bigger than any SQL query, because SQL queries stay inside a schema and this doesn’t stay inside anything.

It only had one caller. The capture-confirmation dialog stores the path Rust just handed back from creating an idea folder, and passes that same string to “Show in Finder.” Completely legitimate today. But legitimate-today isn’t the property that matters for a function reachable from a compromised or buggy webview - the function’s own signature is the security boundary, not who currently calls it responsibly.

The app already had the right shape sitting next to the wrong one. open_item_folder(id) takes an item id, looks up its folder from the database, and opens that - the path is never something the frontend hands in, only something the backend resolves from data it already owns. The frontend physically cannot ask this command to open anything outside the vault, because it never gets to specify a path at all.

The only snag: the confirmation dialog needed the raw path too, for a “here’s where to save your files” display and a copy-to-clipboard button. Losing that would be a real UX regression, not just a refactor. So the capture command’s return value grew a field instead of losing one - it already returned a path string, now it returns the id alongside it:

#[derive(Debug, Serialize)]
pub struct CapturedIdea {
    pub id: String,
    pub folder_path: String,
}

The path still flows to the display block and the copy button, exactly as before - showing a user their own file path in their own app was never the risk. Only the “open” action changed hands, from openFolder(savedPath) to openItemFolder(savedId). Same button, same behavior from the user’s seat, one fewer arbitrary-execution primitive reachable from JavaScript.

Reflection

None of the original trio would have fired today. The vanishing status, the silent downgrade, the mismatched boundaries - all latent, all waiting for a future change to step on them. The kind of thing tests don’t catch, because there’s no code yet to test. What caught them was treating “this looks already done” as a reason to read more carefully, not less.

And the review findings that followed held the same pattern all the way through: something that looks handled turns out to be handled for the case someone was thinking about, not the case that actually matters. A status nothing wrote. A sort that ran backwards, and one that never sorted. A constraint that wasn’t constraining. Half a decision left uncommitted. A rate computed in two languages. A settings file that could brick the app. And a command that does precisely what its docstring promises - where the promise was the bug.

The lesson I keep relearning: when your system encodes the same idea two ways, one of them is eventually wrong. Pick one. Delete the other.