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
7 changes: 4 additions & 3 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ they disagree with an old comment, issue, or generated summary.
source-code inference, and observed test results. A release-note keyword or
passing help/flag contract test cannot establish full compatibility. Cite
upstream advisories and name any untested interactive or TUI paths.
- If GitHub's automated Copilot review is enabled, apply these instructions to
its comments too. A human maintainer retains the decision to merge releases
and dependency updates.
- The `ProtectMain` ruleset requests Copilot review on new PRs. Request a fresh
review manually after later pushes when needed; automatic review of new
pushes and drafts is not enabled. A human maintainer retains the decision to
merge releases and dependency updates.
8 changes: 6 additions & 2 deletions .github/workflows/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ intermediate files. Linux AOT requires `clang` and `zlib1g-dev`.
scanning where available.
- GitHub Pages from Actions with custom domain `skillview.dev`.
- Release immutability once all assets and checks are validated.
- A `main` ruleset requiring PR review and passing CI checks; allow the
maintainer to merge reviewed dependency PRs, not bots to bypass checks.
- `ProtectMain` targets the default branch and requests Copilot review on new
pull requests. As currently configured, it does not request another review
on new pushes or drafts, and it does not require a pull request or passing
checks before updating `main`. Enable those rules in repository settings if
they should become merge requirements; workflow files alone cannot enforce
them.

`agent_docs/release-engineering.md` contains release development notes.
41 changes: 12 additions & 29 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,23 +148,13 @@ the terminal, with both a full-screen TUI and scriptable CLI commands.
replaced. `tests/SkillView.Tests/Build/PackageReferenceTests.cs` and
`CliDispatcherHelpTests.VersionFlag_IncludesTerminalGuiVersion` hardcode the
exact Terminal.Gui version string — update both alongside any bump.
- xunit was bumped 3.x → 4.0.0 (2026-08); the project didn't use any of the
now-obsolete parallelization APIs (`ParallelizeTestCollections`,
`[CollectionBehavior]`) so no test-code migration was needed. That bump also
triggered a required `global.json` change: `.NET 10 SDK` removed the legacy
VSTest-bridge `dotnet test` path entirely, so `global.json` now sets
`"test": {"runner": "Microsoft.Testing.Platform"}` to opt into the new MTP
`dotnet test` mode. See `agent_docs/running-tests.md` for the resulting
command-syntax changes: `--project` is now required (bare/positional project
paths no longer work), and filtered runs need xunit's MTP-native
`--filter-namespace`/`--filter-class`/`--filter-trait` flags passed straight
to `dotnet test`. For one exact method, use xunit's query filter, for example
`--filter-query "/*/Namespace/Class/Method"`; with the current xunit 4.0.0
integration, `--filter-method` can silently select zero tests even when the
discovered fully qualified name matches. xunit's older
single-dash console-runner flags (`-namespace`, `-trait`, ...) and the
platform's own `--treenode-filter` are silently rejected by `dotnet test`'s
MTP handshake here and report "Zero tests ran" instead of erroring loudly.
- `global.json` selects `Microsoft.Testing.Platform` for xunit 4.0.1. Pass
`--project` for a specific project and use xunit's MTP-native
`--filter-namespace`/`--filter-class`/`--filter-trait` options for filtered
runs. For an exact method, use `--filter-query
"/*/Namespace/Class/Method"`. `--filter-method`, older single-dash xunit
flags, and `--treenode-filter` can silently select zero tests here. See
`agent_docs/running-tests.md` for examples.
- `src/SkillView.Core/SkillView.Core.csproj` owns the default
`TerminalGuiVersion` property. Keep the `PackageReference` on
`Version="$(TerminalGuiVersion)"` so CI can override it via MSBuild without
Expand Down Expand Up @@ -458,25 +448,18 @@ the terminal, with both a full-screen TUI and scriptable CLI commands.
release workflow only generates Homebrew / WinGet artifacts when the repo
variables (`HOMEBREW_TAP_ENABLED`, `HOMEBREW_TAP_REPO`, `WINGET_ENABLED`) are
explicitly enabled. It does not push to a tap repo or submit to WinGet yet.
- Terminal.Gui `2.5.0` remains compatible with the modern
`Application.Create().Init()` lifecycle; the local
`UnconditionalSuppressMessage` workaround and temporary App-level warning mask
stay removed after a verification publish proved the App entrypoint no longer
needs them. Its planned API breaks remove legacy `ConfigurationManager`, make
`View.Text` nonvirtual, and move `IAcceptTarget` to `Terminal.Gui.Input`; keep
using `TuiConfigurationBuilder` and check these boundaries on later upgrades.
- Terminal.Gui `2.5.0` supports `Application.Create().Init()`. Keep using
`TuiConfigurationBuilder` and check the documented lifecycle, `View.Text`,
and `IAcceptTarget` API changes before a later package upgrade.
- CI's standalone AOT smoke publish now promotes `IL2026`, `IL3050`, and
`IL3053` to errors for `SkillView.App`; keep the gh extension's project-level
suppression local until that host gets its separate re-evaluation.
- Prefer `KeyBindings` for view-local command remaps like table preview
shortcuts. Keep the current window/table `KeyDown` routing where the app is
intentionally centralizing whole-screen actions (search/install/open/logs,
installed-screen filter/sort/remove, cleanup actions, etc.), not because of
the old `TableView` type-to-search swallowing bug.
installed-screen filter/sort/remove, cleanup actions, etc.).
- On Terminal.Gui `2.5.0`, `TableView.CollectionNavigator = null` is the
supported way to disable type-to-search. Treat `#5232` as the fix for the old
printable-key swallowing behavior and prefer this documented path over the old
custom matcher workaround.
supported way to disable type-to-search. Do not restore the custom matcher.
- Sanitize untrusted text before assigning it to preview/detail/log panes.
`TerminalEscapeSanitizer` is now the shared UI-layer guard for remote preview
markdown, search metadata, installed-skill detail markdown, cleanup/remove
Expand Down
30 changes: 18 additions & 12 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,17 @@ Thanks for helping improve SkillView. Report bugs and feature requests in [GitHu

Build requirements:

- .NET SDK `10.0.100` or newer in the same feature band
- the .NET SDK selected by [`global.json`](global.json)
- on Linux AOT publish: `clang` and `zlib1g-dev`

### Architecture

SkillView is intentionally small and explicit:
SkillView uses explicit composition and shared services:

- **3 production projects**: `SkillView.Core`, `SkillView.App`, `SkillView.GhExtension`
- **2 test projects**: `SkillView.Tests`, `SkillView.IntegrationTests`
- shared logic lives in `SkillView.Core`
- both executables call the same entry point
- no DI container
- Native AOT-safe code paths by default

Execution flow:
Expand All @@ -31,9 +30,16 @@ Program.cs
-> TUI: SkillViewApp.RunAsync(...)
```

The TUI is a single Terminal.Gui `Window` that hosts a persistent shell: `TabBarView`, `ContextBarView`, and `StatusStripView`. The primary workflows live in embedded `DiscoverTabView`, `InstalledTabView`, and `ChangesTabView` instances under `src/SkillView.Core/Ui/Tabs/`; `SkillViewApp` coordinates tab activation and shared shell state while those views own their workspace layouts. Temporary drill-in workspaces such as `UpdatesTabView` and `DoctorTabView` are still embedded views, but they are not part of the persistent three-tab shell. Tab activation flips `Visible` flags; no nested `Application.Run` subloops are used for the primary workflows. Escalation paths (advanced install wizard, remove wizard, cleanup) keep their modal `Application.Run` semantics intentionally.
The TUI has a persistent Discover, Installed, and Changes shell. `SkillViewApp`
owns that shell and its application lifetime; `SkillViewWorkflowCoordinator`
orchestrates install, update, inventory, remove, cleanup, and Doctor work.
Views under `src/SkillView.Core/Ui/Tabs/` render the primary and temporary
workspaces. Advanced install, remove, and cleanup flows use owned modals.
See the [architecture guide](docs/architecture.md) for the current boundaries.

SkillView uses GitHub CLI's preview `gh skill` commands. It checks for `gh` 2.97.0 or newer and runs `gh skill --help` before enabling the app. GitHub CLI 2.99.0 added support for Pi's `PI_CODING_AGENT_DIR` and corrected Codex's user-scope install location. When GitHub CLI 2.101.0 or newer starts SkillView as an extension, SkillView uses the host-provided `GH_PATH` so subprocesses run through that same CLI executable.
SkillView requires GitHub CLI 2.97.0 or newer and checks `gh skill --help`
before enabling the app. When launched as an extension by GitHub CLI 2.101.0
or newer, it uses the host-provided `GH_PATH` for subprocesses.

### Project layout

Expand All @@ -52,18 +58,18 @@ SkillView uses GitHub CLI's preview `gh skill` commands. It checks for `gh` 2.97
### Build and test

```bash
dotnet restore
dotnet restore --locked-mode
dotnet build
dotnet test --no-build
```

The repo currently pins Terminal.Gui `2.5.0` (Terminal.Gui.Editor `2.5.7`) and
xUnit `4.0.1`. The release uses only stable package versions; the newer
Terminal.Gui development builds are intentionally not consumed. If you pulled
package changes, run `dotnet restore` before building so stale package assets
do not leave the test projects on xUnit 2.x.
NuGet lock files are committed for all projects. After intentionally changing
package versions, run `dotnet restore --force-evaluate`, commit the lock-file
changes, and confirm `dotnet restore --locked-mode` still passes. See
[`agent_docs/running-tests.md`](agent_docs/running-tests.md) for filtered tests.

There is no separate lint step. Build warnings and code-style violations are treated as errors.
CI also verifies formatting with `dotnet format SkillView.sln --no-restore
--verify-no-changes`, lints GitHub Actions, and checks the static site.

### Run locally

Expand Down
6 changes: 3 additions & 3 deletions agent_docs/running-tests.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Running tests

The solution runs on xunit v3/v4's `dotnet test` integration with the .NET 10 SDK's
The solution runs on xunit v4's `dotnet test` integration with the .NET 10 SDK's
`Microsoft.Testing.Platform` runner (opted in via `global.json`'s `test.runner`).
A bare project path (e.g. `dotnet test tests/Foo/Foo.csproj`) no longer works —
pass `--project <path>` explicitly, or run `dotnet test` from the repo root to
Expand All @@ -18,7 +18,7 @@ product failure.
`--filter-not-trait`, or `--filter-query "query"` for the query-filter language.
For a single exact method, use a four-segment query such as
`--filter-query "/*/SkillView.Tests.Ui/SearchAgentMetadataLoaderTests/FilterAsync_TimeoutContainsCallbackFailure"`.
On the current .NET 10/xunit 4.0.0 integration, both simple and fully qualified
On the current .NET 10/xunit 4.0.1 integration, both simple and fully qualified
`--filter-method` values have been observed to reach the runner but silently
select zero tests; do not count such a run as verification. Do **not** use
xunit's older single-dash console-runner
Expand All @@ -40,7 +40,7 @@ flags instead.
2.97.0) and the latest GitHub CLI release. A manual version override applies
only to the latest lane. A failure opens an assigned compatibility issue.
Keep the minimum lane equal to the app's hard minimum.
- **Automation checks:** `node --test .github/scripts/critical-dependencies.test.js`,
- **Automation checks:** `node --test .github/scripts/critical-dependencies.test.js .github/scripts/critical-dependency-assessment.test.js`,
`python site/build.py`, and `node --check site/app.js`. CI also runs
`actionlint` and dependency review for PRs.
- For PTY-driven TUI checks, prefer the built binary and follow `agent_docs/tui-pty-testing.md`.
5 changes: 2 additions & 3 deletions agent_docs/tui-pty-testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,8 @@ Screen text can mislead because:
- result tables truncate repo names
- old frames remain in the PTY buffer

For startup smoke detection, accept the current `Search:` label as well as the
older `Query:` label so a harmless UI copy change does not masquerade as an
event-loop failure.
For startup smoke detection, wait for the `Search:` field rather than relying
on a transient status message.

### 3. Modal screens need unique wait conditions

Expand Down
45 changes: 45 additions & 0 deletions docs/architecture.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# Architecture

SkillView ships two executables: the standalone `skillview` app and the
`gh skillview` extension. Both call `EntryPoint.RunAsync` in `SkillView.Core`,
so CLI behavior and the terminal interface share the same services.

## Entry and command paths

- `src/SkillView.App/` and `src/SkillView.GhExtension/` contain the entrypoints.
- `src/SkillView.Core/Bootstrapping/` parses global arguments and creates
application services. `GhBinaryLocator` resolves `gh` and requires version
2.97.0 or newer. `EnvironmentProbe` checks `gh skill --help` once; SkillView
does not probe individual flags at runtime.
- `src/SkillView.Core/Cli/` dispatches commands and formats text or JSON output.
The small command surface uses explicit parsers so it stays Native AOT safe.
- `src/SkillView.Core/Gh/` invokes `gh` with argument arrays. Install and
listing calls put trusted flags before `--` and selectors after it.

## Terminal interface

`SkillViewApp` owns the Terminal.Gui application lifetime, the persistent
Discover/Installed/Changes shell, and shared pane state.
`SkillViewWorkflowCoordinator` orchestrates install, update, inventory, remove,
cleanup, and Doctor workflows. The views in `src/SkillView.Core/Ui/Tabs/`
render those workflows. Dialogs and background tasks retain explicit lifetime
and cancellation owners; UI updates run through the active application loop.

## Inventory and removal

The inventory combines local skill directories with `gh skill list` data.
`PathIdentity` supplies path equality and containment. Real removal uses the
platform-specific `SecureRemovalBackend` after validation has captured the
target's native identity and its allowed scan root. It never falls back to
recursive path deletion when the secure backend is unavailable. See
[`AGENTS.md`](../AGENTS.md) and
[`agent_docs/ui-lifecycle-and-resource-bounds.md`](../agent_docs/ui-lifecycle-and-resource-bounds.md)
for the detailed safety contracts.

## Builds and releases

The solution targets .NET 10 and Terminal.Gui 2.5.0. Dependency lock files are
committed; CI restores in locked mode, tests on Linux, macOS, and Windows, and
smoke-publishes four Native AOT targets. Release tags trigger the asset,
checksum, provenance, and publication workflow described in
[`agent_docs/release-engineering.md`](../agent_docs/release-engineering.md).
Loading
Loading