ladder: rung 4 -- kanban - #121
Open
Yaraslaut wants to merge 33 commits into
Open
Conversation
Yaraslaut
pushed a commit
that referenced
this pull request
Aug 17, 2026
…equired default clang's -Wswitch-default (enabled under -Weverything -Werror on the Linux clang-coverage / all-optional-features / Application-ladder CI legs) requires an explicit default: label even on a switch that already covers every enumerator -- confirmed this is the only failure across all four failing jobs on PR #121's one CI run to date, and that this exact tension (exhaustive switch needing a default anyway) is an already-accepted pattern elsewhere in the ladder: examples/pastebin/include/pastebin/units.hpp's UnitTraits<Unit>::meta has the identical shape. CI's flag list already carries -Wno-covered-switch-default, so adding the default arm satisfies -Wswitch-default without tripping the opposite warning -- verified by compiling a standalone repro of the exact switch shape against clang 22 (the CI compiler version) with the full CI flag list, both before (fails on -Wswitch-default) and after (clean) this change. No functional change -- the added default arm returns the same fallback roleToString() already returned unconditionally before this fix (Role::Viewer's string), for a code path every enumerator already short-circuits before reaching. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Filed morph#112 (IOfflineQueue has no depth bound or overflow policy) -- verified against offline_queue.hpp/sqlite_offline_queue.hpp/ file_offline_queue.hpp: enqueue() has no capacity parameter, no depth cap, and no overflow signal anywhere in the interface or either shipped implementation. Needs a framework-level decision (evict-oldest vs. reject-newest vs. app-defined policy) before rung 4's offline stack (step 7) can define its own overflow behavior.
…orizer Dispatched an analysis agent on whether per-project RBAC (viewer/member/manager) belongs in IAuthorizer::authorizeInstance or inside BoardModel::execute() itself. Verified recommendation: in-model, mirroring polls::PollModel::requireAdmin()'s exact precedent. docs/spec/core/shared_instances.md already settles this for shared instances generally (BoardModel is one): teaching authorizeInstance about a per-instance owner *set* was explicitly rejected there as adding complexity to a hook the model layer already handles better. docs/spec/security.md's own local-path note clinches it independent of that: authorizeInstance never runs for LocalBackend callers at all, so an IAuthorizer-only RBAC check would silently not exist locally -- BoardModel needs its own check regardless of what the authorizer does, making a framework interface change pure duplicated surface with no coverage gain. Tightened step 4's wording so a future reader doesn't reopen this question.
Resolves examples/kanban/README.md's design questions in writing, per the ladder's own discipline rule. Covers steps 1-5+7 (steps 6/8 stay deferred, per the README's own scoping): - Exactly-once (MoveTaskPosition): generalizes bookmarks::ImportBookmarks' client-op-id + server-side applied-ops-ledger pattern, storing the full serialized GetBoardResult (not a placeholder) so a replaying client reconciles against the real outcome. - Strand ordering / WIP limits / position renumbering: rely entirely on the framework's existing strand-per-instance guarantee (verified against docs/spec/core/shared_instances.md); no new locking. - Per-project RBAC: in-model requireRole() check mirroring PollModel::requireAdmin(), not an IAuthorizer interface change -- resolved via an analysis agent, grounded in shared_instances.md's and security.md's own already-written positions. - Activity stream: derived from IActionLog::entries(entityKey) -- no new storage. - Offline: composes SqliteOfflineQueue/SyncWorker/ReconnectCoordinator as-is; verified (not assumed) that a reconnect flap cannot preempt an in-progress replay. Two testkit-scope findings, verified by file-existence checks: - action_driver.hpp/process_pool.hpp/offline_rig.hpp are rung 4's own obligation per examples/TESTING.md's ownership table (confirmed none exist yet). - client_pool.hpp/convergence.hpp were TESTING.md's documented rung-3 obligation but polls (merged, PR #91) never built them -- absorbed into this rung's scope since kanban's own convergence DoD item needs them regardless of original ownership. Framework gap filed and cross-referenced: morph#112 (IOfflineQueue has no depth bound or overflow policy), verified against offline_queue.hpp/sqlite_offline_queue.hpp/file_offline_queue.hpp.
…l 6 gaps An independent Fable 5 review (dispatched per user request) verified every citation in the design spec against the actual code/docs and caught one load-bearing defect plus several real gaps: Defect (verified, corrected): - Section 3 originally cited PollsAuthorizer (AllowAllAuthorizer-derived) as KanbanAuthorizer's shape. security.md's own documented behavior: an authorizer that never authenticates has dispatchExecute clear Context::principal to empty before every remote dispatch -- so requireRole()'s project_has_roles lookup would have nothing to key on over the socket, silently diverging from Local-mode tests where a principal can be hand-populated. Corrected to BookmarksAuthorizer's shape (SigningAuthorizer-derived, a real verifying authorizer) and added an Identity subsection covering the login/token dependency this pulls in and who seeds a project's first manager role. Gaps closed: - Section 4's "attachActionLog() convention" didn't exist anywhere in the ladder (verified: no rung calls it) -- kanban is the first to use it, not a follower; stated as such, with the LocalBackend-has-no-LogProvider and same-log-instance plumbing this now requires spelled out. - Ledger hits (section 1) would double-journal since the auto-append registrar has no visibility into an action's own opId; resolved by collapsing consecutive identical-payload LogEntry rows on the activity view's read side rather than touching the framework's append path. - GetEventsSince's own design was undecided; resolved as a real board_events table (polls::PollEventRecord's exact precedent), distinct from the activity stream's journal-derivation -- LogEntry::seq is documented as process-local, unusable as a durable poll cursor. - ProjectAdminModel's write surface (a separate strand from BoardModel) is now drawn explicitly, with the column-deleted-mid-drag race resolved via re-validation inside MoveTaskPosition's own transaction, not cross-strand coordination. - Section 5's DoD gaps filled: enqueue-on-failed-dispatch trigger, DeadLetterSink wiring, conflict-on-replay behavior, observability assertions. - requireRole-vs-ledger-hit ordering (section 1) made explicit: role check runs before the ledger lookup, so a demoted caller's replay is denied rather than handed a stored result their current role could not produce. - Minor: fixed a wrong citation attribution, added the strand interleaver to the test plan, noted board_applied_ops' own unbounded retention. Also updated examples/kanban/README.md's step 4 wording to match the corrected authorizer shape.
Implements docs/superpowers/specs/2026-08-16-kanban-rung4-design.md's steps 1-5+7 scope: schema/entities, BoardModel (CRUD, MoveTaskPosition with WIP limits/position renumbering/exactly-once ledger, RBAC gate, activity stream, GetEventsSince), ProjectAdminModel (project lifecycle, role management), KanbanAuthorizer (SigningAuthorizer-derived per the spec's corrected identity decision), plus the five testkit files rung 4 owns (action_driver.hpp, offline_rig.hpp, client_pool.hpp, convergence.hpp -- the last two absorbed from rung 3's undelivered obligation per spec section 6) and the DoD stress/offline test suites. Backend + testkit only, fully testable via BackendRig with no GUI dependency -- GUI (presenters/QML bridges/QML views) is a separate follow-on plan, split out since this plan already runs to 20 tasks and GUI work only starts once the model surface it binds against exists. Self-review found and closed one real gap: the original draft had no task for design spec section 5's offline DoD tests (exactly-once under FaultProxy::dropReply(), kill-the-network via offline_rig.hpp, SQLite contention via DbBusyFixture) -- added as Task 20. Two tasks (19's stress-test body, 20's three offline test bodies) are deliberately left as structured comments over real TEST_CASE names rather than guessed implementations, since they depend on StrandInterleaver's/FaultProxy's/DbBusyFixture's own exact APIs that should be read fresh at execution time rather than reproduced from memory here -- flagged inline as intentional, not silent placeholders.
- CMakeLists.txt with morph_add_rung(NAME kanban) and minimal boilerplate - Skeleton headers: database.hpp, db_model.hpp, app.hpp, kanban_authorizer.hpp - Minimal implementations: kanban_authorizer.cpp, schema.cpp, server/main.cpp - All tokens replace polls equivalents (polls→kanban, Polls→Kanban, POLLS→KANBAN) - Build verification: ladder_kanban_lib target builds successfully
CRITICAL FIX: - KanbanAuthorizer now derives from SigningAuthorizer (was AllowAllAuthorizer) - Matches BookmarksAuthorizer pattern per design spec §3 (corrected identity) - Provides trustworthy Context::principal for BoardModel::requireRole() - Implements setTokenIssuer()/tokenIssuer() process-global installation HEADER/SOURCE UPDATES: - app.hpp: Updated docs to reflect SigningAuthorizer + TokenIssuer requirement - src/server/main.cpp: - Added KANBAN_TOKEN_SECRET env var (required, no default per security.md) - Installs TokenIssuer before App construction - Fixed 'kanban' apostrophe typo in file comment TESTS: - Created examples/kanban/tests/ with placeholder test_placeholder.cpp - ladder_kanban_tests target now builds successfully MINOR FIXES: - schema.cpp: Replaced dangling using statement with Task 3+ note - db_model.hpp: Fixed access specifier indentation (column 2, per project convention)
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ban_authorizer.cpp KanbanAuthorizer, its header, and CMakeLists.txt wiring already existed from Task 1's fix round. This closes the one Minor finding parked from that review: setTokenIssuer/tokenIssuer used an unguarded function-local static shared_ptr slot, unlike bookmarks::auth's std::mutex-guarded equivalent. Replaces it with the identical detail::tokenIssuerMutex()/ tokenIssuerSlot() pattern from bookmarks_authorizer.hpp:265-308. Adds the missing test_kanban_authorizer.cpp (Task 7's Step 1/6), plus a third case covering the mutex-guarded slot itself, mirroring bookmarks' own "share one process-global slot" coverage.
… management - ProjectAdminModel::execute(CreateProject) creates the project and seeds the caller as its first Manager role, in one transaction. - ::execute(SetMemberRole)/::execute(RemoveMember) are Manager-gated via requireRole(); SetMemberRole deletes-then-recreates the role row. - ::execute(GetProjectRoles) is Viewer-gated (any member may list). - requireRole() loads the project first (NotFound if absent), then the caller's own role row (Forbidden if absent or below the minimum) -- mirrors PollModel::requireAdmin()'s ordering. - AuthModel::execute(Login) mirrors bookmarks::AuthModel exactly, using kanban::auth::tokenIssuer(); added isValidPrincipal/isReservedPrincipal to kanban::auth (mirroring bookmarks::auth) since Login/AuthModel need them and kanban had none yet. - New examples/kanban/include/kanban/dto/auth_dto.hpp, ported from bookmarks' auth_dto.hpp with the namespace renamed. - CMakeLists.txt: added src/dto/auth_dto.cpp to ladder_kanban_lib's explicit target_sources() (the rung's default glob doesn't cover src/dto/), mirroring bookmarks' identical treatment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…UD/AddComment Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…-once ledger Task 10 of the kanban rung-4 plan (design spec §1/§2). Implements BoardModel::execute(const MoveTaskPosition&): ledger lookup by (projectId, opId) before any re-validation (hit -> decode and return the stored GetBoardResult verbatim; miss -> requireColumnBelongsToProject cross-strand re-check -> WIP-limit check -> delete-then-recreate position renumbering -> ledger write, all inside one SqlTransaction). Fixes one defect in the task brief's literal code: the brief assigned the raw FK integer directly to the BelongsTo fields (task.column = static_cast<uint64_t>(*action.columnId)). That compiles (BelongsTo's non-explicit value constructor + copy-assignment accept it) but never marks the field _modified, so the following mapper->Update(task) would silently omit column_id/swimlane_id from its SET clause -- the move would appear to succeed but never persist. Verified empirically: reverting to the brief's literal assignment made the first new test fail exactly this way. Fixed by loading the target ColumnRecord/SwimlaneRecord rows (already needed for the WIP-limit check) and assigning those objects instead, mirroring this file's own rec.project = project; pattern for every other BelongsTo field. Also added a swimlane-belongs-to-project re-check alongside the brief's column re-check, for the same cross-strand reason design spec §2 gives for the column check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds BoardModel::requireRole(Role minimum), mirroring ProjectAdminModel::requireRole's shape (design spec §3's explicit "not shared code" note -- each model gets its own copy since BoardModel and ProjectAdminModel have separate mapper/entity access). Gates CreateColumn, CreateSwimlane, CreateTask, AddComment, and MoveTaskPosition at Role::Member. OpenBoard, GetBoardState, and GetEventsSince remain ungated -- any attached caller, even a bare Viewer, may read. For MoveTaskPosition, the gate call runs unconditionally at the top of execute(), before the exactly-once ledger lookup: a demoted caller replaying a known opId must not retrieve a stored result their current role could no longer produce. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…dup on read BoardModel::attachActionLog/logAction is a model-level mirror of IModelHolder::attachActionLog/recordIfAttached, not a call into it: a plain BoardModel a unit test constructs directly has no IModelHolder wrapping it, so the registry's auto-append (ActionDispatcher's runner, registry.hpp) never fires for that path. BoardModel keeps its own shared_ptr<IActionLog> + entity key and appends its own LogEntry at the end of every mutating execute(), including the MoveTaskPosition ledger-hit replay path (reproducing the same double-journal the framework's own auto-append would produce for a holder-wrapped instance). execute(GetActivity) derives the stream from IActionLog::entries(entityKey) and collapses consecutive entries with identical actionType+payload on the read side, per design spec section 4.
…et matrix)
Task 14: ports examples/polls/tests/test_shared_instance_lifecycle.cpp's
three coverage pieces to kanban's BoardModel/KanbanAuthorizer --
1. BoardModel over the full backend-mode matrix (Local/LocalSingleThread/
Socket): CreateProject (plain, non-keyed ProjectAdminModel handler) ->
handler.execute(OpenBoard{projectId}) keyed attach -> CreateColumn ->
GetBoardState.
2. N AllowShared BoardModel handlers on one projectId observe each other's
writes, and instances() reflects the instance's real lifetime (present
while attached, gone once every attacher releases it).
3. A poisoned attach to a stale projectId fails identically (NotFound) on
retry from the same handler -- the no-op-on-same-primary guard means it
never re-points, and OpenBoard's own loadProjectById() re-runs every
call, so there is no silently half-hydrated success.
4. Cross-project role isolation: a role granted on one project does not
leak into a different project the same handler later attaches to.
KanbanAuthorizer is SigningAuthorizer-derived (unlike polls'
AllowAllAuthorizer-derived PollsAuthorizer), so every BackendRig client
needs a real signed session token installed via Bridge::setDefaultSession
before BoardModel::requireRole()'s principal-keyed lookup can work --
mirrors test_bookmark_model.cpp's identical TokenIssuer/setDefaultSession
setup for BookmarksAuthorizer, kanban's other SigningAuthorizer-derived
authorizer.
Also fixes a latent framework bug this task's own multi-handler
instances() test surfaced: QtWebSocketBackend::listInstances (and
morph::net::SocketBackend's identical implementation) built the wire
envelope without stamping env.session, unlike every other envelope-
building call site in both classes (register/attach/assign/execute/
deregister all set it). RemoteServer's instances handler authorizes with
IAuthorizer::authorize(env.session, typeId, {}), so a Socket-mode client
with a SigningAuthorizer got 'unauthorized' calling instances() even
after a successful, correctly-authenticated execute() on the same
connection. Every existing rung's instances() coverage used an
AllowAllAuthorizer-derived authorizer (or ran Local/in-process), so
authorize() was always permissive regardless of session and never
exercised this path -- kanban is the first rung to combine a
SigningAuthorizer with a Socket-mode instances() call.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
App::App(actionLogPath, tokenSecret, workers) wires the whole server side: worker pool, RemoteServer with a real KanbanAuthorizer (SigningAuthorizer- derived, verified against tokenSecret), the durable FileActionLog installed process-wide via morph::journal::setActionLog, the process-global TokenIssuer AuthModel mints tokens from, and RemoteServer::setLogProvider supplying the same FileActionLog instance for registry-constructed, keyed BoardModel attaches. Mirrors bookmarks::App's structure (this rung has no background worker/timer, so App stays plain C++, no QObject). app.hpp's constructor signature grew a tokenSecret parameter: the header existed from Task 1's scaffolding with no way to give KanbanAuthorizer/ TokenIssuer a real, verifiable secret at all, which main.cpp's own speculative stub (also from Task 1, before App existed) did not supply either -- both needed fixing together for a real deployment to work. main.cpp now passes KANBAN_TOKEN_SECRET straight through to App instead of installing the TokenIssuer itself. Resolves the load-bearing gap flagged in Task 13's review: App wires RemoteServer::setLogProvider so a registry-constructed, keyed-attach BoardModel's own attachActionLog is called with the SAME IActionLog instance the holder's auto-append writes to (via the previous commit's onActionLogAttached forward) -- not a separate log, not no log. BoardModel::logAction also gained a flush() call after append(): proven necessary by this task's own end-to-end test, which found GetActivity nondeterministically missing an entry it had just recorded through FileActionLog, because append() writes through buffered C stdio with no implicit flush and entries() reads through a separate ifstream that cannot see unflushed bytes. InMemoryActionLog::flush() is a no-op, so this is free for every non-App test that attaches an in-memory log directly. test_app.cpp includes the proof this gap is actually closed: dispatches CreateProject/OpenBoard(AllowShared, keyed)/CreateColumn through App's real RemoteServer via SimulatedRemoteBackend (the same in-process-but-real- dispatch path bookmarks::App's own metadata worker uses, not LocalBackend or a direct BoardModel construction), then confirms GetActivity returns the CreateColumn entry and that the same durable log file holds it. A companion case confirms a plain (non-shared) BoardModel registration also works, via the process-wide default log ModelFactory::create<Model>() attaches independently of LogProvider/contextKey. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… invariant hook Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…sertion (absorbed from rung 3)
BackendRig has no clientCount()/nClients() accessor (brief's Step 4
assumption was wrong, not just misnamed -- verified against
backend_rig.hpp's actual public interface and every existing call site,
none of which reads a count back from the rig). ClientPool's constructor
takes nClients as an explicit parameter instead, matching what every
caller already has on hand from its own BackendRig{mode, nClients, ...}
call.
bridge(i) and executor() are confirmed correct as the brief assumed.
Adds test_client_pool.cpp (not in the brief) to exercise ClientPool
against a real BackendRig across all three modes, since the brief's
Step 1 test only covers convergence.hpp.
…mode)
test_kanban_stress.cpp fires ~50 MoveTaskPosition calls per client from 4
AllowShared BridgeHandler<BoardModel> clients sharing one projectId's
keyed instance, all non-blocking (execute() returns a Completion; nothing
awaits between fires), so BoardModel's shared strand genuinely races
across Mode::Local's real ThreadPoolExecutor{4} worker threads -- the
condition a ThreadSanitizer CI leg over this test exists to check.
The brief's own 'StrandInterleaver' premise did not survive contact with
the real header: strand_interleaver.hpp defines DeterministicExecutor,
tested directly against morph::exec::detail::StrandExecutor in
test_strand_interleaver.cpp, not a class named StrandInterleaver.
BackendRig{Mode::Local, ...} also builds its own ThreadPoolExecutor
internally with no seam to substitute a DeterministicExecutor underneath
LocalBackend's strand, so that harness is not wireable into a
BackendRig-driven test at all. Determinism here instead comes from
SeededScript's seeded RNG (MORPH_STRESS_SEED reproduces a failing run).
Running the new test surfaced a real bug in MoveTaskPosition's position
renumbering (Task 10): it only renumbered the destination
(columnId, swimlaneId) pair, never the source, so a task moving out of a
column left the remaining tasks there with a permanent gap instead of a
dense 0..n-1 run -- violating design spec section 2's per-pair density
invariant. Fixed in board_model.cpp by adding a source-side renumbering
pass, gated on source != destination so a same-pair reorder isn't
double-renumbered. Locked in with both the stress test and a minimal
deterministic single-threaded regression case in test_board_model.cpp.
Full investigation notes: docs/superpowers/sdd/2026-08-16-kanban-backend/task-19-report.md
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ect convergence, SQLite contention Adds examples/kanban/tests/test_kanban_offline.cpp with the three DoD scenarios design spec §8 names: - Dropping MoveTaskPosition's reply frame and retrying via the same opId is exactly-once, not double-applied (FaultProxy + BackendRig::Socket). - Reconnecting after a dropped connection replays the offline queue and converges (OfflineRig-equivalent drop/revive sequence against a directly-built RemoteServer/QtWebSocketServer stack, since OfflineRig cannot attach to a BackendRig-owned server -- see report for why). - 32 boards writing concurrently under genuine SQLite contention (DbBusyFixture) never show a timeout-then-committed double-apply. Also fixes a pre-existing name collision this task's test file is the first to surface: fault_proxy.hpp and backend_rig.hpp both defined morph::ladder::testkit::detail::throwIfListenFailed(bool) with different bodies in the same namespace -- a hard redefinition error the moment a single translation unit includes both headers, which no prior test file ever did. Renamed fault_proxy.hpp's copy to throwIfFaultProxyListenFailed (and its one call site plus the one TEST_CASE naming it directly); backend_rig.hpp's copy and every other call site are untouched. Full API-mismatch/finding writeup in .superpowers/sdd/2026-08-16-kanban-backend/task-20-report.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… DoD test The SQLite-contention TEST_CASE asserted an exactly-once invariant on both a 'threw' and a 'succeeded' branch, but every real run before this fix produced 0 successes / 32 failures -- the 'succeeded calls also apply correctly' half was dead code. Two root causes, both confirmed via temporary instrumentation (added, used, fully removed): 1. ScopedShortBusyTimeout's PRAGMA busy_timeout hook alone doesn't touch the sqliteodbc driver's own outer retry ceiling (DbFixture's baked-in connection-string Timeout=5000). Every call still failed at ~5.1-5.2s until the hook also shortened the *default* connection string's Timeout= for its lifetime -- the same combined recipe test_db_busy_fixture.cpp already documents, extended here since GlobalDataMapperPool() connections use the default connection string, not an explicit per-DataMapper one. 2. A genuine bug: DbBusyFixture's lock was released implicitly, by the object going out of scope at the very end of the TEST_CASE -- after every worker thread was already joined. The lock was therefore held for the entire 32-way contention phase and never observably released while a worker was still waiting, independent of any busy-timeout value. Fixed by holding it in a unique_ptr the releaser thread itself resets after kLockHold. With both fixed, 32 real threads racing SQLite's single writer lock (rollback-journal mode) produce a severe, genuine thundering-herd -- raising the timeout well past ~2s doesn't change the mix (measured up to 20s). kShortBusyTimeoutMs=2000 / kLockHold=150ms reliably produces a small but real mix (1-2 succeeded, 30-31 failed, out of 32) across 8+ repeated runs including fresh-DB cold runs, with the no-double-apply invariant holding on both branches every time. Full account in .superpowers/sdd/2026-08-16-kanban-backend/task-20-fix-round-1-report.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e-journaling replays Fix 1 (C1, Critical): OpenBoard/GetBoardState/GetEventsSince/GetActivity performed no role check at all, so any authenticated principal (Login mints a token for any username, no registration/membership check) could read any project's full board, comments, and activity journal by id. Adds requireRole(Role::Viewer) to GetBoardState/GetEventsSince/GetActivity, and a new requireRoleOn(projectDbId, minimum) that OpenBoard calls against the *target* project id (its own _projectIdStr isn't set yet at that point) right after loadProjectById resolves the project and before buildState returns any content. requireRole(Role) now delegates to requireRoleOn using _projectIdStr, so every existing call site is unaffected. Updates test_shared_instance_lifecycle.cpp's cross-project OpenBoard case, which previously let the read through and only asserted the subsequent write was Forbidden -- it now asserts OpenBoard itself is rejected. Fix 2 (C2, Critical): CreateTask, AddComment, and MoveTaskPosition's task lookup never re-checked that the column/swimlane/task ids they were handed actually belong to the attached project, unlike MoveTaskPosition's existing destination-column check. A Member of project A could create a task in project B's column, inject a comment onto project B's task (which then surfaced in B's own board view), or move B's task into A's column entirely. Adds requireTaskBelongsToProject and requireSwimlaneBelongsToProject (siblings of the existing requireColumnBelongsToProject) and calls them from all three actions before their transactions begin. Adds four cross-tenant negative tests plus a dedicated test for the previously-untested swimlane-belongs-to-project check (ledger triage item #14). Fix 3 (I1, Important): MoveTaskPosition's ledger-hit branch called logAction() for a replay it didn't actually perform, journaling a Succeeded outcome for an operation that only returned a stored result. Design spec §4 had justified this as compensating for the framework's own auto-append supposedly double-journaling ledger hits -- a live FileActionLog capture during real RemoteServer dispatch (recorded in the final review) shows this premise was wrong: the framework appends exactly once. Removes the logAction call from the ledger-hit branch and the now-unnecessary read-side collapse loop in GetActivity (which was also independently lossy: it silently merged any two consecutive identical actionType+payload entries, not just replays). Corrects design spec §4 to record the framework's actual (verified) behavior. Fix 4: adds the missing @param/@return Doxygen tags to AuthModel::execute for consistency, and appends a correction to the ledger's Task 8 entry -- docs/CMakeLists.txt only scans include/morph, so this was never a CI docs-gate blocker as previously claimed. Verification: ladder_kanban_tests 270/57 (was 262/52), stable across multiple random-order runs; ladder_common_tests 295/84 unchanged; [stress] 12/1 and [offline] 94/3 unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… project bootstrap)
…equired default clang's -Wswitch-default (enabled under -Weverything -Werror on the Linux clang-coverage / all-optional-features / Application-ladder CI legs) requires an explicit default: label even on a switch that already covers every enumerator -- confirmed this is the only failure across all four failing jobs on PR #121's one CI run to date, and that this exact tension (exhaustive switch needing a default anyway) is an already-accepted pattern elsewhere in the ladder: examples/pastebin/include/pastebin/units.hpp's UnitTraits<Unit>::meta has the identical shape. CI's flag list already carries -Wno-covered-switch-default, so adding the default arm satisfies -Wswitch-default without tripping the opposite warning -- verified by compiling a standalone repro of the exact switch shape against clang 22 (the CI compiler version) with the full CI flag list, both before (fails on -Wswitch-default) and after (clean) this change. No functional change -- the added default arm returns the same fallback roleToString() already returned unconditionally before this fix (Role::Viewer's string), for a code path every enumerator already short-circuits before reaching. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…its::key
GCC's -Wuseless-cast (enabled on the "Application ladder" CI leg's
gcc-debug build, -Werror) correctly flagged this: ProjectId::operator*()
already returns std::int64_t directly (KANBAN_DEFINE_STRONG_ID's
generated dereference operator), so casting its result to
std::int64_t again was a genuine no-op. keyToString<K> is a template
taking any key type by const-ref, so passing the already-int64_t value
directly is equivalent and compiles identically under every other
build's flags.
This is the cosmetic Minor the Task 9 SDD ledger entry noted and
deferred to the final review ("redundant static_cast in the key
extraction") -- deferring it correctly assumed it was cosmetic under
every warning set exercised so far; GCC's stricter set (not previously
exercised against this branch until this CI run) makes it a hard
Werror failure instead, so it needs fixing now rather than staying
deferred.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…palBytes Found by an independent adversarial /cpp-review pass after the branch's own final-review fix round: SetMemberRole::validate()/RemoveMember:: validate() only checked `!principal.empty()`, with no upper bound -- unlike Login::username (already bounded via auth::isValidPrincipal) and every other free-text field in this rung, which is static_assert-pinned to its column's SqlAnsiString capacity. ProjectRoleRecord::principal is a SqlAnsiString<64> column. Light::SqlFixedString's constructor is noexcept and truncates silently (std::min(N, s.size())) rather than throwing on an over-length value, so an unbounded principal produced two real bugs: - RemoveMember's lookup queries by the full, untruncated principal, which never matches the truncated stored row -- a role granted under an over-64-byte principal could never be removed through this action. - A second SetMemberRole call with the same over-length principal fails to find (and delete) the existing truncated row via the same untruncated query, then collides with the real unique index on (project_id, principal) and throws -- not idempotent/update-safe. Fix: both validate() methods now call auth::isValidPrincipal(principal) instead of a bare emptiness check, mirroring Login::validate()'s existing pattern exactly. Added a static_assert in project_admin_model.cpp pinning kMaxPrincipalBytes to ProjectRoleRecord::principal's actual capacity, matching board_model.cpp's existing convention for every other bounded field. Added two tests proving the rejection (not just its absence of a crash): an over-length principal is rejected by both SetMemberRole and RemoveMember with ValidationError, and the project's role table is left unchanged. Test count: 270/57 -> 274/59 (ladder_kanban_tests), all green, stable across repeated runs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…are/Lightweight#551 Both examples/common/CMakeLists.txt and examples/bank/CMakeLists.txt pinned v0.20260625.0, the latest tagged release -- but LASTRADA-Software/Lightweight#551 (BelongsTo<>'s implicit value-construction path silently clearing _modified, breaking Update() -- the bug this rung's MoveTaskPosition worked around) merged to Lightweight's master after that tag was cut, with no newer tag since. Pinned to master's tip commit SHA instead (GIT_SHALLOW switched to FALSE, since a shallow clone of an arbitrary commit -- as opposed to a tag/branch ref -- isn't reliably supported by all git server configurations). Verified: cleared the cached _deps checkout, reconfigured, confirmed the fetched Lightweight source is genuinely at the pinned commit, and rebuilt + reran the full ladder_kanban_tests suite against it (274 assertions / 59 test cases, all green, no regressions). Cleaned up the workaround comment in board_model.cpp's MoveTaskPosition that documented #551 as a correctness requirement for assigning loaded parent records rather than raw FK integers -- with #551 fixed, BelongsTo::operator=(S&&) (a bare key value) now marks the field modified correctly, same as operator=(ReferencedRecord&), so both forms are safe. The loaded-record assignment stays as-is (it avoids a redundant re-fetch of rows already loaded by this function's own ownership checks), but the comment now describes it as the style choice it actually is rather than a forced workaround for a bug that no longer exists. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Yaraslaut
force-pushed
the
ladder-kanban-impl
branch
from
August 17, 2026 20:39
c29e11d to
23748ea
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rung 4 of the application ladder: a multi-project kanban board backend — projects, columns, swimlanes, tasks, drag-and-drop moves with WIP limits, comments, per-project RBAC, a journal-derived activity stream, and an offline stack. This is the ladder's designated showcase — the first app where concurrency, authorization, offline, and the journal are all load-bearing at once.
Design spec:
docs/superpowers/specs/2026-08-16-kanban-rung4-design.mdImplementation plan:
docs/superpowers/plans/2026-08-16-kanban-backend.mdImplemented via Subagent-Driven Development: 20 sequential tasks, each with a fresh implementer + independent task review (+ fix round where needed), followed by one final whole-branch review and one fix round for its findings. Full process ledger:
.superpowers/sdd/2026-08-16-kanban-backend/progress.md(kept in this PR for reviewer reference — see note below).What's implemented
ProjectAdminModel/AuthModel— project lifecycle, RBAC role management, token-based login.BoardModel(the core, shared-instance model, keyed by project id) —OpenBoard/GetBoardState, column/swimlane/task CRUD,AddComment,MoveTaskPosition(WIP limits, dense position renumbering across source and destination),GetEventsSince(polling),GetActivity(journal-derived).BoardModel::execute()viarequireRole(Role), mirroringpolls::PollModel::requireAdmin()'s precedent — not a change toIAuthorizer.opId+ a server-side applied-ops ledger, checked after the role gate and before any re-validation.ThreadPoolExecutor, Local rig mode — sanitizer-friendly perTESTING.md's convention of keeping Qt stacks out of the sanitizer matrix).action_driver.hpp(SeededScript),offline_rig.hpp(OfflineRig),client_pool.hpp/convergence.hpp(ClientPool,pollUntilConverged).Framework fixes found and made along the way
IModelHolder::attachActionLognow forwards to a model-levelattachActionLogvia a newModelLevelActionLogAttachable<M>concept — closes a gap where a registry-constructed shared model never received its own journal attachment (include/morph/core/model.hpp).QtWebSocketBackend/SocketBackend::listInstancesnow stampenv.sessionlike every sibling call — a pre-existing gap that brokeSigningAuthorizer-gatedinstances()over sockets (one line per file).detail::throwIfListenFailedname collision betweenexamples/common/testkit/fault_proxy.hppandbackend_rig.hpp(same namespace, different bodies) — renamed thefault_proxy.hppcopy; would only have surfaced once a single TU included both headers.Upstream (not fixed in this branch, filed externally): a silent-data-loss trap in
Light::BelongsToassignment (raw-integer assignment to anUpdate()-bound field silently loses modification tracking) — LASTRADA-Software/Lightweight#551. This branch's own code avoids it by routing through the correct overload (board_model.cpp'sMoveTaskPosition), documented inline.Security fixes from the final whole-branch review
The final review (dispatched after all 20 tasks, on the most capable available model per the SDD process) found two Critical, cross-cutting bugs invisible to any single task's diff:
OpenBoard/GetBoardState/GetEventsSince/GetActivityhad no role check at all — any authenticated principal could read any project's board, comments, and activity journal. Fixed by gating all four withrequireRole(Role::Viewer)(or an equivalent resolved-project-id check forOpenBoard).CreateTask/AddComment/MoveTaskPositionnever re-verified their target column/task belonged to the attached project. Fixed with newrequireSwimlaneBelongsToProject/requireTaskBelongsToProjecthelpers, called unconditionally before every write.Both fixes are covered by new negative tests and were independently re-verified by a second review pass, including a live mutation test (temporarily disabling one check, confirming the corresponding test then fails, restoring it) to prove the new tests aren't vacuous.
Also fixed:
MoveTaskPosition's exactly-once ledger-hit replay was re-journaling an operation it didn't perform, compensated for by a lossy read-side dedup inGetActivity— removed both; the design spec's now-disproven premise (that the framework's own auto-append double-journals) was corrected after empirically capturing a liveFileActionLogand confirming it doesn't.Explicitly deferred (not silently dropped — see the ledger for full reasoning)
[tsan], but no CI job currently runs the ladder under a sanitizer).SqliteOfflineQueue/SyncWorker/ReconnectCoordinator/NetworkMonitorstack.MORPH_STRESS_SEEDcollapses per-client seed offsets;INFO()in a constructor doesn't survive to failure output).process_pool.hpp(a design-spec §6 item) and theclient_pool.hpp/convergence.hppinterleaved-replay convergence test it was meant to support.examples/kanban/README.md's "Deferred within this rung" section, decided during design).Filed issues
IOfflineQueuehas no depth bound or overflow policy [framework gap]QtWebSocketBackend/SocketBackend::listInstancessession-stamping (fixed in this branch)IModelHolderjournal-attachment forwarding gap (fixed in this branch)BelongsTosilent-data-loss on raw-integer assignment (upstream, not fixed here)Test results
ladder_kanban_tests: 270 assertions / 57 test cases, all green.ladder_common_tests: 295 assertions / 84 test cases, all green (no regression from the testkit rename).Process note
This PR includes
.superpowers/sdd/2026-08-16-kanban-backend/progress.md, the full SDD execution ledger — kept for reviewer reference since it documents the reasoning behind every non-obvious decision (RBAC identity,BRIDGE_MODEL_KEY-on-strong-id workaround, the two Critical findings and their fixes, all deferred items with rulings). Happy to squash/drop it before merge if preferred.