Skip to content

Move sync marker to EOF for better CLI preview#5

Merged
sebastientaggart merged 2 commits into
devfrom
feature/marker-to-eof
Mar 25, 2026
Merged

Move sync marker to EOF for better CLI preview#5
sebastientaggart merged 2 commits into
devfrom
feature/marker-to-eof

Conversation

@sebastientaggart

Copy link
Copy Markdown
Member

Claude CLI shows the first ~80 chars of each command file as preview text. With the sync marker on line 1, every skill previewed as <!-- generated by CodeCanon/sync.sh | skill: ... — unhelpful. Moving the marker to the last line lets the invocation header show instead.

Changes in sync.sh:

  • sync_skill(): marker appended to end of file instead of prepended
  • read_file_info(): detects markers at both positions (end=current, start=legacy), forces regeneration of legacy-format files

All generated command files regenerated with the new format.

Issue #4

Claude CLI uses the first line of command files as preview text. With the
marker at the top, every skill showed "<!-- generated by CodeCanon/sync.sh..."
instead of useful context. Moving it to EOF lets the invocation header show
in the preview instead.

Also adds backward compatibility: read_file_info detects markers at both
positions (end=current, start=legacy) and forces regeneration of legacy files.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@sebastientaggart

Copy link
Copy Markdown
Member Author

Review Summary

Verdict: REQUEST CHANGES

Findings

  • [CRITICAL] Legacy marker migration is silently blocked. In read_file_info(), when the marker is at the start (legacy position), body_hash is set to None (line 160). The intent (per the comment) is to force regeneration. However, in sync_skill() at lines 250-254, the guard if body_hash != stored_hash evaluates None != "<hash>"True, which triggers the "customized since last sync — skipping" branch. Legacy-format files will never be auto-migrated; users will have to pass --force every time.

    Fix: for legacy markers, compute the actual body_hash from parts[1] (as the old code did) so the customization guard works correctly. The marker relocation will still happen because the freshly computed h won't match body_hash (since full_content now includes a trailing \n before the marker). Alternatively, add an explicit marker_pos == 'start' bypass in sync_skill() that skips the customization guard.

  • [NOTE] The function first_line_has_sync_marker is now called on both the first and last lines. The name is slightly misleading but the docstring ("True if the line was produced by this tool") is accurate, so this is cosmetic.

The initial implementation set body_hash=None for legacy files, which
caused them to be flagged as "customized" and skipped. Instead, compute
the real body_hash and return a needs_migration flag so sync_skill can
force a rewrite even when content hashes match.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@sebastientaggart

Copy link
Copy Markdown
Member Author

Review Summary

Verdict: APPROVE

Findings

  • [NOTE] In read_file_info(), the body extraction for both legacy (lines[1:]) and current (lines[:-1]) positions correctly reconstructs the original full_content string, preserving hash consistency across the migration boundary. The needs_migration flag properly bypasses the "up to date" short-circuit while still honoring the customization guard — edited legacy files will not be silently overwritten. Logic is sound.

No blocking issues found. The marker relocation is clean, the legacy-detection and auto-migration path is correct, and the regenerated command files are consistent with the new format.

@sebastientaggart
sebastientaggart merged commit 1b07589 into dev Mar 25, 2026
2 checks passed
@sebastientaggart sebastientaggart mentioned this pull request Mar 25, 2026
@sebastientaggart
sebastientaggart deleted the feature/marker-to-eof branch April 8, 2026 15:33
sebastientaggart added a commit that referenced this pull request Apr 8, 2026
Move sync marker to EOF for better CLI preview
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant