reflect: check for an already-drafted-but-unlanded prior lesson - #2
reflect: check for an already-drafted-but-unlanded prior lesson#2EdbertChan wants to merge 1 commit into
Conversation
…e writing a new one Found via /reflect in the Invoker repo: a reflect commit documenting a "same root cause fixed N times on unmerged disposable branches" pattern sat unmerged on its own PR branch, and the exact pattern it documented recurred and was independently rediscovered by a sibling workflow about an hour later -- the lesson about unlanded fixes was itself an unlanded fix. Adds a check to step 6: before drafting a new skill edit, search the target repo's history for a prior reflect commit on the same file/section and verify it actually landed (is an ancestor of the branch being landed on) before assuming the lesson is in effect. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ccfe8e8. Configure here.
| - Substantive edit (a new section, a new principle, more than ~10 lines): write it out in full, matching the target skill's existing structure and tone, and show the diff before it's considered done. | ||
| - Backlog item: describe the concrete script/check/test to write, but don't write it as part of `reflect` itself — that's separate implementation work once the user confirms it's wanted. | ||
| - Route-to-`automate-me` item: don't draft it here. Either invoke `automate-me` directly if the user wants it done now, or leave it as a named follow-up in the summary below. | ||
| - Before drafting a new edit, check whether the same lesson was already captured and drafted by an earlier `reflect` pass but never landed — a skill-edit commit is subject to the same disposable-branch/unmerged-PR risk as any other fix, and a lesson that only exists on an unmerged branch is not actually in effect. Search the target repo's history for a prior reflect commit touching the same file/section (e.g. `git log --all --grep="reflect" -i -- <file>`) and check whether it's an ancestor of the branch you're landing on (e.g. `git merge-base --is-ancestor <candidate> HEAD`). If a matching unlanded draft exists, prefer adapting/extending it — and note its commit/branch in the summary so a reviewer can consider closing the duplicate — over writing a third divergent copy of the same lesson. |
There was a problem hiding this comment.
Landed check uses wrong ref
Medium Severity
The new unlanded-draft check treats git merge-base --is-ancestor <candidate> HEAD as proof a prior reflect lesson has landed and is in effect. HEAD is the working-branch tip, not the trunk sibling workflows start from. On a stacked PR, an earlier unmerged reflect commit is already an ancestor of HEAD, so the check can miss the exact disposable-branch failure mode this bullet exists to catch.
Reviewed by Cursor Bugbot for commit ccfe8e8. Configure here.
All four predated the a7fc343 refactor (lens table moved to references/lenses.md) and had gone stale; ported their content onto the current structure and closing them in favor of this commit: - PR #1: path/file_path alias normalization in claude-mode dedup, edit-streak, and error-by-file detectors (+ both tests). The is_error half was already structural on main. 77/77 tests pass. - PR #2 (folded into #3 by its own description): step-6 unlanded-prior-draft check before writing a new lesson. - PR #3: step-3 sibling-transcript check, History-lens template-identical caution (now in references/lenses.md), step-6 commit-as-you-go. - PR #4: when-to-invoke bullet for automated reflect-ci runs + sibling branch grep folded into the step-3 sibling check.
|
Landed on main in 9b9f97d — all four open PRs were written against the pre-a7fc343 SKILL.md layout and had gone stale (3 of 4 CONFLICTING). This PR's content was ported onto the current structure: see commit message for the per-PR mapping. Tests: 77/77 pass including this PR's two path-alias tests. |
Summary
/reflecton a CI-repair task in the Invoker repo: a priorreflectcommit had already documented a "same root cause independently fixed N times on unmerged disposable branches" pattern, but that commit itself sat unmerged on its own PR branch — and the exact pattern it documented recurred and was independently rediscovered by a sibling workflow about an hour later. The lesson about unlanded fixes was itself an unlanded fix.SKILL.mdstep 6: before drafting a new skill edit, search the target repo's history for a priorreflectcommit touching the same file/section and verify it actually landed (git merge-base --is-ancestor) before assuming the lesson is in effect, to avoid writing a third divergent copy of the same lesson.Test plan
🤖 Generated with Claude Code
Note
Low Risk
Procedural documentation only in
skills/reflect/SKILL.md; no runtime code, auth, or data paths.Overview
Step 6 of the
reflectskill now requires checking for an earlierreflectcommit that already drafted the same lesson on the target file/section but never merged, so agents don’t add another divergent copy of the same guidance.The new bullet explains that unmerged skill edits aren’t in effect, and points to concrete git checks (
git log --all --grep="reflect" -i -- <file>,git merge-base --is-ancestor <candidate> HEAD). When a matching draft exists, extend or adapt it and call out commit/branch in the summary for duplicate-PR cleanup.Reviewed by Cursor Bugbot for commit ccfe8e8. Bugbot is set up for automated code reviews on this repo. Configure here.