Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
55 changes: 37 additions & 18 deletions .agents/skills/ci-prep/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ name: ci-prep
description: Prepares the current branch for CI by running the exact same steps locally and fixing issues. If CI is already failing, fetches the GH Actions logs first to diagnose. Use before pushing, when CI is red, or when the user says "fix ci".
argument-hint: "[--failing] [optional job name to focus on]"
---
<!-- agent-pmo:74cf183 -->

# CI Prep

Expand All @@ -15,7 +16,7 @@ Prepare the current state for CI. If CI is already failing, fetch and analyze th

If `--failing` is NOT passed, skip directly to **Step 2**.

## Step 1 �� Fetch failed CI logs (only when `--failing`)
## Step 1 Fetch failed CI logs (only when `--failing`)

You MUST do this before any other work.

Expand All @@ -41,11 +42,27 @@ Read **every line** of `--log-failed` output. For each failure note the exact fi

## Step 2 — Analyze the CI workflow

1. Read `.github/workflows/ci.yml` completely. Parse every job and every step.
2. Extract the ordered list of commands the CI actually runs.
3. Note environment variables, matrix strategies, conditional steps, and service containers.
1. Find the CI workflow file. Look in `.github/workflows/` for `ci.yml`, `build.yml`, `test.yml`, `checks.yml`, `main.yml`, `pull_request.yml`, or any workflow triggered on `pull_request` or `push`.
2. Read the workflow file completely. Parse every job and every step.
3. Extract the ordered list of commands the CI actually runs. In a spec-compliant repo this is `make lint → make test → make build` (REPO-STANDARDS-SPEC [MAKE-TARGETS]), but the actual CI may use `npm`, `cargo`, `dotnet`, raw shell commands, or anything else. Extract what is *actually there*.
4. Note any environment variables, matrix strategies, or conditional steps that affect execution.

**Do NOT assume the steps are `make lint`, `make test`, `make build`.** Extract what the CI *actually does*.
**Do NOT assume the steps are `make lint`, `make test`, `make build`.** The actual CI may run different commands, in a different order. Extract what the CI *actually does*. If you find extra targets beyond the 7 in [MAKE-TARGETS] (e.g. `make fmt-check`, `make coverage-check`), flag them in your final report — they should be consolidated by the agent-pmo skill.

### Release workflow blocker scan

If `.github/workflows/release.yml` exists, scan it before broad local CI. These are critical blockers
and must be fixed before release work is considered CI-ready:

- Tag-triggered jobs checking out `ref: main` instead of the tagged SHA.
- Any `git commit`, `git push`, branch mutation, or tag mutation during release.
- Version bump commits after the tag already exists.
- Ad hoc `sed` version stamping of structured files instead of a first-class stamper/build input.
- Missing tests that pass a test version into the same stamper used by release.
- Native VSIX releases without Node `22.x`, `npx vsce package --target <vsceTarget>`, one VSIX per
target, target-suffixed filenames, and package-content verification.
- VS Code native-binary activation that reads or mutates PATH, uses package-manager/global installs
as normal startup sources, or copies bundled VSIX binaries after install.

## Step 3 — Run each CI step locally, in order

Expand All @@ -58,45 +75,47 @@ Work through failures in this priority order:

For each command extracted from the CI workflow:

1. Run the command exactly as CI would run it.
1. Run the command exactly as CI would run it (adjusting only for local environment differences like not needing `actions/checkout`).
2. If the step fails, **stop and fix the issues** before continuing to the next step.
3. After fixing, re-run the same step to confirm it passes.
4. Move to the next step only after the current one succeeds.

### Hard constraints

- **NEVER modify test files** — fix the source code, not the tests
- **NEVER add suppressions** (`#pragma warning disable`, `#[allow(...)]`, `// eslint-disable`)
- **NEVER add suppressions** (`#[allow(...)]`, `// eslint-disable`, `#pragma warning disable`)
- **NEVER use `any` in TypeScript** to silence type errors
- **NEVER delete or ignore failing tests**
- **NEVER remove assertions**

If stuck on the same failure after 5 attempts, ask the user for help.

## Step 4 — Loop
## Step 4 — Report

- Go back to the first step and repeat until all steps pass locally. If `--failing`, you should see the exact same errors in your terminal that CI shows in the logs. Fix those errors until they are resolved.
- List every step that was run and its result (pass/fail/fixed).
- If any step could not be fixed, report what failed and why.
- Confirm whether the branch is ready to push.

## Step 5 — Commit/Push (only when `--failing`)
## Step 5 — Remote CI follow-up (only when `--failing`)

Once all CI steps pass locally:

1. Commit, but DO NOT MARK THE COMMIT WITH YOU AS AN AUTHOR!!! Only the user authors the commit!
2. Push
3. Monitor until completion or failure
4. Upon failure, go back to Step 1
1. Report the local fixes and exact commands that now pass.
2. Do not commit or push. The user owns source-control writes.
3. If the user pushes, monitor the new run until completion or failure.
4. Upon failure, go back to Step 1.

## Rules

- *You are not allowed to commi/push until all tests pass*. Do not waste GitHub action minutes! The local CI must prove that everything is working.
- **Always read the CI workflow first.** Never assume what commands CI runs.
- Do not push if any step fails (unless `--failing` and all steps now pass)
- Do not commit or push from this skill.
- Fix issues found in each step before moving to the next
- Never skip steps or suppress errors
- If the CI workflow has multiple jobs, run all of them (respecting dependency order)
- Skip steps that are CI-infrastructure-only (checkout, setup actions, cache steps, artifact uploads) — focus on the actual build/test/lint commands
- Skip steps that are CI-infrastructure-only (checkout, setup-node/python/rust actions, cache steps, artifact uploads) — focus on the actual build/test/lint commands

## Success criteria

- Every command that CI runs has been executed locally and passed
- All fixes are applied to the working tree
- The CI passes successfully (if you are correcting an existing failure)
- The CI passes successfully (if you are correcting and existing failure)
81 changes: 45 additions & 36 deletions .agents/skills/code-dedup/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
name: code-dedup
description: Searches for duplicate code, duplicate tests, and dead code, then safely merges or removes them. Use when the user says "deduplicate", "find duplicates", "remove dead code", "DRY up", or "code dedup". Requires test coverage — refuses to touch untested code.
---
<!-- agent-pmo:74cf183 -->

# Code Dedup

Expand All @@ -12,8 +13,11 @@ Carefully search for duplicate code, duplicate tests, and dead code across the r
Before touching ANY code, verify these conditions. If any fail, stop and report why.

1. Run `make test` — all tests must pass. If tests fail, stop. Do not dedup a broken codebase.
2. Run `make coverage-check` — coverage must meet the repo's threshold. If it doesn't, stop.
3. This repo uses **C#, F#, Rust, and TypeScript** — all statically typed. Proceed.
2. Run `make test` — tests are fail-fast AND enforce the coverage threshold from `coverage-thresholds.json`. If anything fails, stop and fix it before deduping.
3. Verify the project uses **static typing**. Check for:
- Rust, C#, F#, Dart, Go: typed by default — proceed
- TypeScript: `tsconfig.json` must have `"strict": true` — proceed (Lql/LqlExtension already does)
- **Untyped JavaScript: STOP. Refuse to dedup.** Print: "This codebase has no static type checking. Deduplication without types is reckless — too high a risk of silent breakage. Add type checking first."

## Steps

Expand All @@ -33,74 +37,79 @@ Dedup Progress:

Before deciding what to touch, understand what is tested.

1. Run `make test` and `make coverage-check` to confirm green baseline
1. Run `make test` to confirm green baseline. `make test` is fail-fast AND enforces the coverage threshold from `coverage-thresholds.json` (REPO-STANDARDS-SPEC [TEST-RULES], [COVERAGE-THRESHOLDS-JSON]). It exits non-zero on any test failure OR coverage shortfall.
2. Note the current coverage percentage — this is the floor. It must not drop.
3. Identify which files/modules have coverage and which do not. Only files WITH coverage are candidates for dedup.

### Step 2 — Scan for dead code

Search for code that is never called, never imported, never referenced.

1. Look for unused exports, unused functions, unused records, unused variables
2. Use language-appropriate tools:
- **C#/F#:** Analyzer warnings for unused members (build with `-warnaserror` catches these)
- **Rust:** The compiler already warns on dead code — check `make lint` output
- **TypeScript:** Check for unexported functions with zero references in `Lql/LqlExtension/`
3. For each candidate: **grep the entire codebase** for references (including tests, scripts, configs). Only mark as dead if truly zero references.
1. Look for unused exports, unused functions, unused classes, unused variables.
2. Use language-appropriate tools where available:
- C#/F#: analyzer warnings (CA1801 unused parameters, IDE0051 unused private members) via `make lint`
- Rust: `cargo clippy` already warns on dead code — check `make lint` output
- TypeScript: `noUnusedLocals`/`noUnusedParameters` in `tsconfig.json`; look for unexported functions with zero references
3. For each candidate: **grep the entire repo** (including tests, scripts, samples). Skip generated code under `Lql/lql-lsp-rust/crates/lql-parser/src/generated/`. Only mark as dead if truly zero references.
4. List all dead code found with file paths and line numbers. Do NOT delete yet.

### Step 3 — Scan for duplicate code

Search for code blocks that do the same thing in multiple places.

1. Look for functions/methods with identical or near-identical logic
2. Look for copy-pasted blocks (same structure, maybe different variable names)
3. Look for multiple implementations of the same algorithm or pattern
4. Check across module boundaries — duplicates often hide in different projects (DataProvider, Lql, Sync, Gatekeeper, Samples)
5. For each duplicate pair: note both locations, what they do, and how they differ (if at all)
1. Look for functions/methods with identical or near-identical logic.
2. Look for copy-pasted blocks (same structure, maybe different variable names).
3. Look for multiple implementations of the same algorithm — particularly likely across `DataProvider/`, `Migration/`, `Sync/`, `Reporting/` C# projects, and across `Lql/Nimblesite.Lql.Core` SQL transpiler dialects (`Postgres`, `SqlServer`, `SQLite`).
4. Check across module boundaries — duplicates often hide in different `.csproj` projects or Rust crates (`lql-parser`, `lql-analyzer`, `lql-lsp`).
5. For each duplicate pair: note both locations, what they do, and how they differ (if at all).
6. List all duplicates found. Do NOT merge yet.

### Step 4 — Scan for duplicate tests

Search for tests that verify the same behavior.

1. Look for test functions with identical assertions against the same code paths
2. Look for test fixtures/helpers that are duplicated across test files
3. Look for integration tests that fully cover what a unit test also covers (keep the integration test, mark the unit test as redundant per CLAUDE.md rules)
1. Look for test functions with identical assertions against the same code paths.
2. Look for test fixtures/helpers that are duplicated across test projects (`Tests.Shared/` is meant to hold these).
3. Look for integration tests that fully cover what a unit test also covers (keep the integration test, mark the unit test as redundant per CLAUDE.md "Prefer E2E/integration tests").
4. List all duplicate tests found. Do NOT delete yet.

### Step 5 — Apply changes (one at a time)

For each change, follow this cycle: **change -> test -> verify coverage -> continue or revert**.
For each change, follow this cycle: **change test verify coverage continue or revert**.

#### 5a. Remove dead code
- Delete dead code identified in Step 2
- After each deletion: run `make test` and `make coverage-check`
- If tests fail or coverage drops: **revert immediately** and investigate
- Delete dead code identified in Step 2.
- After each deletion: run `make test` (fail-fast + coverage + threshold all in one).
- If `make test` exits non-zero (test failure OR coverage drop): **revert immediately** and investigate.
- Dead code removal should never break tests or drop coverage.

#### 5b. Merge duplicate code
- For each duplicate pair: extract the shared logic into a single function/module
- Update all call sites to use the shared version
- After each merge: run `make test` and `make coverage-check`
- For each duplicate pair: extract the shared logic into a single function/module. Prefer placing shared code in `Tests.Shared/` (for test helpers) or the closest `.Core` project (e.g. `Sync/Nimblesite.Sync.Core`, `Lql/Nimblesite.Lql.Core`).
- Update all call sites to use the shared version.
- After each merge: run `make test`.
- If tests fail: **revert immediately**. The duplicates may have subtle differences you missed.
- If coverage drops: the shared code must have equivalent test coverage. Add tests if needed before proceeding.

#### 5c. Remove duplicate tests
- Delete the redundant test (keep the more thorough one)
- After each deletion: run `make coverage-check`
- If coverage drops: **revert immediately**. The "duplicate" test was covering something the other wasn't.
- Delete the redundant test (keep the more thorough one).
- After each deletion: run `make test`.
- If coverage drops below threshold, `make test` exits non-zero — **revert immediately**. The "duplicate" test was covering something the other wasn't.

### Step 6 — Final verification

1. Run `make test` — all tests must still pass
2. Run `make coverage-check` — coverage must be >= the baseline from Step 1
3. Run `make lint` and `make fmt-check` — code must be clean
4. Report: what was removed, what was merged, final coverage vs baseline
1. Run `make lint` — all linters and the format check must pass.
2. Run `make test` — tests must pass AND coverage must remain ≥ the baseline from Step 1.
3. Report: what was removed, what was merged, final coverage vs baseline.

(Only the 7 standard targets exist — `make lint` and `make test` cover formatting and coverage checks respectively.)

## Rules

- **No test coverage = do not touch.** If a file has no tests covering it, leave it alone entirely.
- **Coverage must not drop.** The coverage floor from Step 1 is sacred.
- **No test coverage = do not touch.** If a file has no tests covering it, leave it alone entirely. You cannot safely dedup what you cannot verify.
- **Coverage must not drop.** If removing or merging code causes coverage to decrease, revert and investigate. The coverage floor from Step 1 is sacred.
- **Untyped code = refuse to dedup.** This repo has no untyped surfaces today; if any appear, refuse.
- **One change at a time.** Make one dedup change, run tests, verify coverage. Never batch multiple dedup changes before testing.
- **When in doubt, leave it.** If two code blocks look similar but you're not 100% sure they're functionally identical, leave both.
- **Preserve public API surface.** Do not change function signatures, record names, or module exports that external code depends on. Internal refactoring only.
- **Three similar lines is fine.** Only dedup when the shared logic is substantial (>10 lines) or when there are 3+ copies.
- **When in doubt, leave it.** If two code blocks look similar but you're not 100% sure they're functionally identical, leave both. False dedup is worse than duplication.
- **Preserve public API surface.** Do not change the public surface of `Nimblesite.DataProvider.Core`, `Nimblesite.Lql.Core`, `Nimblesite.Sync.Core`, or any package that is published to NuGet. Internal refactoring only.
- **Three similar lines is fine.** Do not create abstractions for trivial duplication. Only dedup when the shared logic is substantial (>10 lines) or when there are 3+ copies.
- **Never touch ANTLR-generated code.** `Lql/lql-lsp-rust/crates/lql-parser/src/generated/` is regenerated from the `.g4` grammar; any dedup there will be erased on the next regen.
67 changes: 67 additions & 0 deletions .agents/skills/fix-bug/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
---
name: fix-bug
description: Fix a bug using test-driven development. Use when the user reports a bug, describes unexpected behavior, wants to fix a defect, or says something is broken. Enforces a strict test-first workflow where a failing test must be written and verified before any fix is attempted.
argument-hint: "[bug description]"
allowed-tools: Read, Grep, Glob, Edit, Write, Bash
---
<!-- agent-pmo:74cf183 -->

# Bug Fix Skill — Test-First Workflow

You MUST follow this exact workflow. Do NOT skip steps. Do NOT fix the bug before writing a failing test.

## Step 1: Understand the Bug

- Read the bug description: $ARGUMENTS
- Investigate the codebase to understand the relevant code
- Identify the root cause (or narrow down candidates)
- Summarize your understanding of the bug to the user before proceeding

## Step 2: Write a Failing Test

- Write a test that **directly exercises the buggy behavior**
- The test must assert the **correct/expected** behavior — so it FAILS against the current broken code
- The test name should clearly describe the bug (e.g., `test_orange_color_not_applied_to_head`)
- Use the project's existing test framework and conventions

## Step 3: Run the Test — Confirm It FAILS

- Run ONLY the new test (not the full suite)
- **Verify the test FAILS** and that it fails **because of the bug**, not for some other reason (typo, import error, wrong selector, etc.)
- If the test passes: your test does not capture the bug. Go back to Step 2
- If the test fails for the wrong reason: fix the test, not the code. Go back to Step 2
- **Repeat until the test fails specifically because of the bug**

## Step 4: Show Failure to User

- Show the user the test code and the failure output
- Explicitly ask: "This test fails because of the bug. Can you confirm this captures the issue before I fix it?"
- **STOP and WAIT for user acknowledgment before proceeding**
- Do NOT continue to Step 5 until the user confirms

## Step 5: Fix the Bug

- Make the **minimum change** needed to fix the bug
- Do not refactor, clean up, or "improve" surrounding code
- Do not change the test

## Step 6: Run the Test — Confirm It PASSES

- Run the new test again
- **Verify it PASSES**
- If it fails: go back to Step 5 and adjust the fix
- **Repeat until the test passes**

## Step 7: Run the Full Test Suite

- Run ALL tests to make sure nothing else broke
- If other tests fail: fix the regression without breaking the new test
- Report the final result to the user

## Rules

- NEVER fix the bug before the failing test is written and confirmed
- NEVER skip asking the user to acknowledge the test failure
- NEVER modify the test to make it pass — modify the source code
- If you cannot write a test for the bug, explain why and ask the user how to proceed
- Keep the fix minimal — one bug, one fix, one test
Loading
Loading