[LeoLan-Digital]

Summary

Keep project watches alive in permanent daemon mode after the last client session disconnects.

This allows daemon-owned background file watching and indexing to continue independently of client session lifetime.

What changed

• Preserve the physical project watch when the application runs in permanent mode
• Keep non-permanent behavior unchanged
• Allow watcher-triggered background indexing without an active client session
• Retain daemon ownership of watcher-triggered jobs until completion
• Add regression coverage for permanent daemon watch persistence

Tests

Focused daemon application suite:

• 50 passed

Full CLI baseline comparison:

• baseline: 280 passed, 1 failed
• remaining known failure: tests/test_cli.c:1748
• the same failure reproduces on the unmodified baseline and is unrelated to this change

git diff --check passes.

Behavior

Non-permanent applications retain the existing session-scoped watch lifecycle.

Permanent daemon mode now treats the physical watch as daemon-owned rather than session-owned, while project pruning and daemon shutdown still remove the watch through their existing lifecycle paths.

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

[DeusData]

Thank you for keeping the change focused on permanent-daemon watch ownership and preserving the non-permanent lifecycle. Session disconnects, watcher ownership, pruning, and background jobs form a sensitive lifecycle boundary, so we need more time to review it carefully. The queue is quite full, but we will come back with grounded feedback as soon as possible.

[DeusData]

Reviewed. The change is sound and the design question it turns on is answered well. Two lint items stand between it and merge, and the second one deserves care rather than the obvious fix.

The ownership shift is the right shape. Treating the physical watch as daemon-owned rather than session-owned only in permanent mode, with non-permanent behaviour untouched, keeps the blast radius to the mode that actually wants it. And you answered the question I would have asked — if the watch outlives the session, who removes it — by leaving project pruning and daemon shutdown as the lifecycle paths that still tear it down. A watch nothing owns is a leak; a watch owned by the daemon in permanent mode is the point.

The two lint failures

clang-format at src/daemon/application.c:3390-3392 — mechanical. git clang-format on the changed lines clears it. Note the CI formatter is the Homebrew LLVM build; a standalone clang-format-20 produces spurious whole-file drift here.

cppcheck: Variable 'watcher_owns_job' is assigned a value that is never used [unreadVariable] at :3460 — please do not fix this by deleting the assignment.

The flag itself is load-bearing: it is read at the if (watcher_owns_job) immediately above, and that read is what makes the unsubscribe conditional. cppcheck is objecting only to the watcher_owns_job = false; that follows the unsubscribe.

The reason to be careful is that this sits inside the for (;;) loop that begins around :3451. If control can re-enter that block on a later iteration with the flag still true, the reset is what stops a second application_job_unsubscribe_locked(job) on the same job — and removing it to quiet the linter would introduce a double-unsubscribe rather than remove dead code.

So the question to settle is narrow: can the loop re-reach that if after taking it once?

• If it cannot — every path through the block breaks or returns — the assignment really is dead and deleting it is correct.
• If it can, cppcheck is wrong here, and the house rule applies: refactor so the intent is visible to the analyser, or keep it with a // WHY: comment explaining the re-entry. Never silence a linter by removing a line that is doing work.

You are much better placed to answer that than I am from the diff, since the loop body is mostly context here.

Otherwise

Your baseline comparison is the right method — tests/test_cli.c:1748 reproducing on the unmodified base is what makes "unrelated" a measurement rather than an assertion. That failure is environmental and well known here; it depends on the coding agents installed on the machine.

Please also rebase: main moved three times yesterday — broken by a duplicate-symbol merge, repaired by #1993, then #1703 landed.

[DeusData]

Thanks for your patience on this one, @LeoLan-Digital. The permanent-daemon watch-ownership change is still the right shape. To close the loop on the open lint question from our last note, we traced it: the if (job->terminal) block always ends in cbm_mutex_unlock(...); break;, so the loop can never reach if (watcher_owns_job) again. That makes watcher_owns_job = false; at src/daemon/application.c:3460 a genuinely dead store. cppcheck's unreadVariable is right, and deleting that one line is the correct fix. The flag itself stays, because its read above is what makes the unsubscribe conditional.

The other red is clang-format on application.c:3390-3392 (the bool watch_live = ... expression). Running the Homebrew LLVM clang-format over the changed lines clears it. While you're amending: the commit has three Signed-off-by lines, two of them template placeholders. Could you leave just the real one? Then rebase on main and it's ready for another look. Thank you!

[DeusData]

Thanks again for the permanent-watch fix. The September 27 clarification still applies: remove the dead reset, keep the ownership flag and its read, and address the formatting and placeholder sign-offs. Please let us know when that revision is ready for another look.

[DeusData]

Thank you, @LeoLan-Digital, for this fix and for your patience while it waited in the queue. Making the physical watch daemon-owned only under daemon start, and leaving the session-scoped lifecycle untouched everywhere else, is exactly the right shape, and your regression test pins the behaviour down well.

Since the PR allows maintainer edits, we merged current main into your branch so it can move forward without another round trip on your side. Main reworked this part of src/daemon/application.c in the meantime, so the merge needed some care:

1. A session can now hold several watch memberships, so application_release_session_watch_locked walks all of them. Your rule now sits inside that loop: under daemon start the physical watch stays when its last subscriber leaves; otherwise it is removed as before.
2. The watch_live check keeps main's root matching and the index-argument encoding that follows it, and additionally admits a permanent watch with no subscribers, as your change intended.
3. Main split background indexing into an admit step and a wait step. Your watcher_owns_job flag is set at admission but released in the wait loop, so it now travels from one to the other as a parameter, and the wait step releases the retained subscription once the job finishes. That block always leaves the loop, so the reset after the release is no longer needed; this settles the earlier cppcheck note while the flag keeps doing its real job.
4. The watch_live expression is now clang-formatted, which clears the other lint failure.

Your commit is unchanged; everything above is one signed-off merge commit (a728eeb) on top of it. We do not rewrite contributor history, so the extra sign-off lines in your commit simply stay, and there is nothing you need to do about them.

A second, smaller step follows on top of this one: a few lifecycle edges of the permanent watch, the matching docs, and more tests. We will explain each change when it lands. If you push to this branch in the meantime, please pull first so nothing gets lost. Thanks again for the careful work!

[DeusData]

Thank you again, @LeoLan-Digital. As promised, here is the second step on top of your change. Your core idea carries all of it: under daemon start the physical watch belongs to the daemon, and the watcher callback keeps its own subscription so a background reindex can run with no client connected. The follow-ups below only close the edges that a longer-lived watch opens up.

We pushed three small commits:

1. 74e301d4 fix(daemon): end a permanent watch whose project was deleted.
   Once a watch outlives its sessions, nothing unwatched a project removed with delete_project from another session or the CLI, so the next change in that directory quietly re-created the deleted index. The watcher callback now ends a permanent watch that has no subscribers, no running index job and no database, and skips the reindex. The running-job check matters because a full reindex deletes the database while it rebuilds it: a second change during that rebuild joins the running job instead of ending the watch. New tests: daemon_application_permanent_watch_ends_when_project_db_is_deleted and daemon_application_permanent_watch_kept_while_its_reindex_runs (the latter also checks that each watcher callback hands its ownership back when the job finishes).

2. 63141868 fix(daemon): keep the workspace-policy release under daemon start.
   application_release_session_watch_locked also runs when a session's root stops being allowed by the workspace settings (for example when an approval is withdrawn). The session is still open there, so this is not a disconnect, and the watch should end in permanent mode as well. The function now takes a session_ending flag: the cancel and close paths pass true and keep the daemon-owned watch exactly as in your change; the workspace-policy path passes false and removes it. New test: daemon_application_permanent_workspace_release_still_ends_watch.

3. 70d6d501 docs(watcher): describe the watch lifetime under daemon start.
   README (the watcher section and the index_repository row), docs/CONFIGURATION.md and the comment above the index_status watch provider now say that under daemon start a watch lasts until the daemon stops, the project is deleted, its directory disappears, or auto_watch is set to false.

Every new test was checked in both directions: it fails with its production piece reverted and passes with it. The same check now also covers the two parts of your change that did not yet have a failing test of their own, the watch_live admission of a permanent watch and the retained watcher subscription: your test fails without either of them.

If you push to this branch again, please pull first, since it now carries these commits on top of the merge. Thanks again for a well-scoped contribution and for your patience while it worked its way through the queue!

[DeusData]

Thank you for your patience, @LeoLan-Digital. CI went green on this branch, but this evening's merges on main touched files this pull request also changes, so that green no longer covered today's main. We merged current main into the branch once more (one merge commit, 545c1bd; your changes are untouched) and will merge once the fresh checks are green. Please pull before pushing again.

[DeusData]

Thank you for your patience, @LeoLan-Digital. A pull request that merged into main a few minutes ago (#2717) changes README.md, which this pull request also changes, so the CI run in progress here could no longer vouch for today's main. We merged current main into your branch once more (one merge commit, ed17097; it merged cleanly and your own change is untouched) and started a fresh run. We will merge once the required checks are green. Please pull before pushing again.

[DeusData]

A small correction to my note above, @LeoLan-Digital: CI had already finished green on this branch, and #2717 landed on main earlier tonight rather than a few minutes ago. It added one line to README.md, which this pull request also edits. The two changes do not touch each other, but a green only counts for merging here when the files it covers are unchanged on main, hence the fresh run. Everything else in the note stands - sorry for the noise.

[DeusData]

Thank you for your patience, @LeoLan-Digital. A pull request that merged into main tonight (#2017) changes docs/CONFIGURATION.md, which this pull request also changes, so CI on this branch could no longer vouch for today's main. We merged current main into your branch once more (one merge commit, 9c83689; it merged cleanly and your own change is untouched) and started a fresh run. We will merge once the required checks are green. Please pull before pushing again.

[DeusData]

Thank you for your patience, @LeoLan-Digital. The last CI run on this branch failed to compile on every platform, and not because of anything you wrote: #2017 landed on main and added an approve_broad parameter to cbm_workspace_grant_add, and the new auto-watch test in tests/test_daemon_application.c still called it the old way.

We pushed one merge commit to your branch (b33a645, signed off): it merges current main and passes false for the new approve_broad argument in that one call, the same value the existing callers use. Your daemon change itself is untouched. On the merged tree the daemon_application and workspace suites pass (120 tests), including your new tests. A fresh CI run is on its way, and we will merge once the required checks are green. Please pull before pushing again so the merge commit is kept.

[DeusData]

Thank you for your patience, @LeoLan-Digital. The macOS legs of the last CI run here were cancelled by GitHub itself: it is retiring its macOS 14 runners on November 2 and ran scheduled brownouts beforehand, so those jobs never got a runner. Nothing in your change caused it. Main now runs CI on macOS 15 (#2849), so we merged current main into your branch (one merge commit, 2609e56; it merged cleanly and your own change is untouched) to get a run on the new runners. Please pull before pushing again.

[DeusData]

Merged as d1d8bbf - thank you, @LeoLan-Digital! Under daemon start a project's watch now belongs to the daemon and survives client disconnects, so background reindexing keeps running with no client connected; the follow-up commits close the edges that a longer-lived watch opens (deleted projects, workspace-policy releases, docs). Thank you for a well-scoped contribution and for your patience while it worked through the queue.