fix(daemon): defer jobs refused the palace lock, don't fail them (#2014)

[mvalentsev]

What does this PR do?

A daemon job refused the palace write lock was marked terminal failed and never retried. mine_palace_lock guards the palace write itself, so a refusal means no drawer was filed, but the queue recorded it exactly like a crash mid-execution, whose outcome is unknown. The work was dropped: it only ran again if a hook happened to re-submit equivalent work, and a job kind no hook re-emits was lost outright.

On develop, with a peer process holding the lock (the lock is re-entrant within one process, so the holder has to be external):

state    : failed
attempts : 1
-- lock released, 3s later --
state    : failed      <- never reconsidered

Refusals now defer instead of failing:

• QueueStore.defer() puts the job back to queued, clears started_at, undoes the claim's attempt increment, and records why it was parked. A palace held across many claims can no longer walk work that never landed up to MAX_ATTEMPTS. The state = 'running' guard mirrors finish(only_if_running=True), so a job already cancelled by shutdown isn't resurrected, and the update is scoped to the claim that was refused (started_at match), so a defer racing a recovery re-claim cannot re-queue work it does not own.
• The worker keys the deferral on error_class == "LockHeldByOtherProcess", then cools the job off in memory and moves on to the next one. It does not sleep in line. This is the only worker, and the holder keeps the lock until its write finishes, which can be a long mine, so blocking here would stall every unrelated job behind a lock that has nothing to do with them. claim_next(exclude=...) skips a cooling job, so the oldest-first ordering cannot hand the same refused job back forever while newer work waits.
• The cooldown is the worker's, held in memory. A not_before column would survive a restart, but the queue schema is CREATE TABLE IF NOT EXISTS with no migration step, so persisting it means adding one, and the only thing a restart costs here is a single immediate refusal before the job cools off again. That did not seem worth a schema change in a bugfix; if persistence matters more than schema stability, a not_before column is the follow-up.
• claim_next clears the recorded reason, so it never outlives the claim it describes.
• Every other failure stays terminal. That boundary is the whole point: a crashed job's outcome is unknown, and blindly re-running a non-idempotent kind would re-file verbatim content, which is exactly what MAX_ATTEMPTS guards.

MEMPALACE_DAEMON_LOCK_BACKOFF_SECONDS (default 60) tunes the cooldown. Anything not positive and finite falls back to the default: inf parses fine and passes a bare positivity check, which would park the job until the daemon restarted.

Why the retry is safe

diary_write is the awkward case, so it is worth being precise. _wal_log runs before the collection write, so a refusal happens after a WAL line was already appended. That line is an audit record: entry is in _WAL_REDACT_KEYS, the file has no readers anywhere in the tree, and nothing replays it. The palace write is what the lock guards and it did not happen, so re-running cannot duplicate verbatim content.

Each retry does append another such record, so a palace held for a long time leaves one audit line per cooldown period for the job. That is log noise, not duplicated content, and it is the price of not dropping the write.

Callers that wait

Deferral makes a refused job non-terminal, and DaemonClient.wait only returns on a terminal state. Three callers wait on purpose, and all three would have waited for a state a parked job cannot reach:

• mine --daemon (foreground) would have blocked for the full one-hour default and then printed daemon submission failed: timed out waiting for job ..., which is wrong twice over: the submission succeeded, and the job is still queued to run.
• The hook paths in hooks_cli (the pre-compaction mine at wait=True, timeout=60, and the Stop-hook diary checkpoint at wait=True, timeout=30) would have burned their whole timeout on every fire and then logged a failure that never happened.

wait(stop_on_lock_deferral=True) hands the parked job back instead. The CLI reports the holder and a command that actually runs, and the hooks log the deferral rather than a failure:

mempalace: palace /tmp/fg2014_e6F0/palace is held by PID 45909 (/tmp/fg2014_e6F0/holder.py 40); wait for it to finish or stop the holder before retrying
mempalace: job 9187e327120c4650b4332d6f946bc466 is queued and runs when the holder exits (check it with: mempalace --palace /tmp/fg2014_e6F0/palace daemon jobs)

--palace is global, so it has to be echoed back ahead of the subcommand; without it the suggestion silently lists the default palace's queue instead of the one the job is parked in.

Because a cooling job no longer blocks the worker, a job submitted behind a deferred one is still claimed and refused on its own, so it gets its own marker and its caller short-circuits too. A job that is genuinely running (a long mine) is still waited out: the short-circuit only fires on queued plus the refusal marker. Background callers keep the old behaviour.

The same bug class elsewhere

tool_diary_write had to be fixed for the gate to reach it. It swallowed MineAlreadyRunning in its bare except Exception, so the refusal arrived with no error_class, and diary_write (one of the two kinds in the queue dump on the issue) would still have been dead-lettered. It now uses a typed handler ahead of the bare Exception, the way tool_mine and tool_sync already do.

Of the four job kinds, mine and sync mark the refusal in service.run_mine / run_sync, and diary_write is fixed here. Two gaps are left deliberately, both pre-existing and both keeping exactly the behaviour they had before this change:

• mcp_tool dispatches the write tools, and four of them (tool_add_drawer, tool_update_drawer, tool_delete_drawer, tool_delete_by_source) reach a collection write but swallow MineAlreadyRunning in bare handlers, so their refusals would still be dead-lettered. tool_checkpoint is a fifth, one step quieter: it files through tool_add_drawer and buries the refusals in its errors list with no top-level error, so the job even reports succeeded. Nothing submits that kind internally right now.
• mine_formats swallows it per file and in its outer handler, so a mine --mode extract --daemon job reports success on a refusal rather than surfacing it at all. This one is reachable through the daemon today, and the refusal never leaves mine_formats, so the new gate cannot see it either way.

All of these are worth their own fix; folding them in would mean touching most of the write surface in a bugfix PR.

How to test

python -m pytest tests/test_daemon.py -k "lease or defer or backoff or claim_clears or cooling or stall or behind or excluded or run_mine_lease" -v
python -m pytest tests/test_hooks_cli.py -k lock_deferral -v
python -m pytest tests/test_cli.py -k lock_deferral -v
python -m pytest tests/test_mcp_server.py -k diary_write_lease_refusal -v

End to end, with a peer process holding the lock:

export MEMPALACE_DAEMON_LOCK_BACKOFF_SECONDS=3
mkdir -p /tmp/p/palace /tmp/p/proj

python - <<'PY' &
import time
from mempalace.palace import mine_palace_lock
with mine_palace_lock("/tmp/p/palace"):
    time.sleep(40)
PY

mempalace --palace /tmp/p/palace mine /tmp/p/proj --daemon --background
mempalace --palace /tmp/p/palace daemon jobs   # not failed
# once the holder exits:
mempalace --palace /tmp/p/palace daemon jobs   # succeeded

Before the change the first daemon jobs shows failed, and the job stays failed after the holder exits.

Checklist

• Tests pass (python -m pytest tests/ -v)
• No hardcoded paths
• Linter passes (ruff check .)

Fixes #2014