ladder: rung 4 -- kanban - #121
Open
Yaraslaut wants to merge 72 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! |
Yaraslaut
force-pushed
the
ladder-kanban-impl
branch
from
August 17, 2026 20:39
c29e11d to
23748ea
Compare
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>
Yaraslaut
force-pushed
the
ladder-kanban-impl
branch
from
August 18, 2026 19:18
76d97ab to
8109ffa
Compare
Yaraslaut
pushed a commit
that referenced
this pull request
Aug 18, 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>
Yaraslaut
pushed a commit
that referenced
this pull request
Aug 18, 2026
Phased implementation plan covering the gaps found by an audit of PR #121 against the rung's own Definition of Done: the GUI (never built, only designed), client-side offline-stack wiring, three missing tests (interleaved replay, permission revocation while attached, WAL contention), a CI leg that actually runs the concurrent stress test under ThreadSanitizer, the cascade-journaling decision, and the two previously-deferred features (automation rules, task attachments). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Aug 19, 2026
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.
Finding 1: replayMoveTaskPosition's comment claimed an `alive`/weak_ptr guard protected its .then()/.onError() lambdas, but no such variable is declared there and the lambdas never capture `this`. Traced the actual call chain (NetworkMonitor's posted, alive-checked lambda -> ReconnectCoordinator::onOnline() -> its replay dep -> SyncWorker::run() -> replayMoveTaskPosition) and confirmed it is one uninterrupted synchronous call stack with no re-entrant return to the executor in between, so no guard is needed here. Rewrote the comment to say so precisely instead of describing a mechanism that doesn't exist. Finding 2: enableOfflineQueue() posts onOnline()/onOffline() onto _executor, which main.cpp wires to a QtExecutor (Qt GUI thread), not a background worker executor as docs/spec/offline/offline.md's "NetworkMonitor callback constraint" requires. Harmless today only because the wired tryReconnect always succeeds immediately. Documented this as an explicit caveat on enableOfflineQueue()'s doc comment and a cross-reference at main.cpp's call site, so a future real (retry- capable) tryReconnect isn't wired through this same executor without first moving these callbacks to a genuine background executor -- which would otherwise freeze the GUI thread for up to ~20s. No production behavior changed; doc comments only. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… DoD Adds Q_PROPERTY int queueDepth / Q_PROPERTY int deadLetterCount to BoardBridge, both backed by Task 5's existing syncStatusChanged(int, int) signal and gated behind MORPH_BUILD_OFFLINE_SQLITE exactly like every other offline member on that class. A new _queueDepth member mirrors the value each syncStatusChanged emission site already computes, so the getter has something to read without needing a live _offlineQueue pointer. BoardView.qml gains a banner bound to boardBridge.deadLetterCount > 0, reading "N changes could not be synced" -- the exact wording examples/kanban/README.md's Definition of Done names. Extends test_board_offline_bridge.cpp with a new case that forces five cumulative replay failures (a WIP-limit-1 column already occupied, so every replay of a second task's move into it throws Conflict identically) across five online/offline flaps, then asserts deadLetterCount() == 1 and queueDepth() == 0. Also updates test_board_qml_bridge.cpp's fixed property-count assertion to branch on MORPH_BUILD_OFFLINE_SQLITE, since the offline-enabled build now legitimately exposes 5 properties instead of 3. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ge on a valid board Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…cut off Adds a TEST_CASE to test_shared_instance_lifecycle.cpp proving the per-execute authorization guarantee (docs/spec/core/shared_instances.md) holds for a member demoted mid-session on a shared, still-attached BoardModel instance: a Member-or-above write (MoveTaskPosition) is rejected immediately after SetMemberRole demotes to Viewer, and reads (GetEventsSince/GetBoardState) are rejected once the role is removed entirely via RemoveMember -- the README's explicit 'reads must also be cut off' strain point. The pre-existing enforcement (requireRole(Role::Viewer) already present in both execute(GetBoardState) and execute(GetEventsSince)) worked correctly on the real run; no change to board_model.cpp/board_model.hpp was needed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… DoD Adds a WAL-mode sibling of the existing 32-board contention TEST_CASE, proving the same no-timeout-then-committed-double-apply and dense/unique-position invariants hold under WAL as under rollback-journal mode -- fulfilling examples/kanban/README.md's 'WAL on and off' DoD wording. - ScopedShortBusyTimeout gains a useWalJournalMode flag, issuing PRAGMA journal_mode=WAL in the same post-connect hook that installs the short busy_timeout. - ScopedWalDatabaseFile runs the new scenario against its own dedicated SQLite file (a filesystem copy of DbFixture's freshly-migrated shared database) rather than the shared DbFixture file directly: switching a file *away* from WAL requires SQLite's exclusive access, which is unreachable once GlobalDataMapperPool() and MigrationManager's permanently-pinned thread-local connection both keep the shared file open for the rest of the process -- confirmed empirically (see that class's own doc comment for the full account). Both the scenario's own setup and its ScopedWalDatabaseFile destructor drain GlobalDataMapperPool()'s idle connections to guarantee every Acquire() sees the intended connection string, never a stale one left by a sibling TEST_CASE running in the same process. Verified via real ctest execution (each TEST_CASE its own process, per this project's catch_discover_tests-style registration): 8+ consecutive ctest runs of both contention tests together, and a full 96/96 pass of the kanban ladder suite. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…current-move stress test
Adds kanban-tsan, a new CI job sibling to linux-sanitizers, that builds only
MORPH_LADDER_RUNGS=kanban under the clang-tsan preset and runs
test_kanban_stress.cpp's [kanban][stress][tsan]-tagged TEST_CASE, which runs
entirely on Mode::Local's ThreadPoolExecutor{4} with no Qt/GUI involvement.
The brief's suggested `ctest -L tsan` does not work: no catch_discover_tests
call in this repo passes ADD_TAGS_AS_LABELS, so Catch2 tags are never
translated into ctest labels -- every ladder test only ever carries `ladder`
and `ladder-<rung>`. Uses `-L ladder-kanban -R ThreadSanitizer` instead,
confirmed unique across every kanban TEST_CASE name.
Also fixes a gap that would have made the new job build and pass while
providing zero real TSan coverage: no ladder CMake target ever called
apply_sanitizers() (AF_SANITIZER had exactly one call site in the whole
tree, on the header-only-adjacent morph_example). Adds the same
if(DEFINED AF_SANITIZER) apply_sanitizers(<target> ${AF_SANITIZER}) endif()
guard the ladder already uses for AF_COVERAGE/apply_coverage() to every
ladder target in cmake/morph_add_rung.cmake and examples/common/CMakeLists.txt.
No-op for every existing CI leg (AF_SANITIZER is only set by the
asan/tsan/ubsan presets, none of which built the ladder before now).
Updates examples/TESTING.md's kanban-TSan note to name the actual CI job and
selector, and fixes two stale claims found along the way: nonexistent
`stress`/`socket-only` ctest labels, and the kanban TSan leg's CI tier
(it's a separate job, not folded into ladder-tests).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t kanban TSan test The 'CI tiers' paragraph (and the kanban-stress-case paragraph before it) claimed ctest -L ladder -LE stress excludes kanban's [tsan]-tagged stress case from the ladder-tests job. This repo never sets a ctest label named stress (confirmed by grep and stated two sections earlier in the same file), so -LE stress is a no-op and excludes nothing. Corrected both paragraphs to state the actual current behavior: the kanban TSan-tagged stress test runs three times today -- uninstrumented in ladder-tests (gcc-debug, no AF_SANITIZER), uninstrumented in linux-sanitizers' clang-coverage leg (which, unlike its clang-asan/clang-tsan/clang-ubsan siblings, does build the full ladder and applies no stress exclusion in its ctest invocation), and instrumented with -fsanitize=thread only in the dedicated kanban-tsan job -- the only one of the three providing real ThreadSanitizer coverage. Re-read the full file after editing and grepped for similar exclusion/never-build claims; no other paragraph repeats the error. Documentation-only change; no CI YAML or CMake changes needed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…press rule eval during replay) Cascades are journaled with a causal parent-id, and rule evaluation is suppressed during replay (Option A of the two README step-6 names). Option B (no cascade journaling, require rule determinism) is rejected: Phase 6's rules are runtime, user-editable data, so a rule edited after some firings were recorded cannot replay deterministically from the trigger alone. LADDER.md's Journal honesty section already names causal-parent-ids as framework growth the ladder should propose, and ledger independently plans to reuse this same answer for its own rule cascades. Updates examples/kanban/README.md's build-order step 6 and docs/superpowers/specs/2026-08-16-kanban-rung4-design.md (scope statement + new §9) to state the decision as current design. The rules engine itself (Phase 6) remains deferred; only this decision, and the morph::journal framework support it requires (a LogEntry::causalParentId field, replay-mode signaling), are newly in scope. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ign spec + README A task reviewer found this load-bearing constraint (seq is sink-local and re-stamped on every forward, so it cannot key causalParentId) existed only in the prior task's report, not in either committed file. Adds a precise statement to docs/superpowers/specs/2026-08-16-kanban- rung4-design.md's §9 (new paragraph after the "what is new in morph::journal" paragraph), citing docs/spec/journal/journal.md's actual Invariants section. Adds a one-line pointer to the same effect in examples/kanban/README.md step 6, keeping the full reasoning in the design spec per CLAUDE.md's docs/spec/ authority. Also corrects the prior task's own report, which had mis-cited this claim as action_log.hpp:39-41's "Invariants" section — that file has no such section; that citation was action_log.hpp's plain seq field doc comment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ban: prove replay doesn't re-fire a cascade
- LogEntry::causalParentId (include/morph/journal/action_log.hpp): a new
additive/defaulted std::string field, empty by default ("no parent"
sentinel, mirroring idempotencyKey's shape). Set by application code
journaling a cascaded mutation to the triggering entry's own stable,
app-minted identity -- explicitly NOT LogEntry::seq, which is sink-local
and re-stamped on every forward (docs/spec/journal/journal.md's own
Invariants section), so it cannot serve as a cross-sink/cross-restart
causal key. Round-trips through toJson/fromJson; a legacy line missing
the key decodes with the empty default per the existing leniency
contract; does not bump kLogFormatVersion (additive, not breaking).
- morph::journal::isReplaying() (include/morph/journal/journal.hpp): a
thread-local replay-mode signal mirroring morph::session::current()'s
exact shape (detail::tlsIsReplaying() + RAII detail::ScopedReplayFlag).
replay() installs the guard around its dispatch loop, so isReplaying()
reads true for every entry it dispatches and false again once replay()
returns -- restored via RAII regardless of how the loop exits. Additive:
no existing replay()/Model::execute call site needed to change.
- docs/spec/journal/journal.md: new "Causal links and replay-mode
signaling" section (with a Contents entry) documenting both additions
in full, plus updates to the LogEntry field table, API reference,
Design decisions, Invariants, and Cross-references sections.
- Tests: tests/test_action_log.cpp gains framework-level coverage
(causalParentId's default/round-trip/legacy-decode behavior;
isReplaying() false outside replay(), true only for replay()'s own
dispatch loop and observable from inside a replayed Model::execute).
examples/kanban/tests/test_board_model.cpp gains the divergence test:
a hand-simulated trigger+cascade pair (Phase 6's rules engine doesn't
exist yet) linked via causalParentId, replayed through the real
morph::journal::replay() entry point, asserting the cascade's own
recorded mutation applies exactly once.
Broader morph_tests (1075 cases), ladder_kanban_tests (kanban/model and
kanban/journal tags), and morph_concepts_tests all pass; Doxygen doc
build (MORPH_BUILD_DOCUMENTATION=ON) completes with zero
warnings/errors under WARN_AS_ERROR=FAIL_ON_WARNINGS.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds RuleRecord (db/kanban_entity.hpp) plus a new LIGHTWEIGHT_SQL_MIGRATION for the rules table (src/db/schema.cpp), and the CreateRule/CreateRuleResult/ GetRules/GetRulesResult/DeleteRule/RuleView DTOs (dto/rule_dto.hpp) -- README build-order step 6 (automation rules engine), storage and API surface only. Rule evaluation is a later task. RuleMutationType is scoped to AddTag/RemoveTag per this rung's own ruling: the README illustrative example needs an 'assign to closer' concept that doesn't exist anywhere in kanban's schema/DTOs, so it is not invented here. RuleId is a new strong id (core/types.hpp), following every other kanban id's optional-based shape. Extends test_kanban_schema.cpp with a rules-table round-trip test and a CreateRule/GetRules/DeleteRule validate()/enum-string-round-trip test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… a causal parent, suppressed during replay Implements CreateRule/GetRules/DeleteRule (Manager-only create/delete, Viewer-or-above read) on BoardModel, mapping CreateRule's concrete triggerColumnId to/from RuleRecord's general conditionField/conditionValue storage shape (Task 13's deliberately-left-undone mapping). Adds a minimal task_tags join table (task_id, tag) and TaskView::tags, since RuleMutationType::AddTag/RemoveTag carry a bare tag name (no TagId or tags table existed) -- the smallest concrete storage that makes a fired rule observable. Wires evaluateRules(TaskId, ColumnId, causalParentId) into the end of execute(MoveTaskPosition), after the move's own commit. evaluateRules checks morph::journal::isReplaying() first and no-ops during replay (Phase 5's suppression). Each matching rule's mutation is applied via a new registered action, ApplyTagMutation, journaled with causalParentId set to the triggering move's own stable identity (minted from its board_events row's autoincrement id, independent of LogEntry::seq per design spec Sec 9). ApplyTagMutation is a real BRIDGE_REGISTER_ACTION action (not a bare private helper) so its LogEntry independently replays via morph::journal::replay()'s dispatcher. Adds the two brief-specified tests proving the real mechanism (not Task 12's hand-simulation): a rule firing via an actual MoveTaskPosition adds a tag and journals a causal-linked entry, and replaying that journal does not re-fire the rule (tag applied exactly once). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Extends BoardBridge/BoardPresenter with createRule/getRules/deleteRule Q_INVOKABLEs and a rules Q_PROPERTY, routing Task 14's CreateRule/ GetRules/DeleteRule through the same transport-only presenter pattern every other BoardPresenter action already follows. Adds RulesView.qml, structurally mirroring MembersView.qml (Phase 1): a flat ListView over rules, a create form (trigger-column picker reusing board.columns + mutation-type picker + tag-value field), and a per-row delete button. BoardPresenter gained a _projectId member (set by openBoard()) purely to satisfy CreateRule/GetRules' own validate() gate, which requires an engaged projectId even though BoardModel::execute() never reads it back (the handler's attach state names the board) -- not consulted for RBAC or board selection. ApplyTagMutation (Task 14's cascade-only action) is deliberately not exposed anywhere in this surface, per Task 14's hand-off note. Test: extended test_board_qml_bridge.cpp's surface-introspection case and added a createRule/getRules/deleteRule round-trip case, mirroring ProjectAdminBridge's listRoles/setMemberRole/removeMember test shape. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Task 15 review found two issues: 1. RulesView.qml was never reachable from the running app. Adds a "Rules" header button to BoardView.qml that opens RulesView inside a Popup, mirroring TaskDetailPopup's own open-on-demand mechanism exactly (same Popup property set: modal, focus, centered x/y). Wired into BoardView.qml rather than ProjectListView.qml (unlike MembersView's own precedent) because a rule's triggerColumnId picker needs the open board's own board.columns, only available once a board is already open. boardBridge is bound straight through to the same bridge instance BoardView.qml already holds; no new bridge/presenter surface was needed since Task 15 already exposed everything RulesView.qml uses. 2. docs/superpowers/specs/2026-08-17-kanban-gui-design.md line 320 claimed automation rules have no backend surface, which Task 14 (CreateRule/GetRules/DeleteRule, rule evaluation) and Task 15 (RulesView.qml) have since made false. Updated to state the current status; the attachments half of that line is unchanged since Phase 7's backend genuinely does not exist yet. Updated test_gui_qml_smoke.cpp's comments to note the existing 'board view loads standalone' case now also exercises RulesView.qml (no new TEST_CASE needed, since it's exercised transitively). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…/RemoveAttachment) Metadata-only task attachments (README build-order step 8): AddAttachment records a task-scoped AttachmentRecord after a separate HTTP side channel (a later task) has already uploaded bytes and returned a storageKey; GetAttachments lists a task's attachments; RemoveAttachment deletes the metadata row (not the underlying bytes). AttachmentRecord mirrors CommentRecord's exact shape (task-scoped child table, BelongsTo<&TaskRecord::id>). AddAttachment/RemoveAttachment gate at Role::Member, GetAttachments at Role::Viewer -- the same RBAC bar AddComment/GetBoardState already use, since attachments are task-content like comments, not board administration like CreateRule/DeleteRule (Role::Manager). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…capacity Task 16 review finding: AddAttachment::validate() only checked non-empty on filename/contentType/storageKey, never their length against the SQL column capacity (Varchar(255)/Varchar(127)/Varchar(255)). Every other bounded-SqlAnsiString-backed DTO field in this codebase pairs its length bound with a validate() check and a static_assert tying the DTO constant to the column's own .capacity() -- this is the same class of bug already found and fixed once before in SetMemberRole/RemoveMember's principal check: an over-length value silently truncates on write, and a later equality lookup against the caller's untruncated string then never matches the truncated stored row. Adds kMaxAttachmentFilenameBytes/kMaxAttachmentContentTypeBytes/ kMaxAttachmentStorageKeyBytes (255/127/255) to attachment_dto.hpp, wires them into AddAttachment::validate(), and adds three matching static_asserts in board_model.cpp tying each constant to AttachmentRecord's actual field capacity. Extends the existing AddAttachment/GetAttachments/RemoveAttachment validate() test with over-length-rejection assertions for all three fields plus an at-exactly-max-length control case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…with its own size bound
Adds kanban::http::AttachmentServer, a hand-rolled HTTP server over
QTcpServer/QTcpSocket (no QHttpServer dependency exists anywhere in this
tree) implementing the two routes README build-order step 8 calls for:
- POST /attachments: Authorization: Bearer <token> required, raw body
bytes, X-Attachment-Content-Type header. Returns {"storageKey": "..."}
on success -- the opaque key Task 16's AddAttachment action is then
called with to commit the metadata row.
- GET /attachments/{storageKey}: same auth requirement, streams the
stored bytes back with the recorded content type, or 404 if the key
names no stored blob (including the dangling-metadata-row case: a
storageKey committed via AddAttachment that no upload ever produced).
Security-relevant design choices (see task-17-report.md for full
reasoning): hand-rolled listener over QHttpServer (smaller, fully
auditable, no new Qt module for two fixed routes); storageKey is a random
64-hex-char token (std::random_device), not a content hash, to avoid a
dedup-confusion/probing-oracle surface; storageKey is validated to that
exact shape before ever reaching a filesystem path, closing off path
traversal in one check; authentication happens before any route logic,
size check, or body byte is read; the size bound is enforced both against
the declared Content-Length and against the running total actually
received, so a dishonest Content-Length can't be used to bypass it; one
request per connection with Connection: close, no keep-alive/chunked
encoding.
Wired into src/server/main.cpp alongside the existing QtWebSocketServer,
constructing its TokenVerifier from the exact same tokenSecret/hmacSha256
App's own KanbanAuthorizer already uses -- not a second, independently-
sourced secret. New KANBAN_ATTACHMENT_PORT env var (default 8769),
parsed with the same std::from_chars discipline as the existing
KANBAN_PORT.
Tests (examples/kanban/tests/test_attachment_server.cpp): 10 new test
cases covering valid upload, oversized upload (413, including a
dishonest-Content-Length variant caught during review), GET of an
existing key, GET of a nonexistent key, a malformed/garbage-input
robustness test (9 adversarial byte strings, no crash/hang), the
dangling-metadata-row scenario against a real BoardModel::AddAttachment
call, and three unauthenticated/forged-token rejection variants. No
MORPH_BUILD_FUZZERS harness added (that apparatus is Clang/libFuzzer-only
and morph-framework-scoped; a thorough Catch2 malformed-input test covers
the same robustness requirement for this app-level parser instead).
docs/spec/security.md was not touched -- it documents morph's own
session/RemoteServer trust model and has no side-channel enumeration
list this app-level example server belongs in.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…arer token
AttachmentServer::handleRequest authenticated (verified the bearer token
was validly signed and unexpired) but never authorized: any principal
holding a valid token for ANY project could GET any attachment blob by
storage key, softened only by the key's unguessability rather than a real
authorization boundary. This contradicted docs/spec/security.md's "Opaque
model ids" section.
Fix: handleRequest now captures the verified SessionToken (not just a
bool) from TokenVerifier::verify(), and GET /attachments/{storageKey}
resolves the key's owning db::AttachmentRecord -> db::TaskRecord ->
project, then requires the verified principal hold at least Role::Viewer
there -- mirroring BoardModel::execute(const GetAttachments&)'s own
requireRole(Role::Viewer) + requireTaskBelongsToProject gate.
loadCallerRole is duplicated (not shared/exported) from board_model.cpp,
following that file's own established "design spec §3: not shared code,
each model gets its own copy" convention (project_admin_model.cpp already
has an independent second copy). A nonexistent storageKey and an
existing-but-unauthorized one both return 404, so a caller can never
distinguish "doesn't exist" from "exists but you have no access."
POST /attachments is left as documented-gap, not an added check: it mints
a fresh storageKey with no AttachmentRecord yet to resolve ownership
from, so there is nothing meaningful to authorize at upload time; the
real boundary for committing an attachment is AddAttachment's existing
requireRole/requireTaskBelongsToProject gate, and the real boundary for
reading one is this GET fix. Documented explicitly in the class doc
comment.
New test: a principal with a validly-signed token for her own,
completely separate project gets 404 (not 200) attempting to GET another
project's committed attachment. The existing positive-control download
test is rewritten to actually commit the storageKey via AddAttachment
first (previously it downloaded an uncommitted upload directly, which
now correctly 404s, since an uncommitted blob has no project to check a
role against). The pre-existing never-uploaded-404 test gains a
DbFixture, since every GET now performs an authorization DB lookup.
ctest -L ladder-kanban: 119/119 passed (was 118; +1 net-new test case).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Wires Task 16's AddAttachment/GetAttachments metadata actions and Task 17's AttachmentServer HTTP side channel into the GUI: - BoardPresenter gains addAttachment()/getAttachments(). addAttachment() returns its own Completion<Ack> (not a shared signal) so a bridge-level upload can chain its own outcome without cross-attribution between overlapping calls, mirroring moveTaskForReplay/getEventsSinceForPolling. - BoardBridge gains an `attachments` Q_PROPERTY plus uploadAttachment(), downloadAttachment(), getAttachments(), and setAttachmentServerUrl() Q_INVOKABLEs. uploadAttachment() reads a local file, POSTs it to AttachmentServer via QNetworkAccessManager (X-Attachment-Content-Type header, no multipart, per the server's own documented protocol), then commits its metadata via AddAttachment on success. downloadAttachment() GETs a storageKey's bytes and writes them locally, treating a 404 (the server's real per-project authorization gate, not just authentication) the same as any other failure via failed(QString). The bearer token is read from Bridge::defaultSession().token -- the same session Login already installs -- rather than inventing a new auth-storage mechanism. - TaskDetailPopup.qml gains an attachment list and "Attach file"/ "Download" buttons backed by QtQuick.Dialogs' FileDialog, alongside the existing comment section. - gui/main.cpp gains an --attachment-server <url> flag (mirroring --server), defaulting to the server's own KANBAN_ATTACHMENT_PORT default (8769) when --server is given; left unset in Local mode, which runs no AttachmentServer of its own. - kanban's CMakeLists.txt links Qt6::QuickDialogs2 onto ladder_kanban_qml (not the consuming gui/tests executables -- qt_add_qml_module's own import-scanning needs the plugin visible as a dependency of the QML module itself) so FileDialog resolves at runtime, not just at AOT compile time. Test: extends test_board_qml_bridge.cpp with an end-to-end upload -> commit -> list -> download round trip against a real AttachmentServer, and a no-server-configured failure case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Task 18 review finding fix: the only pre-existing failed()-signal test for attachments tested upload's pre-flight guard (no attachment server configured), which never issues an HTTP request. Nothing exercised downloadAttachment() against a real, running AttachmentServer returning a genuine 404, for either of Task 17's two collapsed-to-one-status-code causes. Adds two tests to test_board_qml_bridge.cpp, both driving a real kanban::http::AttachmentServer: - "BoardBridge::downloadAttachment reports failed() for a storageKey that was never uploaded (a real 404 from a real AttachmentServer)" -- mirrors test_attachment_server.cpp's own nonexistent-key 404 case one layer up at the bridge; asserts failed() fires, attachmentDownloaded does not, and no file is left at the destination path. - "BoardBridge::downloadAttachment reports failed() the same way for a storageKey that belongs to a DIFFERENT project the caller has no role on (authenticated, not authorized)" -- mirrors test_attachment_server.cpp's cross-tenant regression test, wired through two real BoardBridge instances (alice uploads and commits an attachment; mallory, a separately signed principal with no role on alice's project, tries to download it). Proves the GUI collapses both causes to the same failed() behavior, per the server's deliberate one-status-code security design. Both reuse this file's existing kAttachmentTestSecret/ freshAttachmentStorageDir/makeAuthedRigWithToken/seedProject helpers --no new scaffolding needed. Full kanban suite: 123/123 passed (121 pre-existing + 2 new), no regressions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…mplemented
- Test 19's own re-read of README.md's Definition of Done against every
prior task's shipped work found two real gaps no task-scoped review
caught (each is cross-cutting, not owned by any single task):
- The offline tests never asserted the framework's own morph::observe
metrics (queueDepth, reconnectAttempts, reconnectOutcome), though the
DoD explicitly requires it. Added a dedicated test
(test_board_offline_bridge.cpp) installing a MetricSink via
ScopedObserveOverride around the existing offline queue/reconnect
flow and asserting all three metrics fire.
- The 'demo scripted' DoD claim for the kill-the-network scenario was
unsubstantiated -- kanban has no --seed CLI path (LADDER.md's
ladder-wide convention), only tests. Corrected the wording to state
this accurately rather than claim a demo that doesn't exist.
- The HTTP attachment side channel's 'joins the fuzz corpus' claim was
also stale: Task 17 deliberately used a dedicated adversarial Catch2
test instead (no MORPH_BUILD_FUZZERS harness targets HTTP parsing).
Corrected to describe the actual, already-reviewed substitution.
- Updated the 'Deferred within this rung' section (steps 6/8 were
marked deferred; both are now implemented) and the top status line
to reflect current, complete state.
124/124 kanban tests passing (123 prior + 1 new metrics test, verified
non-flaky across 5 repeated runs).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ction The upload header X-Attachment-Content-Type was stored unvalidated, written verbatim to a .contenttype sidecar file, read back verbatim, and interpolated directly into the GET response's Content-Type: header. parseHeaders only splits on \r\n, so a header value containing a bare \n (no preceding \r) survives parsing intact as part of the value -- and a lenient HTTP client/intermediary that honors bare-LF line termination could turn an upload like `X-Attachment-Content-Type: text/plain\nX-Injected: evil` into an injected header on every subsequent GET of that attachment. Add isPlausibleMediaType()/sanitizedContentType() in attachment_server.cpp: a strict type/subtype allowlist ([A-Za-z0-9!#$&^_.+-] per half, both non-empty, one '/'), capped at kanban::kMaxAttachmentContentTypeBytes (the existing Task 16 bound from attachment_dto.hpp, reused rather than duplicated). Anything that fails substitutes the existing default application/octet-stream -- fails closed rather than rejecting the upload, since content type is convenience metadata, not a security-critical field in its own right. Applied at both the point the header is captured on upload (before it is even kept on ConnectionState, let alone written to the sidecar file) and the point the sidecar is read back for GET (defense in depth: the sidecar is a plain file on disk that could in principle be written by other means). Adds a regression test that uploads with a bare-LF-bearing X-Attachment-Content-Type, downloads it back, and asserts against the raw response bytes that no injected header line appears anywhere and the Content-Type falls back to the safe default. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Running ladder_kanban_tests.exe as one process (not via ctest, which runs each TEST_CASE in its own process via catch_discover_tests) crashes reproducibly at a location that shifts with Catch2's test-run order -- the signature of process-wide state leaking across TEST_CASEs. Not a regression from this branch: examples/common/testkit/db_fixture.hpp (the suspected culprit -- its ensureConnectionConfigured() gates process-wide SqlConnection/MigrationManager singleton setup behind a static-once guard never reset per TEST_CASE) is untouched by any commit on this branch; its last change predates this plan (rung 0, 557b892). This plan roughly doubled the TEST_CASE count compiled into the single ladder_kanban_tests binary, which is what made the pre-existing bug newly observable. CI is unaffected: cmake/morph_add_rung.cmake's catch_discover_tests(...) call registers one CTest entry per TEST_CASE, each launched by CTest as its own separate process, so the singleton state never survives across tests there. Documents the finding for human triage into a tracked issue; does not attempt to fix the underlying testkit bug, which is out of scope for a kanban-focused plan and deserves its own scoped investigation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ssManager
Task 18's BoardBridge::uploadAttachment/downloadAttachment use
QNetworkAccessManager (board_qml_bridge.hpp/.cpp) to talk to Task 17's
HTTP attachment side channel. This target only ever picked up
Qt6::Network *transitively*, by linking morph::ladder_kanban_lib
(morph_add_rung.cmake's own conditional block for that) -- and
ladder_kanban_lib does not exist at all under Emscripten (persistence
is server-side only for a WASM client). Native builds passed by
accident through that transitive chain; the WASM CI job
('Build the ladder's WASM clients') failed with
"'QNetworkAccessManager' file not found", since gui_lib had no
transitive path to Qt6::Network there at all.
Links Qt6::Network directly and unconditionally onto
ladder_kanban_gui_lib, gated only on MORPH_BUILD_QT (matching Task 17's
identical gating for ladder_kanban_lib's own Qt6::Network need) --
this dependency is needed by shared presenter/bridge code both the
desktop and WASM clients link, on every platform, not just natively.
Verified: full kanban test suite still 125/125 after reconfigure +
rebuild on the native (clangcl-release) tree. Could not run the actual
Emscripten toolchain locally; confirmed via the CI workflow
(wasm-ladder.yml) that MORPH_BUILD_QT=ON is set there (so this fix's
guard condition is true) and that Qt6::Network is part of qtbase's base
install (unlike qtwebsockets, an explicitly-listed add-on module), so
no CI workflow change is needed alongside this CMake fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…op useless cast Kanban/ThreadSanitizer, Linux/clang-coverage, and Linux/all-optional- features(clang) all failed on the same underlying bug: a heap-use- after-free in QtExecutor::post()/CompletionState::setException, confirmed by TSan and reproduced as a plain SIGSEGV in the two uninstrumented builds. Root cause: QtExecutor::post() (qt_executor.hpp) uses Qt::QueuedConnection -- it enqueues a callback on the Qt event loop and returns immediately, it does not run it. ThreadPoolExecutor's destructor joins every worker thread, which guarantees every post() call was *made* before it returns, but not that the resulting event was *pumped*. Bridge::executeVia (bridge.hpp) chains two Completion objects per action, so a single failed action can enqueue a *second*, nested post() from inside the first posted callback -- invisible to a caller's own pumpUntil(outstanding == 0), which only tracks the outer completion. BackendRig could observe done and tear itself down, freeing _qtExecutor, while that inner post's callback was still sitting undelivered in the Qt event queue; when it finally ran, it touched freed memory. Fix: BackendRig::~BackendRig() now resets _workerPool explicitly (forcing every pool-issued post() to have already happened) and then drains the Qt event loop for a few bounded slices -- long enough for a nested post to both arrive and run -- before the rest of member destruction (including _qtExecutor) proceeds. This mirrors QtWebSocketServer::closeGracefully()'s own established processEvents-drain pattern, just applied to the worker-pool/executor pair instead of the socket server. Also drops a redundant static_cast<int> in test_board_concurrent_drag.cpp that GCC's -Wuseless-cast (clang has no equivalent warning) correctly flagged as dead code: the expression was already int-typed before the outer cast. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Kanban/ThreadSanitizer's CI leg (Task 10, this branch) is the first CI job to ever run kanban's Qt-linked concurrent tests under real ThreadSanitizer instrumentation, and surfaced two framework-level bugs neither specific to kanban: 1. A heap-use-after-free in QtExecutor::post()/Completion's nested post chain, racing BackendRig's teardown -- fixed on this branch (917ea54) and verified via a standalone repro (5/5 crashes without the fix, 10/10 clean with it, at kanban's own real concurrency scale). Filed as #127 for the framework maintainers, since the underlying Completion/QtExecutor lifetime gap affects every consumer, not just this test harness. 2. A second, distinct race that survives fix #1: 165 ThreadSanitizer warnings centered on Qt's own internal QCallableObject/invokeMethod machinery, plus a load-bearing discovery that the kanban-tsan CI job's own comment (claiming this test has "no Qt/GUI involvement") is factually wrong -- BackendRig's Mode::Local unconditionally builds a real QtExecutor. Not resolved here: two repro attempts (a 200-chain scale-up of fix #1's own repro, and a targeted publishResult fan-out repro) both ran clean, but neither had ThreadSanitizer available locally to confirm the race itself, only to rule out a plain crash. Filed as #128 with full evidence and suggested next steps (confirm/rule out Qt's own lack of TSan instrumentation; fix or retire the CI job's incorrect premise). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tId instead of asserting
Linux / all optional features (gcc) failed with a SIGABRT: libstdc++'s
hardened build asserts inside std::optional<int64_t>::operator*() when
the optional is disengaged. Root cause: BoardBridge::openBoard()
parses an unparseable QString into a default-constructed, disengaged
ProjectId{} (board_qml_bridge.cpp's own parseId<ProjectId>(), which
returns IdT{} rather than throwing, per its own doc comment). That
action reaches morph::bridge::BridgeHandler::execute() before
BoardModel::execute(OpenBoard)'s own hasValue()-guarded validation
ever runs -- ActionKeyTraits<OpenBoard>::key() computes the routing
key that attach relies on, earlier in the dispatch pipeline, and used
to dereference action.projectId unconditionally.
bridge.hpp's own BridgeHandler::execute() already wraps key extraction
in a try/catch specifically so a throwing ActionKeyTraits::key() routes
to the caller's onError() rather than escaping (see that call site's
own comment) -- this fix uses exactly that sanctioned seam, throwing
kanban::ValidationError (mirroring BoardModel::execute(OpenBoard)'s
identical "projectId is required" rejection for the case where the
key never even reaches that check) instead of dereferencing.
"BoardBridge relays failed() on a bad projectId" (test_board_qml_bridge.cpp)
already exercised this exact path and expected a clean failed() signal;
it just happened to only crash under libstdc++'s hardened assertions
(the gcc CI leg), not under every other job's libc++/MSVC build, so it
went undetected until this branch's CI matrix widened enough to catch it.
Added a dedicated, DB-free regression test
(test_board_model.cpp's "ActionKeyTraits<OpenBoard>::key() rejects a
disengaged projectId instead of asserting") that exercises
ActionKeyTraits<OpenBoard>::key() directly -- a pure function, no
database needed -- proving the fix without depending on a live DB
connection (this session's local environment has an unrelated,
pre-existing SQL Server credential gap that blocks every DB-touching
kanban test; this new test sidesteps it entirely and passes cleanly).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Yaraslaut
force-pushed
the
ladder-kanban-impl
branch
from
August 20, 2026 14:43
a2249d1 to
64ef2e4
Compare
…es morph#128's blocking symptom)
morph#128 found the kanban-tsan CI job's own premise false: its comment
claimed test_kanban_stress.cpp ran with 'no Qt/GUI involvement', but
BackendRig's Mode::Local unconditionally constructs a real
morph::qt::QtExecutor for client-facing callback delivery, and all 165
ThreadSanitizer warnings the job produced bottomed out in genuine
Qt-internal frames (QMetaObject::invokeMethod, QCallableObject,
QObject::event) reached through it -- undetectable as real bugs or
false positives from outside a TSan-instrumented Qt build (which this
CI's prebuilt Qt package is not).
Rather than resolve that ambiguity (would need a TSan-instrumented Qt
build, or a scoped suppressions file accepting unverified risk), the
test is rewritten to drive BoardModel through a bare
morph::bridge::Bridge wrapping a morph::backend::LocalBackend
directly -- the exact pattern tests/test_concurrency_invariants.cpp's
own concurrent-dispatch test already uses -- with a two-line
InlineExecutor (post(fn) { fn(); }) standing in for QtExecutor on the
client-callback side, and a plain waitUntil busy-poll instead of
pumpUntil/awaitQt. BoardModel's own requireRole/session checks and
ModelKeyTraits<BoardModel>'s shared-per-project instance semantics are
backend-agnostic (session context read directly from
morph::session::current(); shared-instance keying is a Bridge-level
mechanism, registerModelShared), so what design spec §8 actually
requires this test to check (dense/unique positions, no task lost or
duplicated under concurrent MoveTaskPosition calls) is unchanged --
only the plumbing that delivers callbacks changed. Zero Qt frames
remain anywhere in this test's call graph, making the kanban-tsan
job's 'no Qt/GUI involvement' premise genuinely true.
This does not resolve morph#128's own open framework question (real
bug vs. Qt-instrumentation gap) -- it sidesteps it for this one test,
leaving the issue open upstream for whoever audits other Qt-linked
concurrent code under a real sanitizer. Updated
docs/superpowers/plans/2026-08-19-kanban-tsan-ci-findings.md,
examples/TESTING.md, and .github/workflows/ci.yml's own comment to
match.
Verified locally: ladder_kanban_tests full suite passes (122 test
cases, 875 assertions), the stress test itself stable across 4 repeat
runs (29 assertions each), zero compiler warnings under clang-cl
-Weverything -Werror.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…erhead The rewritten test (c48514c) genuinely ran and completed under real ThreadSanitizer instrumentation in CI -- no TSan warnings at all, a plain REQUIRE failure: the 200-action (4 clients x 50 actions) workload's outstanding-completions count never reached 0 within the original 20s budget, finishing at ~32s instead. Root cause: this rewrite delivers .then()/.onError() directly on whichever real ThreadPoolExecutor worker resolves each completion (InlineExecutor), unlike the original Qt-based version's client-side callback delivery -- combined with TSan's own well-documented 5-15x instrumentation overhead, 20s (never actually exercised against real TSan overhead before, since CI never set MORPH_LADDER_DEADLINE_MS for this job either) was too tight. Raised to 90s -- comfortably past the observed ~32s, generous enough for slower CI runners, while still catching a genuine hang/deadlock well within the job's own timeout. Re-verified locally: stable across 3 repeat runs, full kanban suite unaffected (875 assertions, 122 test cases). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.