Skip to content

fix(skills): add sqlite missing-mirror guard guidance#2819

Merged
mergify[bot] merged 3 commits into
mainfrom
fix/issue-2393-sqlite-mirror-guard
Jun 7, 2026
Merged

fix(skills): add sqlite missing-mirror guard guidance#2819
mergify[bot] merged 3 commits into
mainfrom
fix/issue-2393-sqlite-mirror-guard

Conversation

@tmchow

@tmchow tmchow commented Jun 7, 2026

Copy link
Copy Markdown
Owner

Summary

SQLite-backed novel-command guidance now gives agents a copyable missing-mirror guard for first-run live executions, scoped to commands that read the local store. The store-query skeleton now checks dbPath before opening SQLite and returns [] for --json / --agent while giving humans a sync hint.

Closes #2393

Tests

  • go test ./internal/cli -run 'TestPrintingPressSkill|TestInternalSkillMinimumBinaryVersionsTrackMajor' -count=1
  • go test ./internal/pipeline -run 'TestLiveCheck_(PassOnQueryOnlyJSONOutput|FailOnIrrelevantOutput)$' -count=1
  • go test ./... failed in internal/pipeline: TestLiveCheck_PassOnQueryOnlyJSONOutput and TestLiveCheck_FailOnIrrelevantOutput timed out after 5s during the full-suite run; both passed on targeted rerun.

@mergify

mergify Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟢 require-ready-label-and-ci

Wonderful, this rule succeeded.
  • #changes-requested-reviews-by = 0
  • #review-threads-unresolved = 0
  • check-success = build-and-test
  • check-success = generated-test
  • check-success = go-lint
  • check-success = golden
  • check-success = pr-title
  • check-success = test
  • any of:
    • label = ready-to-merge
    • all of:
      • head = release-please--branches--main
      • title ~= ^chore\(main\): release
  • any of:
    • -files ~= ^(\.github/workflows/|\.github/scripts/|scripts/|\.github/CODEOWNERS$)
    • author = tmchow
    • approved-reviews-by = mvanhorn
    • approved-reviews-by = tmchow
    • author = mvanhorn
  • any of:
    • check-success = Greptile Review
    • label = queued
    • check-neutral = Greptile Review
    • check-skipped = Greptile Review
    • head ~= ^mergify/merge-queue/
    • all of:
      • head = release-please--branches--main
      • title ~= ^chore\(main\): release

@greptile-apps

greptile-apps Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a missing-mirror guard for SQLite-backed novel commands, ensuring that when the local store hasn't been populated yet (i.e., the user hasn't run sync), the command returns a clear human-readable hint to stderr and an empty JSON array for machine/agent callers rather than a SQLite open failure.

  • skills/printing-press/SKILL.md: Adds standalone guidance prose and copyable guard snippet (placed after dryRunOK, required-input, and dbPath resolution gates), and injects the same guard into the full command skeleton immediately before store.OpenWithContext.
  • internal/cli/printing_press_skill_test.go: Adds TestPrintingPressSkillSQLiteNovelCommandsGuardMissingMirror to assert the presence of all key strings and that the guard snippet precedes store.OpenWithContext(ctx, dbPath) in the document.

Confidence Score: 5/5

Safe to merge — the change is purely additive documentation and guidance, with no modification to existing logic or interfaces.

Both changed files are documentation and tests. The SKILL.md prose correctly positions the guard after all earlier exit gates (help probe, dry-run, required-input, dbPath resolution), the skeleton reflects that order verbatim, and the new test pins the key strings and their relative ordering in the document. No runtime code paths are altered.

No files require special attention.

Important Files Changed

Filename Overview
skills/printing-press/SKILL.md Adds the missing-mirror guard prose, standalone copyable snippet, and skeleton injection; prose correctly orders the guard after dryRunOK, usageErr, and dbPath resolution, before any SQLite open call.
internal/cli/printing_press_skill_test.go New test asserts six required strings are present in SKILL.md and uses strings.Index ordering to verify the guard snippet precedes the full store.OpenWithContext call in the skeleton; logic is sound because require.Contains guarantees a non-(-1) guard index before the ordering assertion.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[RunE entry] --> B{bare invocation?}
    B -- yes --> C[return cmd.Help]
    B -- no --> D{dryRunOK?}
    D -- yes --> E[print dry-run msg and return nil]
    D -- no --> F{required input missing?}
    F -- yes --> G[cmd.Usage and return usageErr exit-2]
    F -- no --> H[resolve dbPath]
    H --> I{os.IsNotExist dbPath?}
    I -- yes --> J[print hint to stderr]
    J --> K{asJSON or agent?}
    K -- yes --> L[print empty array to stdout]
    K -- no --> M[return nil]
    L --> M
    I -- no --> N[store.OpenWithContext]
    N --> O[query SQLite and scan results]
    O --> P{asJSON or not terminal?}
    P -- yes --> Q[json.Encode results]
    P -- no --> R[human table output]
Loading

Reviews (4): Last reviewed commit: "fix(skills): document sqlite mirror exit..." | Re-trigger Greptile

Comment thread internal/cli/printing_press_skill_test.go
Comment thread skills/printing-press/SKILL.md Outdated
@tmchow tmchow force-pushed the fix/issue-2393-sqlite-mirror-guard branch from 6bb6b8b to 508cbdf Compare June 7, 2026 22:29
@tmchow tmchow added the ready-to-merge Allow Mergify to queue and merge this PR when protections pass label Jun 7, 2026
mergify Bot added a commit that referenced this pull request Jun 7, 2026
@mergify mergify Bot added the queued PR is in the Mergify merge queue label Jun 7, 2026
mergify Bot added a commit that referenced this pull request Jun 7, 2026
@mergify

mergify Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

This pull request spent 9 minutes 19 seconds in the queue, including 9 minutes 2 seconds running CI.

Waiting for
  • check-success = build-and-test
  • check-success = build-and-test
  • check-success = pr-title
All conditions

Reason

The merge conditions cannot be satisfied due to failing checks

Failing checks:

Hint

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio queue comment.

@mergify mergify Bot added dequeued PR was removed from the Mergify merge queue and removed queued PR is in the Mergify merge queue labels Jun 7, 2026
@tmchow tmchow removed the dequeued PR was removed from the Mergify merge queue label Jun 7, 2026
@tmchow

tmchow commented Jun 7, 2026

Copy link
Copy Markdown
Owner Author

@Mergifyio queue

@mergify mergify Bot added queued PR is in the Mergify merge queue dequeued PR was removed from the Mergify merge queue labels Jun 7, 2026
mergify Bot added a commit that referenced this pull request Jun 7, 2026
@mergify mergify Bot removed the dequeued PR was removed from the Mergify merge queue label Jun 7, 2026
mergify Bot added a commit that referenced this pull request Jun 7, 2026
mergify Bot added a commit that referenced this pull request Jun 7, 2026
mergify Bot added a commit that referenced this pull request Jun 7, 2026
@mergify mergify Bot merged commit 393347a into main Jun 7, 2026
35 of 37 checks passed
@mergify mergify Bot deleted the fix/issue-2393-sqlite-mirror-guard branch June 7, 2026 23:37
@mergify

mergify Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

This pull request spent 23 minutes 8 seconds in the queue, including 22 minutes 43 seconds running CI.

Required conditions to merge

@mergify mergify Bot removed the queued PR is in the Mergify merge queue label Jun 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Allow Mergify to queue and merge this PR when protections pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

skill: SKILL.md Phase 3 RunE template missing 'missing local mirror' guard for SQLite-backed novel commands

1 participant