fix: scope OpenSpec sentinel per-change to prevent stale task queue#151
fix: scope OpenSpec sentinel per-change to prevent stale task queue#151vishnujayvel wants to merge 1 commit intoasklokesh:mainfrom
Conversation
|
All contributors have signed the CLA. Thank you. |
b28c63f to
53ff1d7
Compare
|
I have read the CLA Document and I hereby sign the CLA |
Decision Tree: Sentinel Logic Across All ScenariosThis diagram shows how the fix handles every combination of flowchart TD
START["loki start invoked"] --> HAS_FLAG{"--openspec\nprovided?"}
%% === No --openspec branch ===
HAS_FLAG -->|No| CLEANUP["CLI cleanup block\n(autonomy/loki:941-965)"]
CLEANUP --> DEL_SENTINEL["Delete sentinel\nDelete openspec-tasks.json\nDelete openspec/ dir"]
DEL_SENTINEL --> PURGE_PENDING["Purge source:openspec\nfrom pending.json"]
PURGE_PENDING --> RUN_SH_NO["run.sh starts\npopulate_openspec_queue()"]
RUN_SH_NO --> CHECK_TASKS_FILE{"openspec-tasks.json\nexists?"}
CHECK_TASKS_FILE -->|"No (deleted)"| SKIP_CLEAN["Return early\n--- No stale tasks served ---"]
%% === --openspec provided branch ===
HAS_FLAG -->|Yes| EXPORT["Export OPENSPEC_CHANGE_PATH\nRun adapter (regenerates artifacts)"]
EXPORT --> RUN_SH_YES["run.sh starts\npopulate_openspec_queue()"]
RUN_SH_YES --> CHECK_TASKS_YES{"openspec-tasks.json\nexists?"}
CHECK_TASKS_YES -->|No| SKIP_NO_TASKS["Return early\n(adapter error?)"]
CHECK_TASKS_YES -->|Yes| CHECK_SENTINEL{"Sentinel file\nexists?"}
%% --- No sentinel (first ever run) ---
CHECK_SENTINEL -->|No| POPULATE["Populate pending.json\nfrom openspec-tasks.json"]
POPULATE --> WRITE_SENTINEL["Write change path\nto sentinel"]
WRITE_SENTINEL --> DONE_FRESH["--- Fresh population done ---"]
%% --- Sentinel exists ---
CHECK_SENTINEL -->|Yes| READ_SENTINEL["Read stored path\nfrom sentinel"]
READ_SENTINEL --> COMPARE{"Stored path ==\ncurrent path?"}
%% Same change (crash-restart)
COMPARE -->|"Match\n(same change)"| SKIP_RESUME["Skip repopulation\n--- Progress preserved ---\nCompleted tasks stay gone\nfrom pending.json"]
%% Different change
COMPARE -->|"Mismatch\n(change switched)"| PURGE_OLD["Purge all source:openspec\ntasks from pending.json"]
PURGE_OLD --> REPOPULATE["Repopulate from new\nopenspec-tasks.json"]
REPOPULATE --> UPDATE_SENTINEL["Overwrite sentinel\nwith new path"]
UPDATE_SENTINEL --> DONE_SWITCH["--- Clean switch done ---\nOld tasks gone, new loaded"]
%% Styling
style SKIP_CLEAN fill:#2d6a2d,color:#fff
style DONE_FRESH fill:#2d6a2d,color:#fff
style SKIP_RESUME fill:#2d6a2d,color:#fff
style DONE_SWITCH fill:#2d6a2d,color:#fff
style SKIP_NO_TASKS fill:#8b6914,color:#fff
Scenario Coverage Matrix
Why the sentinel is NOT deleted when
|
|
Hey @vishnujayvel -- great contribution! The root cause analysis is thorough, the three-pronged fix is well-structured, and the decision tree comment you posted is genuinely excellent documentation. The crash-restart fix in the second commit shows good attention to edge cases. Thank you for taking this on. That said, I found a few issues during review that I'd like addressed before merging. Tagging you to take a look. Requested Changes1. Purge only targets pending.json, misses completed/in-progress (bug)The Python purge blocks only filter The purge needs to clean all three queues (pending, completed, in-progress), not just pending. 2. No way to reload tasks after editing tasks.md for the same change (design gap)The CLI always re-runs the adapter (regenerating Your scenario matrix (scenario 2) accounts for crash-restart, but not intentional re-runs after editing. Consider either:
3.
|
…e-wide purge
Problem: The OpenSpec sentinel (.openspec-populated) was a boolean touch
file that tracked whether tasks were loaded, but not which change or
content. Switching between OpenSpec changes or editing tasks.md left
stale tasks in the queue. Agents would silently execute the wrong plan.
Fix (three layers of defense):
1. Scoped sentinel with content hash -- sentinel stores change path
(line 1) and md5 hash of openspec-tasks.json (line 2). Detects both
change switches and tasks.md edits. Backward compatible with old
single-line format (triggers safe reload on upgrade).
2. Queue-wide jq purge -- new purge_openspec_from_queue() function
cleans all three queue files (pending, completed, in-progress) using
jq. Replaces inline Python, removes python3 dependency for purge
path. Errors surface via log_warn instead of being swallowed.
3. Scoped task IDs -- task IDs now include the change name
(openspec-{change}-N.M) to prevent dedup collisions across changes.
Design decision: when --openspec is not passed, existing OpenSpec state
is left untouched. Purge only triggers when --openspec IS passed with a
different change path or different content hash.
Tests: 31 shell integration tests (test-openspec-sentinel.sh) covering
all state transitions + 28 existing adapter unit tests passing.
Closes: asklokesh#150
Follow-ups filed: asklokesh#154 (BMAD sentinel), asklokesh#155 (MiroFish sentinel)
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
16669f1 to
5029fa6
Compare
Review Response (v2)Thanks for the thorough review @asklokesh! All 4 requested changes addressed, rebased on v6.75.3, squashed to a single commit. 1. Purge now cleans all 3 queue filesNew 2. Content hash detects tasks.md editsSentinel now stores two lines: change path (line 1) and md5 hash of
3. No more silent error swallowingReplaced all inline Python purge blocks with the jq-based shell function. jq stderr goes to a separate 4. Single purge function, no duplication
Design decision: When Non-blocking suggestions addressed
Additional improvements (beyond requested)
Test coverage28 unit tests ( Scenarios covered:
|
Problem
When using OpenSpec with Loki Mode across multiple change proposals in the same repository, the OpenSpec task queue from a previous change persists into subsequent Loki runs -- even when the new run targets a completely different change or uses no
--openspecflag at all.This causes agents to silently work on wrong tasks, wasting tokens and creating incorrect PRs.
Root cause (3 parts)
Boolean sentinel:
.loki/queue/.openspec-populatedis a touch file -- it knows whether OpenSpec tasks were loaded, but not which change they came from.populate_openspec_queue()inrun.sh(line 8744) checks only[[ -f ".loki/queue/.openspec-populated" ]].No cleanup between runs: The CLI (
autonomy/loki) overwritesopenspec-tasks.jsonwhen--openspecis provided, but does NOT clear the sentinel or remove stale tasks frompending.json. When--openspecis absent entirely, nothing cleans up leftover OpenSpec state.Task ID collisions: All OpenSpec changes produce IDs in
openspec-N.Mformat (e.g.,openspec-1.1,openspec-2.3). The deduplication check inpopulate_openspec_queue()uses these IDs, so when switching changes, new tasks with colliding IDs are silently blocked from loading.Reproduction
Impact
--openspec.loki/queue/,.loki/openspec-tasks.json, and.loki/openspec/between runsSolution
Three-pronged fix addressing each root cause:
Fix 1: Scoped sentinel (
autonomy/run.sh)The sentinel file now stores the full change path instead of being an empty marker.
populate_openspec_queue()reads the stored path and compares it against the current$OPENSPEC_CHANGE_PATH:source: "openspec"tasks frompending.json, then repopulateFix 2: Stale state cleanup (
autonomy/loki)Two new cleanup paths in
cmd_start():--openspecIS provided: Clear the sentinel before running the adapter, sorun.shwill repopulate for the current change--openspecis NOT provided: Proactively remove all leftover OpenSpec artifacts (sentinel, tasks JSON, normalized PRD, delta context directory) and purge any stale OpenSpec tasks frompending.jsonFix 3: Change-scoped task IDs (
autonomy/openspec-adapter.py)parse_tasks()now accepts achange_nameparameter and produces IDs in the formatopenspec-{change_name}-N.Minstead ofopenspec-N.M. This prevents cross-change collisions at the deduplication layer.Files changed (5)
autonomy/run.shautonomy/loki--openspecabsentautonomy/openspec-adapter.pyparse_tasks()acceptschange_name, scopes task IDs per-changetests/test_openspec_adapter.pytest_task_ids_hierarchicalfor new ID formatskills/openspec-integration.mdBackward compatibility
The purge strategy filters by
source == "openspec"(a metadata field), not by ID prefix. This means:openspec-N.M) are correctly purgedopenspec-{name}-N.M) are correctly purgedWhen
change_nameis empty (e.g., directparse_tasks()call without context), the oldopenspec-N.Mformat is preserved as a fallback.Test plan
bash -n autonomy/run.sh-- shell syntax validbash -n autonomy/loki-- shell syntax validpython3 -c "import ast; ast.parse(...)"-- Python syntax validpytest tests/test_openspec_adapter.py-- 28/28 tests passtest_task_ids_hierarchicalupdated and passes with new ID formatloki start --openspec ./changes/Athenloki start --openspec ./changes/B-- verify queue purged and repopulatedloki start --openspec ./changes/Athenloki start prd.md(no --openspec) -- verify all OpenSpec artifacts cleaned upRelated
.bmad-populated) and MiroFish (.mirofish-populated) -- may need similar fix in futureBUG-RUN-005deduplication by task ID exacerbated this bug due to ID collisionsGenerated with Claude Code