Restack #593: wire skills to mark_feature_used after the learning stack - #742
Open
davekilleen wants to merge 7 commits into
Open
davekilleen wants to merge 7 commits into
davekilleen wants to merge 7 commits into
Conversation
What this pull request touches
Gates that will judge this change
Based on 74 changed files. |
davekilleen
marked this pull request as ready for review
September 22, 2026 12:26
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
9 tasks
usage_log.md drives /dex-level-up: it records which Dex features a vault has used so the skill can recommend the ones it has not. mark_feature_used() is the documented way to record that, and it could not do it. Three separate reasons, each of which alone would be enough: 1. It was never exposed as a tool. The only documented caller is a skill, and skills reach Python through MCP. The function has lived in analytics_helper.py unreachable from the place the checklist tells authors to call it from. 2. Its pattern could not match most of the file. The label matcher used [^(\\n]* around the feature name, which stops at the first bracket, so any entry written as "Daily planning (\`/daily-plan\`)" was unmatchable. On a real vault that is 37 of 61 remaining features. 3. It returned None whether it worked or not. A caller could not tell a successful tick from a no-op, so a miss looked exactly like a hit. That is why the drift was invisible: the log on the vault this was found on had not changed in twelve days while the features were in daily use. This replaces the matcher with an ordered one (slash command, then exact label, then label with the command stripped), returns a status instead of None, refuses to guess when several entries match, and writes through a temp file so an interrupted write cannot truncate the consent records that share this file. Adoption tracking is deliberately NOT gated on analytics consent, and there is a test for that: it writes to the vault and sends nothing, and the recommendations it feeds are useful whether or not a user shares anything. The parsed feature map in load_usage_log() had no consumers, so the read side was dead too. A test now covers the loop end to end. Co-authored-by: davekilleen <davekilleen@users.noreply.github.com>
Review of #590: mark_feature_used wrote System/usage_log.md directly, which breaks Dex's vault-mutation contract and is unsafe three ways. All three reproduce against the old approach and are now covered by tests: - a vault whose System directory is a symlink had the write land outside the vault entirely; - a 0600 log became 0644, because write_text on a fresh temporary takes the umask and the rename carries it; - the fixed .tmp sibling is the same path for both writers, so a feature tick and a consent update could each read, modify and write the whole file and silently lose the other's change. Adds rewrite_usage_log() to core/lifecycle/service.py, modelled on the existing analytics-receipt seam: whole-route symlink validation before the file is read, a refusal on a symlinked target, bounded read, expected-SHA guard with stale retry, and the existing mode re-stated in the plan so a write can never loosen it. Scoped by the contract rather than by the caller. The new usage-log operation is authorised for exactly one path, so it cannot write anything else even if a future caller asks it to, and the bounded-read limit is registered at both engine sites. update_consent now uses the same door, per the review: with two direct writers the concurrency guarantee would not have been real. The MCP helper keeps its matching and its five statuses. It decides the outcome from a plain read so a no-op never opens a transaction, and its transform re-matches against the text the transaction actually read, because a retry may run it again after another writer has ticked the same box. 251 tests pass across usage tracking, the portable contract, the transaction core, analytics wiring, instruction honesty and the lifecycle contract.
Making mark_feature_used reachable adds a fifth tool to dex-analytics, which the capability discovery added in v1.97.0 pins exactly: the boundary test asserted 131 tools and 4 on this server, and the committed enriched example listed the old four by name. Updated both, and renamed the test so its name still matches what it asserts. The example is regenerated output, not a hand edit; the only difference is the new tool appearing in the count, the summary sentence and the example list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: davekilleen <davekilleen@users.noreply.github.com>
Follow-up to the parent commit, which made mark_feature_used work and made it reachable. Nothing called it, so the log still depended on the assistant remembering to edit markdown mid-task, which is the failure that started this. Replaces "Update System/usage_log.md to mark X as used" with a call to the tool in the 22 skills whose slash command resolves to exactly one line in the shipped log. Four skills are deliberately left on prose: commitments, initiative-kickoff, meeting-closeout and relationship-radar have no line in the log at all, so wiring them would produce a not_found and record nothing. Adding entries for them is a content change to the log itself and belongs in its own PR. /journal is named by its label rather than its command, because /journal appears against both Journaling and Journaling setup and the tool refuses to guess. The instruction says why, so it does not get "tidied" back. The test parameterises over every wired skill and resolves its feature name against the shipped log with the real matcher, so an instruction can never drift into naming something that does not exist. It caught a nested-backtick bug in the journal instruction while this was being written, which is precisely the class of silent no-op it exists to prevent. Co-authored-by: davekilleen <davekilleen@users.noreply.github.com>
The wiring script matched on the phrase 'to mark X as used' and reported 'wired 22 skills' as though that were complete. Three skills phrase it differently and were excluded before the checkbox test ever ran. Two of them have a box in the shipped log and should have been wired: - week-plan, which says only 'Update System/usage_log.md.' - dex-level-up, whose Step 6 is the more consequential of the two: it marks OTHER features as the user tries them, and it is the skill that reads this log to decide what to recommend next. It was hand-editing the file it depends on. dex-doctor stays on prose: it has no line in the shipped log, like the four already recorded as unwireable. dex-level-up now uses the tool for both jobs, records its own use, and is told to check the returned status, because not_found and ambiguous pass silently and this skill is the one that suffers. Also closes the hole in the test that allowed this. Its detector reads one canonical phrasing, so a skill referencing the tool any other way was skipped without failing. dex-level-up did exactly that on the first attempt at this fix. The suite now fails on any skill that mentions mark_feature_used in a shape it cannot parse, with parameterised callers listed explicitly rather than matched by accident. 42 tests pass. Lens registry and architecture inventory regenerated. Co-authored-by: davekilleen <davekilleen@users.noreply.github.com>
The journal skill runs setup and entry modes, and the usage log carries a separate line for each. One unconditional call to mark "Journaling" meant "Journaling setup" could never be ticked by the only command that performs it, so /dex-level-up would keep recommending setup to people who had already done it. /journal on and /journal off now record "Journaling setup"; the entry modes record "Journaling". Both still name the label rather than /journal, which matches both lines and returns ambiguous. The test grew with it. Expected wiring now holds every feature a skill records, not one, so a dropped mode fails exact equality. Six explicit scenarios tie each documented mode to the line it must tick, and a further case asserts /journal is still ambiguous, so if that ever stops being true the skill's stated reason for using labels cannot quietly go stale. Reintroducing the single unconditional call fails seven of these by name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Thirteen of the skills wired here are Lens capabilities, and the registry pins each one's exact bytes. Editing them left the pins stale, so the release generation refuses. Repinned from the files themselves, then verified every active-skill entry in the registry against its file rather than trusting the edit: zero inconsistent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: davekilleen <davekilleen@users.noreply.github.com>
cursor
Bot
force-pushed
the
cursor/wire-skills-usage-tracking-41dd
branch
from
September 22, 2026 21:09
1c5341e to
87900bd
Compare
cursor
Bot
changed the title
This branch has not been deployed
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
2 participants
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.
Linked Issue
#566/#568/#569/#744/#745) and#741. No Linear issue.What Changed
3a2e6100ccd04be78ece2f703751fdbb3447251f.core/lens-catalog/registry.jsonanddocs/architecture/INVENTORY.md); pins were recomputed from the restacked skill files and inventory was regenerated.mark_feature_usedinstead of hand-editing the log; journal records the mode that actually ran; unwireable skills stay on prose.Old head:
1c5341e7b8d5cc73d31126dd9ad551b5e06bda13New head:
87900bdfec0751fac4c5198c31cb55ab05255d76Stacked on #743:
3a2e6100ccd04be78ece2f703751fdbb3447251fSame-origin branch updated in place:
cursor/wire-skills-usage-tracking-41dd.The original fork PR (#593) is untouched.
Test Plan
core/tests/test_usage_tracking.pyskill-wiring cases (from the original Nothing calls the adoption tool, so skills still hand-edit the log #593 commits)pytest core/tests/test_usage_tracking.pyon this stacked head → 59 passedRalph Wiggum Loop
Quality Gates
Risk & Rollback
Docs Impact
core/lens-catalog/registry.jsonpins anddocs/architecture/INVENTORY.mdonly where hashes/headers drifted