Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 23 additions & 13 deletions docs/findings/003-no-model-level-background-job-seam.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,27 @@ issue: https://github.com/LASTRADA-Software/morph/issues/129

`morph::exec::IExecutor`/`ThreadPoolExecutor` (include/morph/core/
executor.hpp) has no usage anywhere inside a model's own `execute()` in
this codebase. Every existing "background job" (bookmarks' metadata-
fetch worker, examples/bookmarks/src/app/app.cpp) lives at the App/
Bridge/RemoteServer layer, re-entering the model as a fresh, ordinary,
fully-authorized client dispatch through a service-principal token --
not something a bare model with no App/Bridge/RemoteServer around it
can do. Ledger rung 5's report job (`SubmitReport`/`GetReportStatus`)
needed this and found no existing seam, so `LedgerModel` grew its own
this codebase. Every "background job" in the ladder (bookmarks'
metadata-fetch worker, pastebin's expiry sweep, ledger's report runner)
lives at the App layer, re-entering its model as a fresh, ordinary,
fully-authorized client dispatch -- not something a bare model with no
App/Bridge around it can do.

Ledger rung 5's report job (`SubmitReport`/`GetReportStatus`) needed this
and found no existing seam, so `LedgerModel` grew its own
`std::shared_ptr<morph::exec::IExecutor>` member as a local workaround.
A framework-level "background task from inside a model" primitive
(with a defined service-principal/session-propagation story for the
worker thread) would let future rungs avoid re-inventing this
per-model, and would let a future report job be tested with a
deferred/deterministic executor double instead of always spinning a
real thread pool.

**Update (morph#160).** That workaround is gone: rung 5 was given the App
layer it was missing (`examples/ledger/include/ledger/app/app.hpp`), and
the aggregation is now an ordinary `RunReportJob` action the runner
dispatches, so `LedgerModel` owns no executor and includes no morph
executor header. The finding's *premise* -- that there is no framework
primitive for a model to post its own background work -- is unchanged and
still open; what changed is that rung 5 no longer needs one, having taken
the same App-layer route every other rung already takes. So the remaining
question this finding poses is narrower than it was: is a
"background task from inside a model" primitive worth having *at all*,
given that the App-layer route exists, is testable without a deferred
executor double, and additionally makes a job survive the process that
accepted it? Whoever triages this should answer that rather than the
original "ledger needs this" framing.
16 changes: 11 additions & 5 deletions examples/ledger/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,17 @@ cmake_minimum_required(VERSION 3.25)

morph_add_rung(NAME ledger)

# No target_sources() extras yet: unlike polls' src/auth/ (PollsAuthorizer),
# this rung has no source directory morph_add_rung()'s globs
# (src/models/*.cpp, src/db/*.cpp, src/app/*.cpp) don't already cover. Add
# one here, mirroring polls' own CMakeLists.txt treatment of src/auth/, if a
# later task introduces such a directory.
# No target_sources() extras: unlike polls' src/auth/ (PollsAuthorizer), this
# rung has no source directory morph_add_rung()'s globs (src/models/*.cpp,
# src/db/*.cpp, src/app/*.cpp) don't already cover -- src/app/app.cpp
# (ledger::app::App, the report runner) is picked up by the third of those.
# Add one here, mirroring polls' own CMakeLists.txt treatment of src/auth/,
# if a later task introduces such a directory.
#
# There is deliberately no src/server/ yet, so morph_add_rung() emits no
# ladder_ledger_server target: this rung installs no authorizer, and an
# unauthenticated financial server is not something to ship (morph#242). See
# examples/ledger/include/ledger/app/app.hpp.

# gui/*.cpp and gui_wasm/*.cpp don't exist yet (Task 22 wires the desktop
# client) -- morph_add_rung() already skips ladder_ledger_gui and
Expand Down
37 changes: 33 additions & 4 deletions examples/ledger/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,39 @@ Build order (status as of rung 5's implementation, see
pattern**, this rung's framework-level deliverable: `SubmitReport` →
job id → `GetReportStatus` polling → fetch result; the submit→poll idiom
for long-running work that `Completion<T>`'s one-shot callbacks can't
express directly. **Snapshot semantics must be specified**: the job runs
off the strand and can otherwise see mid-action state across
`LedgerModel`/`BudgetModel` — use a SQLite WAL read transaction; the
byte-identical DoD is only meaningful against that snapshot.
express directly. **Snapshot semantics must be specified**: the job can
otherwise see mid-action state across `LedgerModel`/`BudgetModel` — use a
SQLite WAL read transaction; the byte-identical DoD is only meaningful
against that snapshot.

**Who runs the job (morph#160).** `SubmitReport` writes a `Pending` row
and returns; it schedules nothing and starts no thread. `ledger::app::App`
— this rung's App layer — sweeps for `Pending` rows on a timer and
dispatches `RunReportJob` back at `LedgerModel`, where the aggregation
itself lives. That split is `IMPLEMENTATION.md` rule 1 applied literally:
the monthly-statement aggregation is business logic and stays in a model;
only the decision of *when* it runs is orchestration, and orchestration
belongs to the App. It is also the shape `bookmarks::app::App`'s metadata
worker already had. `LedgerModel` owned a `ThreadPoolExecutor` before
this, and was the one ladder model that included
`<morph/core/executor.hpp>`.

Two consequences worth naming. The run now happens on the strand for its
own ledger, so a report and a concurrent `StoreTransaction` against the
same book serialise instead of racing — the WAL read snapshot is still
needed, because `BudgetModel` writes from a strand of its own. And a job
outlives the process that accepted it: it is a row, so a runner that
starts later — after a crash, after a restart — picks it up. The previous
design's queued lambda died with its process.

**The App does not own a `RemoteServer`, and this rung ships no server
binary.** `RemoteServer` clears the session principal for any authorizer
that does not authenticate (`docs/spec/security.md`), and this rung
installs no authorizer at all, so every mutating action reaching a model
over a remote backend arrives with an empty principal and is refused. The
runner therefore dispatches over a `LocalBackend` — the same backend the
shipped client uses. Rung 5's missing login/authorizer story is tracked in
morph#242; it is not a report-runner problem.
8. **Sync benchmark** (written deliverable, not code): reproduce one
concurrent-edit scenario from Actual (two offline clients edit the same
transaction's different fields) and one from ODK-style base-version
Expand Down
194 changes: 194 additions & 0 deletions examples/ledger/include/ledger/app/app.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
// SPDX-License-Identifier: Apache-2.0
#pragma once

#include <morph/core/backend.hpp>
#include <morph/core/bridge.hpp>
#include <morph/core/executor.hpp>
#include <morph/qt/qt_executor.hpp>

#include <QObject>
#include <QTimer>

#include <atomic>
#include <chrono>
#include <cstddef>
#include <memory>

/// @file
/// `ledger::app::App` -- rung 5's server-side bootstrap, and the home of the
/// one background job this rung has.
///
/// It exists because rung 5 had nowhere to put a background job and put it in
/// a model instead: `LedgerModel` owned a `ThreadPoolExecutor` and
/// `SubmitReport` posted the aggregation to it, making the one ladder model
/// that included `<morph/core/executor.hpp>` (morph#160). The layering that
/// resolves it is the same one `bookmarks::app::App` already demonstrates:
/// the App owns the worker pool and decides *when* work runs; the model still
/// owns *what* the work computes, and is re-entered as an ordinary client
/// dispatch (`RunReportJob`), on its own strand, where mutation is safe.
///
/// Mirrors `pastebin::app::App` (rung 1) closely and on purpose -- the same
/// timer-plus-internal-client shape, the same declaration-order-for-teardown
/// rule, the same in-flight settle seam -- minus the pieces this rung has no
/// equivalent for:
///
/// - **No `RemoteServer`, and therefore no `server()` accessor.** Not an
/// omission for brevity: this rung ships no `src/auth/`, no `AuthModel`
/// and no token secret, and `RemoteServer` *clears* the session principal
/// for any authorizer that does not authenticate
/// (`include/morph/core/remote.hpp`'s `dispatchExecute`, and
/// `docs/spec/security.md` on why passing an unverified claim through
/// would be worse). With the allow-all default that is all this rung
/// could install, every action arriving over a `RemoteServer` reaches the
/// model with an empty principal -- which every mutating `LedgerModel`
/// action, `RunReportJob` included, refuses. So the runner dispatches
/// over a `LocalBackend`, which is also exactly the backend this rung's
/// shipped client uses (`morph::ladder::gui::AppContext`'s `Local` mode).
/// Giving rung 5 a login, an authorizer and a server binary is its own
/// piece of work (morph#242); it is not a side effect of moving an
/// executor out of a model.
/// - **No `FileActionLog`.** `LedgerModel` attaches its own log explicitly
/// (`LedgerModel::attachActionLog`); installing a process-wide default
/// here would change what this rung journals, which is unrelated to why
/// this class exists.

namespace ledger::app {

/// @brief Owns rung 5's server side: the model worker pool, and the periodic
/// report runner that drains `ledger_report_jobs` by dispatching
/// `RunReportJob` at `LedgerModel`.
///
/// The runner dispatches through an **internal client** -- a `Bridge` over a
/// `LocalBackend` on this `App`'s own pool -- rather than calling
/// `LedgerModel::execute` directly. That is what makes a report run an
/// ordinary dispatch: authorized, keyed (so it lands on the strand for its
/// own ledger, serialised against that book's other actions) and journaled
/// exactly like a client-issued action, instead of a naked call from a
/// timer slot.
///
/// The bridge carries a default session naming `kReportRunnerPrincipal`,
/// which is the only principal `LedgerModel::execute(const RunReportJob&)`
/// accepts. It needs a bridge of its own for that and cannot borrow a
/// caller's: `Bridge::setDefaultSession` is bridge-wide and has no per-call
/// override, so a runner sharing a user's bridge would have to either
/// impersonate that user or overwrite their session.
///
/// @par What a restart does to a job, and why that is the improvement
/// Job state lives entirely in `ledger_report_jobs`, and a pass is just "find
/// the `Pending` rows and dispatch each". So a job accepted by a process that
/// then dies is not lost: it is still `Pending`, and the first pass of the
/// next process picks it up. Under the model-owned executor this replaced,
/// the same job was a lambda in a `ThreadPoolExecutor` queue -- recoverable
/// only for as long as that exact process lived, and invisible to anything
/// else. The cost of that recoverability is that a job whose aggregation
/// crashes the process is retried on restart rather than abandoned; a job
/// whose aggregation merely *throws* is recorded `Failed` and is not retried.
class App : public QObject {
Q_OBJECT
public:
/// @brief Wires up the server side and starts the report-runner timer.
/// @param runInterval How often the report runner sweeps for `Pending`
/// jobs. Tests pass a long interval (effectively disabling the
/// timer) and call `runPendingReportsOnce()` directly instead, for
/// determinism.
/// @param workers Size of the model worker pool.
/// @param parent Optional `QObject` parent.
explicit App(std::chrono::milliseconds runInterval = std::chrono::seconds{1}, std::size_t workers = 4,
QObject* parent = nullptr);

/// @brief Stops the report-runner timer.
~App() override;

/// @brief Stops the periodic report runner, so nothing this `App` owns
/// can dispatch new work from now on.
///
/// `~App` calls this too, so an owner that never calls it sees exactly
/// the same behavior. It is public because a *shutting-down* owner has to
/// call it earlier than that: the settle contract on `reportsInFlight()`
/// below says "pump until it is `false`, then destroy", and pumping is
/// precisely what lets the timer tick. A drain loop running with the
/// timer still armed could dispatch a brand-new pass out of its own
/// `processEvents()` call, re-raising `reportsInFlight()` after it had
/// settled. Calling this first makes the drain monotonic: the outstanding
/// set can only shrink.
///
/// Idempotent (`QTimer::stop()` on a stopped timer is a no-op) and safe
/// to call from the Qt thread at any point in the object's life.
void stopBackgroundJobs();

App(const App&) = delete;
App& operator=(const App&) = delete;
App(App&&) = delete;
App& operator=(App&&) = delete;

/// @brief Runs one report-runner pass right now: finds every
/// `ReportStatus::Pending` job row, across every ledger, and
/// fire-and-forget dispatches `RunReportJob` for each through the
/// internal client.
///
/// Does not block on the dispatched calls settling -- callers that need
/// to observe completion (tests, shutdown) pump the Qt event loop
/// afterward (`morph::ladder::testkit::pumpUntil`) on
/// `reportsInFlight()`.
///
/// Dispatching a job a previous pass is still working on is harmless and
/// expected: both dispatches key on the same ledger and therefore land on
/// one strand, and `execute(RunReportJob)` returns the already-terminal
/// status without recomputing.
///
/// The internal client used to issue this pass's dispatches stays alive
/// until every dispatched `RunReportJob` has actually settled, success or
/// failure -- see the implementation's own comment for why deregistering
/// it any earlier would race the backend's still-pending dispatches and
/// silently drop the pass.
void runPendingReportsOnce();

/// @brief Whether any `RunReportJob` dispatched by a previous
/// `runPendingReportsOnce()` has not settled yet.
///
/// The settle seam a test -- or a shutting-down server -- needs before
/// letting an `App` go, identical in contract to
/// `bookmarks::app::App::fetchInFlight()` and
/// `pastebin::app::App::sweepInFlight()`. Observing the *effect* of a
/// pass (the job rows are `Done`) is not the same as the dispatches
/// having settled: the aggregation happens on a worker thread while each
/// call's completion callback is delivered later, on the Qt event loop.
/// Destroying the `App` in that window leaves those callbacks queued
/// against objects it owned. Pump on this until it is `false`, then
/// destroy.
///
/// A teardown that does *not* wait is still safe for the data -- the
/// aggregation either committed or it did not, and an uncommitted job is
/// simply still `Pending` for the next process to pick up. What it is not
/// safe for is this object's own callbacks.
/// @return `true` while at least one dispatched `RunReportJob` is
/// outstanding.
[[nodiscard]] bool reportsInFlight() const noexcept { return _reportsInFlight->load() != 0; }

private:
// Declaration order is load-bearing, and `_reportExecutor` comes first on
// purpose -- the identical hazard pastebin::app::App documents at length.
// Members are destroyed in reverse, so this is the *last* thing to go. A
// pass's RunReportJob runs on `_pool`, and the worker thread that
// finishes it resolves the completion by calling `post()` on the executor
// the call was issued with. With the executor declared after the pool
// (its natural reading order), `~App` would destroy it while pool threads
// were still finishing dispatched work, and the next completion to
// resolve would post through a dangling `IExecutor*`. Destroying `_pool`
// (whose destructor joins its threads, so every in-flight completion has
// resolved) before the executor closes that window. `QtExecutor` holds no
// state and queues onto `QCoreApplication`, so callbacks it has already
// posted stay safe after `App` is gone.
::morph::qt::QtExecutor _reportExecutor;
/// Outstanding dispatches from `runPendingReportsOnce()`. A `shared_ptr`
/// so the completion callbacks that decrement it hold it by value rather
/// than through `this` -- a callback delivered after the `App` is gone
/// (the very case `reportsInFlight()` exists to let callers avoid) must
/// not touch a destroyed member.
std::shared_ptr<std::atomic<int>> _reportsInFlight{std::make_shared<std::atomic<int>>(0)};
::morph::exec::ThreadPoolExecutor _pool;
::morph::bridge::Bridge _reportBridge;
QTimer _reportTimer;
};

} // namespace ledger::app
23 changes: 23 additions & 0 deletions examples/ledger/include/ledger/core/types.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <cstdint>
#include <glaze/glaze.hpp>
#include <optional>
#include <string_view>

namespace ledger {

Expand Down Expand Up @@ -44,6 +45,28 @@ enum class RuleAction : std::uint8_t { SetCategory };
enum class ReportKind : std::uint8_t { MonthlyStatement, BudgetReport };
enum class ReportStatus : std::uint8_t { Pending, Done, Failed };

/// @brief The service principal `ledger::app::App`'s report runner dispatches
/// `RunReportJob` under, and the only principal
/// `LedgerModel::execute(const RunReportJob&)` accepts.
///
/// Declared here, in the rung's shared core header, rather than in
/// either place that uses it: the model must be able to check the name
/// without including an `app/` header (models know nothing about the
/// App layer), and the App must be able to name it without reaching
/// into the model's implementation file. The reserved `system:` prefix
/// mirrors `bookmarks::auth::kMetadataFetcherPrincipal` -- a namespace
/// no human login occupies, so a user principal cannot collide with it
/// by accident.
///
/// @warning This rung installs no authorizer, so nothing verifies a claimed
/// principal. The gate on `RunReportJob` is therefore a *layering*
/// check -- it keeps a user-issued action from silently completing a
/// job the runner owns -- not an authorization boundary. It becomes
/// one the moment this rung grows a signing authorizer, exactly as
/// bookmarks' did, and not before. See
/// `examples/ledger/include/ledger/app/app.hpp`.
inline constexpr std::string_view kReportRunnerPrincipal = "system:report-runner";

} // namespace ledger

/// @brief On the wire, each `LEDGER_DEFINE_STRONG_ID` type is its nullable
Expand Down
Loading
Loading