Skip to content

Fix inventory duplication, cleanup JSON, and PTY quit handling - #29

Merged
harder merged 1 commit into
mainfrom
fix/automated-testing-findings
Sep 30, 2026
Merged

harder merged 1 commit into
mainfrom
fix/automated-testing-findings

Conversation

@harder

@harder harder commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Summary

  • Resolve existing Unix path aliases when merging gh skill list with the filesystem scan, and reuse the case-sensitivity result per parent directory. This removes duplicate /var versus /private/var installs on macOS and avoids repeated directory probing.
  • Keep --json independent of a bare cleanup --candidates option.
  • Accept Ctrl+Alt+Q as quit when the ANSI driver combines bare Escape followed by Ctrl+Q. Extend the opt-in PTY test to cover this sequence, add rapid tab-switching coverage, and document the headless PTY behavior.

Verification

  • dotnet build --no-restore — clean
  • dotnet test --no-build — 828 passed
  • Opt-in PTY startup/quit test — passed; three additional headless Escape → Ctrl+Q runs exited cleanly
  • Live gh contract tests — 6 passed
  • dotnet format SkillView.sln --no-restore --verify-no-changes — passed
  • macOS arm64 Native AOT publishes for both entrypoints — passed
  • Isolated CLI walkthrough: list, remove preview, cleanup JSON, remove apply, and broken-link cleanup — passed
  • Synthetic inventory of 1,500 skills: list now returns 1,500 records in about 1.4 seconds, versus 3,001 records and 25 seconds or a timeout before the fix

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The focused fixes are consistent with repository conventions and include appropriate regression coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes inventory alias duplication, cleanup argument parsing, and PTY quit handling.

Changes:

  • Canonicalizes Unix paths and caches case-sensitivity probes during inventory merging.
  • Prevents bare --candidates from consuming --json.
  • Supports Ctrl+Alt+Q and expands TUI/PT​​Y regression coverage.
File Description
src/​SkillView.Core/​Inventory/​LocalInventoryService.cs Resolves path aliases during merges.
src/​SkillView.Core/​Cli/​CliDispatcher.cs Corrects cleanup option parsing.
src/​SkillView.Core/​Ui/​SkillViewApp.cs Accepts Alt-modified Ctrl+Q.
tests/​SkillView.Tests/​Inventory/​LocalInventoryServiceMergeTests.cs Tests Unix alias deduplication.
tests/​SkillView.Tests/​Cli/​CliDispatcherParserTests.cs Tests bare candidate parsing.
tests/​SkillView.Tests/​Ui/​SkillViewAppTests.cs Tests Ctrl+Alt+Q detection.
tests/​SkillView.Tests/​Ui/​PtyStartupTests.cs Exercises PTY Escape/Ctrl+Q shutdown.
tests/​SkillView.IntegrationTests/​Ui/​SkillViewAppIntegrationTests.cs Covers rapid tab switching.
AGENTS.md Records the PTY quit invariant.
agent_docs/​tui-pty-testing.md Documents headless PTY behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@harder
harder merged commit 4bf16d6 into main Sep 30, 2026
9 checks passed
@harder
harder deleted the fix/automated-testing-findings branch September 30, 2026 03:48
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.

2 participants