Skip to content

fix: instinct promote leaves orphaned project-scoped copy #2549

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wouterschoonees wants to merge 2 commits into
base: main
Choose a base branch
from
+50 −0
Open

fix: instinct promote leaves orphaned project-scoped copy #2549

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f4b6f07
fix: remove project-scoped instinct copy after promote
wouterschoonees Jul 23, 2026
85da3db
fix: don't delete source file on failed instinct parse
wouterschoonees Jul 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions skills/continuous-learning-v2/scripts/instinct-cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1314,6 +1314,45 @@ def _show_promotion_candidates(project: dict) -> None:
print(f" Run `instinct-cli.py promote` to promote these to global scope.\n")


def _remove_instinct_from_source(source_file_str: str, instinct_id: str) -> None:
"""Strip one instinct block from its project-scoped source file after promotion.

Without this, the project copy keeps shadowing the new global copy in
load_all_instincts()'s dedup (project wins on ID collision), so `status`
silently under-reports the global count until the stale copy is removed by hand.
Deletes the file if it was the only instinct in it, otherwise rewrites the
remaining blocks in the same frontmatter format.
"""
source_file = Path(source_file_str)
if not source_file.exists():
return

parsed = parse_instinct_file(source_file.read_text(encoding="utf-8"))
if not any(i.get('id') == instinct_id for i in parsed):
# Target not found in a successful parse - either already removed, or the
# file is malformed/foreign content. Either way, don't touch it: a parse
# failure must never be treated as proof the file is safe to delete.
return
Comment on lines +1330 to +1335

@coderabbitai coderabbitai Bot Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Require a complete, lossless parse before modifying the source.

Checking that one parsed block has the target ID is insufficient: parse_instinct_file() filters out blocks without id and tolerates malformed content. A file containing the target plus unreadable/foreign content can therefore pass this guard; remaining may be empty and Line 1340 can delete the whole file, or the rewrite can drop those blocks. Require strict validation of every source block, or preserve unknown/raw blocks during removal.

As per coding guidelines, external data must be treated as untrusted and schema-validated before processing.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@skills/continuous-learning-v2/scripts/instinct-cli.py` around lines 1330 -
1335, Strengthen the removal flow around parse_instinct_file() so source files
are fully and losslessly validated before modification. Do not rely solely on
finding instinct_id, since filtered or malformed blocks may be discarded; either
strictly validate every source block and abort on any unknown/malformed content,
or preserve unknown/raw blocks when rewriting. Ensure remaining being empty
cannot delete the file unless the complete source was safely parsed.

Source: Coding guidelines


remaining = [i for i in parsed if i.get('id') != instinct_id]

if not remaining:
source_file.unlink()
return

Comment thread
greptile-apps[bot] marked this conversation as resolved.
blocks = []
for inst in remaining:
block = "---\n"
for key, value in inst.items():
if key == 'content' or key.startswith('_'):
continue
block += f"{key}: {_yaml_quote(value) if key == 'trigger' else value}\n"
block += "---\n\n"
block += inst.get('content', '') + "\n"
Comment on lines +1346 to +1351

@coderabbitai coderabbitai Bot Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Use YAML-safe serialization for every retained field.

Only trigger passes through _yaml_quote; every other value is inserted with Python string interpolation. A surviving value containing YAML syntax, a newline, or a non-scalar type can change content or type on the next parse, so cleanup does not reliably preserve the remaining instincts. Use the existing schema-aware serializer for every field.

As per coding guidelines, external data must be treated as untrusted and schema-based validation/serialization should be used.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@skills/continuous-learning-v2/scripts/instinct-cli.py` around lines 1340 -
1345, Update the instinct block serialization loop around the `for key, value in
inst.items()` logic to serialize every retained field with the existing
schema-aware YAML serializer, not only `trigger` via `_yaml_quote`. Preserve the
exclusions for `content` and underscore-prefixed keys, while ensuring all
remaining values are safely serialized according to the schema before appending
them to `block`.

Source: Coding guidelines

blocks.append(block)
source_file.write_text("\n".join(blocks), encoding="utf-8")
Comment on lines +1339 to +1353

@coderabbitai coderabbitai Bot Jul 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Promotion is not transactional across global and source files.

The current sequence can lose remaining source data or leave stale project copies after a crash or filesystem error.

  • skills/continuous-learning-v2/scripts/instinct-cli.py#L1333-L1347: stage source replacements and atomically replace them instead of directly unlinking or truncating files.
  • skills/continuous-learning-v2/scripts/instinct-cli.py#L1411-L1415: do not publish the global copy until specific-source cleanup succeeds, or roll back on failure.
  • skills/continuous-learning-v2/scripts/instinct-cli.py#L1489-L1494: apply the same transaction per automatic-promotion candidate so partial multi-project cleanup cannot be reported as success.

As per coding guidelines, errors must be handled explicitly at every level.

📍 Affects 1 file
  • skills/continuous-learning-v2/scripts/instinct-cli.py#L1333-L1347 (this comment)
  • skills/continuous-learning-v2/scripts/instinct-cli.py#L1411-L1415
  • skills/continuous-learning-v2/scripts/instinct-cli.py#L1489-L1494
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@skills/continuous-learning-v2/scripts/instinct-cli.py` around lines 1333 -
1347, The promotion flow is not transactional and can leave source and global
files inconsistent after failures. In
skills/continuous-learning-v2/scripts/instinct-cli.py lines 1333-1347, stage
remaining-source content in a temporary file and atomically replace the source
instead of unlinking or directly rewriting it; in lines 1411-1415, publish the
global copy only after specific-source cleanup succeeds or roll back on failure;
and in lines 1489-1494, apply the same per-candidate transaction for automatic
promotion. Handle and propagate filesystem errors explicitly at each level, and
do not report partial cleanup as success.

Source: Coding guidelines

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Make source cleanup atomic.

unlink() and write_text() mutate the source directly. A crash or short write can delete or truncate the remaining instincts, and callers have already published the global copy by this point. Stage replacements in the same directory and atomically replace them; coordinate deletion with promotion rollback.

As per coding guidelines, errors must be handled explicitly at every level.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@skills/continuous-learning-v2/scripts/instinct-cli.py` around lines 1333 -
1347, The remaining-instincts persistence flow must avoid direct
unlink/write_text mutations and handle failures explicitly. Update the
surrounding function that processes remaining instincts to stage serialized
content in a temporary file within source_file’s directory, atomically replace
the source only after a successful write, and coordinate the empty-result
deletion with promotion rollback. Preserve the existing output format and ensure
cleanup and errors are handled at each operation level.

Source: Coding guidelines



def cmd_promote(args) -> int:
"""Promote project-scoped instincts to global scope."""
project = detect_project()
Expand Down Expand Up @@ -1376,6 +1415,11 @@ def _promote_specific(project: dict, instinct_id: str, force: bool, dry_run: boo
output_content += target.get('content', '') + "\n"

output_file.write_text(output_content, encoding="utf-8")

source_file = target.get('_source_file')
if source_file:
_remove_instinct_from_source(source_file, instinct_id)

print(f"\nPromoted '{instinct_id}' to global scope.")
print(f" Saved to: {output_file}")
return 0
Expand Down Expand Up @@ -1449,6 +1493,12 @@ def _promote_auto(project: dict, force: bool, dry_run: bool) -> int:
output_content += inst.get('content', '') + "\n"

output_file.write_text(output_content, encoding="utf-8")

for _, _, entry_inst in cand['entries']:
entry_source = entry_inst.get('_source_file')
if entry_source:
_remove_instinct_from_source(entry_source, cand['id'])

promoted += 1

print(f"\nPromoted {promoted} instincts to global scope.")
Expand Down