fix(kanban): gate dependencies on exact parent results - #97151
MisterNunm wants to merge 2 commits into
Conversation
alt-glitch
added
type/bug
andrexibiza
left a comment
andrexibiza
left a comment
There was a problem hiding this comment.
Reviewed exact head 73fca308cca0c51ef9a1e36386aebcfb7988e2b4 against live main@9978706e9303dbf990d90e744b131361449d73b9 (merge base 35328345d5e3b5badc47271bdb8828e1fd2d25f4; this head is 1 commit ahead / 2 behind current main). I read the full six-file diff, the current Kanban DB/CLI/tool implementations, the dispatch/claim enforcement path, exact-head workflow state, #67132, and the live competing/predecessor implementations #67156, #68850, and superseded #97141.
The core shape is strong: the predicate is explicit rather than inferred from prose, legacy NULL edges remain backward-compatible, one shared dependency evaluator reaches recompute/manual promotion/dashboard paths, and claim_task() revalidates inside the write transaction immediately before ready -> running. That last check is particularly important; it closes stale/manual-ready state rather than trusting an earlier projection.
P1 — an idempotent create can silently discard the new gate contract
hermes_cli/kanban_db.py:create_task() still returns an existing non-archived task immediately when idempotency_key matches:
if idempotency_key:
row = conn.execute(
"SELECT id FROM tasks WHERE idempotency_key = ? "
"AND status != 'archived' "
"ORDER BY created_at DESC LIMIT 1",
(idempotency_key,),
).fetchone()
if row:
return row["id"]That happens before this PR validates/persists required_parent_results. So an existing child created with a legacy edge (or a different required result) can be returned as success when a retry now requests required_parent_results={qa: "ready_to_deploy"}. If the existing parent is done with qa_failed, the existing legacy edge is already satisfied and that child can remain runnable even though the retry explicitly requested the stricter contract.
This is a proof-scope mismatch at the idempotency boundary: the request says “this exact dependency contract,” but the dedup key proves only “some non-archived task with this key exists.” Please make an idempotency hit compare the safety-relevant dependency contract before returning it. Exact equality may return the existing task; a narrower/different existing contract should fail closed (or be upgraded atomically only if that is an explicit API contract). Add a regression for legacy/different gate -> same idempotency key -> stricter gate proving no child becomes claimable under the stale contract.
P1 — there are three active authorities for the same dependency-success boundary; landing needs one semantic owner
This PR is usefully different from the prior work, but it is not isolated from it:
peacockesqowns #67132 and the direct approval-specific implementation #67156 (task_links.gate_type, structured terminal-runapproved: true, reviewer identity/ambiguity handling).chahababaowns #68850, the alternative semantic-verdict implementation over the same create/link/recompute/claim/manual/dashboard boundary.MisterNunmowns closed #97141 and this republished exact-result implementation #97151.
#97151's exact result predicate is complementary to #67156's approval contract; #68850 is a competing inference model. They all modify the same central dependency authority and several of the same files. Two independent columns/evaluators cannot land without declaring composition/precedence, and the inference path in #68850 should not silently coexist with this PR's explicit-contract guarantee. Please establish the merge/supersession graph and converge on one typed edge-policy evaluator, while preserving all three contributors' credit. #97141 is correctly described here as superseded publication history, not a second active implementation.
P1 landing gate — this re-grows three formal fracture targets
The behavior is worth keeping, but the current placement adds new authority directly into three tracked >2K surfaces: hermes_cli/kanban_db.py (#78632), hermes_cli/kanban.py (#79940), and tools/kanban_tools.py (#79982). The current diff itself reaches past lines ~7,000, ~2,100, and ~2,300 respectively. For kanban_db.py, the accepted decomposition already has a schema/model owner in #79893, transaction owner #79894, and claim/lock owner #79895; this PR's claim-time gate currently edits the exact lifecycle #79895 extracts.
Please preserve this semantic fix but restack it onto bounded owners rather than making the monolith the new authority again. At minimum the schema belongs with the schema owner and the claim-time enforcement must compose with the claim owner. The CLI/tool projections likewise need their #79940/#79982 fracture order called out rather than accumulating new behavior in those files.
Attribution / exact-object acceptance
The sole commit is TJKIM <nunm1107@hotmail.com> and current main has no contributors/emails/nunm1107@hotmail.com mapping. Please add the mapping to MisterNunm; no history rewrite is needed.
Exact-head hosted acceptance is also still absent: CI 33172192385, Docker 33172191821, and Nix 33172191907 are all action_required; the CI run currently contains zero jobs. The reported 388-test local receipt is useful evidence, but it is not an exact-head hosted landing receipt, and this PR has only this one commit, so the per-commit green gate is currently unmet.
One additional regression I would add while touching this: edit_completed_task_result() can mutate tasks.result after a parent is already done. Claim-time revalidation makes the obvious pre-claim case safe, but tests should pin both matching -> edited away -> held and nonmatching -> edited to match -> eligible so the fact that result is now policy-bearing does not remain implicit.
This is a good direction: explicit edge contracts + mutation-time revalidation are much stronger than reconstructing semantic success from titles or prose. Close the idempotency hole, settle the competing-policy/landing graph, restore attribution, and get the exact commit through hosted CI, and the underlying primitive is solid. 🚀
This branch has not been deployed
3 participants
What does this PR do?
Kanban dependency release currently treats a terminal parent as successful even when the parent recorded a machine-readable failure result. That can release QA-gated deployment or release children after QA has completed with
qa_failed.This PR adds an opt-in, nullable edge-level
required_parent_resultpredicate. A gated edge is satisfied only when the parent is exactlydoneandtasks.resultexactly matches the declared value. Existing NULL edges keep their currentdone/archivedbehavior.The predicate is explicit and generic: it does not infer approval from task titles, assignees, summaries, comments, or metadata prose. Idempotent create retries also compare the exact parent/result dependency contract and fail closed instead of silently returning a task with stale or weaker edges.
This supersedes the closed publication attempt #97141; the implementation is republished from the requested fork branch name.
Related Issue and Semantic Ownership
Related to #67132.
This overlaps with #67156 and #68850 but uses a different contract:
approved: truemetadata.tasks.resulton each selected edge, supporting contracts such asready_to_deploywithout changing ordinary dependencies.Proposed landing graph:
peacockesq's credit.chahababa's credit.This PR does not import code from the other active PRs. Consolidation should be coordinated with their authors before landing. The current implementation also still needs to be restacked with the accepted fracture owners in #79893, #79894, #79895, #79940, and #79982 rather than establishing new long-term authority in the monoliths.
Type of Change
Changes Made
hermes_cli/kanban_db.py: add the nullable schema migration, shared dependency predicate, and enforcement across create/link/recompute/claim/reclaim/unblock/manual promotion paths.hermes_cli/kanban_db.py: make idempotent create retries compare exact parent IDs and result predicates before reusing an existing task.hermes_cli/kanban.py: add typed create/link CLI options.tools/kanban_tools.py: expose typed create/link tool schema fields and handlers.plugins/kanban/dashboard/plugin_api.py: accept the predicate and use the same blocker logic for manual/dashboard promotion.tests/hermes_cli/test_kanban_required_parent_result.py: cover exact success, failure/null/case mismatch, archived gated parents, legacy compatibility, dispatcher ticks, force behavior, migration, CLI/tool surfaces, exact and conflicting idempotent retries, and post-completion result edits.tests/plugins/test_kanban_dashboard_plugin.py: cover dashboard create/link persistence and manual ready rejection.contributors/emails/nunm1107@hotmail.com: map the commit email toMisterNunmwithout rewriting history.How to Test
Run the complete Kanban regression family:
./scripts/run_tests.sh tests/hermes_cli/test_kanban*.py tests/tools/test_kanban_tools.py tests/plugins/test_kanban_dashboard_plugin.py -q --file-retries 0Verified result on current head
ef4ac92a: 394 passed, 0 failed, 1 Windows-only skipped across 49 files.Run the contributor mapping tests:
Verified result: 7 passed, 0 failed.
Run Ruff on the changed Python files. Verified result:
All checks passed!.Run
git diff --check origin/main...HEAD. Verified clean.The repository-wide suite was attempted but is not claimed green. A worktree-local optional-dependency environment allowed the Anthropic and ACP tests to pass, but unrelated tests then hit Linux Unix-socket path limits, desktop-environment assumptions, and live-runtime guards. Updater tests also replaced that local virtual environment during the parallel run, invalidating the remaining repository-wide receipt. The disposable environment was removed and the complete Kanban family was rerun cleanly through the canonical wrapper.
Checklist
Code
pytest tests/ -qand all tests pass — the complete 49-file Kanban scope passes; the repository-wide run was not a valid green receipt for the environment reasons documented aboveDocumentation & Housekeeping
cli-config.yaml.example— N/A; no configuration key was addedCONTRIBUTING.md/AGENTS.md— N/A; no repository workflow convention changedScreenshots / Logs
N/A — this is a database and dispatcher invariant with automated regression coverage.