Skip to content

feat(skills): stash-encryption skills up to date with the cs_migratio…#438

Open
calvinbrewer wants to merge 1 commit intomainfrom
migration-skills
Open

feat(skills): stash-encryption skills up to date with the cs_migratio…#438
calvinbrewer wants to merge 1 commit intomainfrom
migration-skills

Conversation

@calvinbrewer
Copy link
Copy Markdown
Contributor

@calvinbrewer calvinbrewer commented May 7, 2026

…ns approach

Summary by CodeRabbit

  • Documentation
    • Enhanced Column Migration Lifecycle documentation with structured phase/event mappings, per-phase invariants, state storage details, and example SQL queries for inspecting migration status and history.

@calvinbrewer calvinbrewer requested a review from a team as a code owner May 7, 2026 17:43
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 7, 2026

⚠️ No Changeset found

Latest commit: 60c9c41

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 7, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR updates the CipherStash encryption skill documentation to provide structured, detailed guidance on the column migration lifecycle state machine. The changes reformat the migration phases into a defined table, clarify the state storage model as an append-only event log, and add complete schema definition with inspection queries.

Changes

Column Migration Lifecycle Documentation

Layer / File(s) Summary
Lifecycle Phase/Event Definition
skills/stash-encryption/SKILL.md
Migration lifecycle is restructured from narrative into a table defining six phases (pending, in_progress, backfilling, backfilled, live), valid events (start, begin, backfill_started, backfill_complete, activate, error), per-phase invariants, and responsibility boundaries between app code, persistence layer, CLI, and backfill engine.
State Storage Model
skills/stash-encryption/SKILL.md
cipherstash.cs_migrations is clarified as an append-only runtime event log where the effective phase per (table_name, column_name) is determined by the row with the greatest id. Three sources of truth are documented: application schema, event log, and backfill engine state.
Schema Definition and Inspection
skills/stash-encryption/SKILL.md
Complete DDL for cipherstash.cs_migrations event log table is provided with column semantics, installation and bundling context, enumeration of valid event and phase values, SQL queries for inspecting current phase and full migration history, and reference to programmatic access via @cipherstash/migrate helper functions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 ✨ A migration tale now clear,
With phases mapped and state sincere,
From pending bloom to live cascade,
The event log path a raft has made!
New schemas shine with queries true,
The lifecycle unfolds—hooray for you! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly references updating stash-encryption skills to match the cs_migrations approach, which aligns with the documented changes to SKILL.md that expand the Column Migration Lifecycle documentation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch migration-skills

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
skills/stash-encryption/SKILL.md (1)

596-605: ⚡ Quick win

Excellent lifecycle documentation with one clarity opportunity.

The phase table is comprehensive and well-structured. The separation between "happy path" transitions (in the table) and error handling (line 605) makes sense, but could be more explicit.

📝 Optional clarity improvement

Consider adding a brief note before or after the table to make the distinction explicit:

 | `dropped` | `dropped` | `<col>_plaintext` is removed via a regular schema migration. The application stops writing to it (dual-writing logic is removed). | App-code change to remove dual-writes + a schema migration. |
 
+> **Note:** The table above shows successful phase transitions. The `error` event (described below) can occur at any phase without changing the effective phase.
+
 A failure at any phase is recorded as an `error` event without changing the effective phase, so a retry resumes from where it failed.
🤖 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/stash-encryption/SKILL.md` around lines 596 - 605, Add a short
clarifying sentence immediately adjacent to the phase table stating that
failures are recorded as an `error` event (named `error`) and do not advance the
lifecycle phase so retries resume from the current phase; reference the
lifecycle phase names shown (e.g., `schema-added`, `dual-writing`,
`backfilling`, `backfilled`, `cut-over`, `dropped`) and the `error` event to
make the behavior explicit to readers.
🤖 Prompt for all review comments with 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.

Nitpick comments:
In `@skills/stash-encryption/SKILL.md`:
- Around line 596-605: Add a short clarifying sentence immediately adjacent to
the phase table stating that failures are recorded as an `error` event (named
`error`) and do not advance the lifecycle phase so retries resume from the
current phase; reference the lifecycle phase names shown (e.g., `schema-added`,
`dual-writing`, `backfilling`, `backfilled`, `cut-over`, `dropped`) and the
`error` event to make the behavior explicit to readers.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 77b12467-d8f4-4475-ab84-8331b0c88b97

📥 Commits

Reviewing files that changed from the base of the PR and between e2136cb and 60c9c41.

📒 Files selected for processing (1)
  • skills/stash-encryption/SKILL.md

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