Skip to content

Harden dependency, CI, release, and agent automation #30

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
harder merged 2 commits into from
Oct 5, 2026
Merged

Harden dependency, CI, release, and agent automation #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d8f0437
Harden automation and document agent workflows
harder Oct 5, 2026
98b1cf0
Quote temporary keyring path in contract workflow
harder Oct 5, 2026
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
6 changes: 6 additions & 0 deletions .github/CODEOWNERS
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Maintainer review for safety-critical code and automation.
/.github/ @harder
/src/SkillView.Core/Inventory/ @harder
/src/SkillView.Core/Gh/ @harder
/src/SkillView.Core/Ui/ @harder
/packaging/ @harder
283 changes: 72 additions & 211 deletions .github/copilot-instructions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,211 +1,72 @@
# Copilot Instructions — gh-skillview

Terminal UI and CLI for viewing/managing AI agent skills. Ships as a GitHub CLI
extension (`gh skillview`) and a standalone binary (`skillview`). C# / .NET 10,
Native AOT, Terminal.Gui v2.

## Project status

Phases 0–8 are complete. Current phase roadmap:

| Phase | Scope | Status |
|-------|-------|--------|
| 0 | TG2 + .NET 10 Native AOT feasibility spike | ✅ Done |
| 1 | Environment probe, capability layer, structured logging, Doctor | ✅ Done |
| 2 | Local inventory discovery, SKILL.md parsing, symlink dedup, `gh skill list` adapter | ✅ Done |
| 3 | Search + preview adapters, `SearchScreen` TUI, CLI subcommands | ✅ Done |
| 4 | Install adapter, `InstallScreen` TUI, search→install handoff, CLI subcommand | ✅ Done |
| 5 | Update adapter, dry-run parsing, `UpdateScreen` TUI, TreeSha-axis diff | ✅ Done |
| 6 | §12.1 safe-remove validator, `RemoveService`, cleanup classifier, `RemoveScreen` + `CleanupScreen` TUIs, CLI subcommands | ✅ Done |
| 7 | Argv parser polish, exit-code contract, snapshot tests, dispatcher coverage | ✅ Done |
| 8 | Release engineering — six-RID AOT matrix, SLSA attestations, end-to-end install verification | ✅ Done |
| 9 | Hardening — contract tests, error classification, TG2 upstream review, scan diagnostics | ✅ Done |

Consult `implementation-plan.md` (§22) for detailed phase specs. Durable
agent-facing workflow notes now live in `AGENTS.md` and focused files under
`agent_docs/`.

## Build, test, publish

```bash
dotnet restore
dotnet build
dotnet test # full suite

# Filtered runs: use xunit's MTP-native --filter-* flags directly (no `--`
# separator, and NOT xunit's old single-dash console-runner flags). Scope to
# --project SkillView.Tests — filtering the whole solution fails the run when
# the *other* test project legitimately matches zero tests (xunit/xunit#3077).
dotnet test --project tests/SkillView.Tests/SkillView.Tests.csproj --filter-class "SkillView.Tests.Logging.RedactorTests" # single test class
dotnet test --project tests/SkillView.Tests/SkillView.Tests.csproj --filter-query "/*/SkillView.Tests.Logging/RedactorTests/RedactsGhTokens" # single test

# AOT publish (Linux needs clang + zlib1g-dev):
dotnet publish src/SkillView.App -c Release -r osx-arm64 \
-p:PublishAot=true -p:StripSymbols=true -o dist/app
```

No separate lint step — `TreatWarningsAsErrors` and `EnforceCodeStyleInBuild`
are enabled in `Directory.Build.props`, so `dotnet build` catches style issues.

## Architecture

Two thin entry-point projects (`SkillView.App`, `SkillView.GhExtension`) each
call `EntryPoint.RunAsync(args)` in `SkillView.Core`. The binary name
determines `InvocationMode` (Standalone vs GhExtension); presence of a
subcommand determines `DispatchMode` (CLI vs TUI).

```
EntryPoint.RunAsync
├─ ArgParser.Parse → AppOptions (mode, debug, scan-roots, subcommand)
├─ TuiServices.Build() → composition root (manual DI, no framework)
└─ DispatchMode switch
├─ Cli → CliDispatcher.RunAsync (pattern-match on subcommand name)
└─ Tui → SkillViewApp.Run (Terminal.Gui v2 event loop)
```

`TuiServices` is the composition root — a sealed record with a static
`Build()` factory. No DI container; all wiring is explicit and AOT-safe.

### Layer map

| Layer | Responsibility |
|---|---|
| `Bootstrapping` | Argv parsing, entry point, `AppOptions` |
| `Cli` | `CliDispatcher` routes subcommands to handlers; JSON rendering helpers |
| `Environment` | `EnvironmentProbe` — locates `gh`, checks version, probes auth + capabilities |
| `Gh` | Subprocess adapters for `gh skill {search,preview,install,update,list}` |
| `Gh.Models` | Result records for each adapter (`SearchResultSkill`, `InstallResult`, …) |
| `Inventory` | Filesystem scan, SKILL.md front-matter parsing, symlink dedup, `gh skill list` merge |
| `Inventory.Models` | `InstalledSkill`, `SkillFrontMatter`, `Scope`, `ValidityState`, `Provenance` |
| `Logging` | Ring-buffer `Logger`, `Redactor`, `FileLogSink` (daily rotation, 14-day retention) |
| `Subprocess` | `ProcessRunner` — argv-array execution, never shell composition |
| `Ui` | TG2 screens: main window, `SearchScreen`, `InstallScreen`, `UpdateScreen`, `RemoveScreen`, `CleanupScreen` |

### Capability gating

`GhSkillCapabilityProbe` parses `gh skill <sub> --help` output to detect which
flags the installed `gh` version supports. That covers preview probing/support
for shared flags like `--allow-hidden-dirs` as well as install-time gating.
Adapters only emit flags that the probe confirmed — this keeps the app
forward-compatible with evolving `gh` releases.

## Key conventions

### AOT compatibility (critical)

Every code path must be Native AOT safe. This means:

- **No reflection**: no `Type.GetProperties()`, `Activator.CreateInstance()`,
or attribute scanning.
- **JSON**: use `System.Text.Json` source generators (`GhJsonContext`).
Deserialize via `GhJsonContext.Default.<TypeName>`, never `typeof(T)`.
- **Regex**: use `[GeneratedRegex]` partial methods, never `new Regex(...)`.
- **YAML**: hand-rolled subset parser (`FrontMatterParser`), not YamlDotNet.
- **Arg parsing**: hand-rolled `ArgParser`, not any third-party library.
- **TG2 trimming**: `Terminal.Gui` is currently pinned to `2.4.2-develop.53` in
`SkillView.Core.csproj`. Keep the modern
`Application.Create().Init()` / `IApplication.Dispose()` lifecycle, and do
not add new suppressions or `[UnconditionalSuppressMessage]` workarounds
unless a new TG2 analyzer regression is reproduced, documented, and tied to a
`// TODO(tg2): upstream` note. `SkillView.GhExtension` still carries its current
project-level TG2-related `NoWarn` entries until that host is separately
re-evaluated under AOT publish verification.

### Immutability

- Prefer `record` (or `record struct`) over `class` for data types.
- Use `required` + `init` properties, not mutable setters.
- Collections: `ImmutableArray<T>`, `ImmutableDictionary<K,V>`,
`IReadOnlyList<T>`. Avoid `List<T>` in public API surfaces.

### Error handling

- Service methods return result records (`SearchResponse`, `InstallResult`,
etc.) with `ExitCode` / `ErrorMessage` fields — they do not throw.
- `ProcessRunner` returns a `ProcessResult` on startup failure instead of
throwing.
- Infrastructure failures (logging, file I/O) degrade gracefully and log a
warning.

### Subprocess calls

Always go through `ProcessRunner.RunAsync(executable, argsArray)`.
Never compose shell strings. The `Redactor` runs automatically on log output.

### Logging

Call `logger.Debug/Info/Warn/Error(category, message)` where `category` is a
dot-separated label (e.g., `"gh.skill.search"`). Redaction is applied at the
log-writer layer — never log raw tokens, but also don't manually redact.

### Naming and file layout

- Namespace mirrors directory: `SkillView.Gh.Models` → `src/SkillView.Core/Gh/Models/`.
- One type per file; filename matches type name.
- Private fields: `_camelCase`. Parameters: `camelCase`.
- Async methods end in `Async` and accept `CancellationToken`.
- Use `.ConfigureAwait(false)` on awaited calls in library code.

### Exit codes

Use `ExitCodes` constants (`Success = 0`, `UserError = 1`,
`InvalidUsage = 2`, `EnvironmentError = 10`, `NoMatches = 20`). These are
part of the public contract — scripts and agent hooks depend on them.

### Testing

- xUnit with `[Fact]` and `[Theory]`/`[InlineData]`. No mocking libraries.
- Embed test data as raw string literals; no fixture files.
- Test `internal` members via `InternalsVisibleTo` (set in `SkillView.Core.csproj`).
- Snapshot tests for JSON-emitting subcommands live in
`CliDispatcherJsonSnapshotTests`.

### Terminal.Gui v2

- Create screens as `Dialog` subclasses or inline `Dialog` instances with
`Dim.Percent` / `Dim.Fill` sizing.
- Run modals via `_app.Run(dialog)`.
- Bind key handlers on the `Window.KeyDown` event (not `AddCommand` — it's
`protected` in the current TG2 RC surface).
- Use `SelectedCellChanged` on `TableView` for live detail updates.
- Track upstream issues with `// TODO(tg2):` comments.

### Cross-references

Code comments use `§N` notation (e.g., `§12.1`) to reference sections in
`implementation-plan.md`. Consult that file for detailed design rationale
behind validators, classifiers, and reconciliation logic.

## Implementation rules (from §24)

These rules govern all changes — they are the project's architectural invariants:

1. Exactly three production projects plus one test project.
2. Domain logic, services, and most UI code live in `SkillView.Core`.
3. Entrypoints are tiny (two lines each).
4. Composition over inheritance.
5. Capability probes via flag-token membership scans, not help-text structural parsing.
6. Never delete anything above a validated skill directory.
7. Resolve symlinks via realpath before any mutation; require the resolved path to remain inside a known root.
8. Never follow symlinks that escape the scan root after resolution.
9. JSON parsing only where `gh` documents JSON output.
10. Always rescan inventory post-op (install / update / remove).
11. Subprocess invocation uses the argv-array API — never shell composition.
12. Apply redaction at the log-writer layer.
13. No central config file. State persistence lives in `.skillview-ignore` markers or flags.
14. Prefer TG2 v2 components over manual implementations. Reference the TG2 `Examples/` directory before hand-rolling UI.
15. Any TG2 workaround must be marked `// TODO(tg2): upstream` and reviewed in Phase 9.
16. Default log level is Info+; Debug requires `--debug` or `SKILLVIEW_LOG=debug`.

## Key upstream dependencies and gating

- **`gh` minimum**: v2.97.0. Hard-enforced in `GhBinaryLocator.MinimumVersion`; it supplies the full 48-host install selector, including Devin and Grok.
- **`gh skill list --json`**: shipped in v2.94.0. `GhSkillListAdapter` uses it as the primary inventory source.
- **`gh skill update --yes`**: not supported; `--all` is the non-interactive update path. The `UpdateScreen` has guardrails for the interactive-prompt quirk.
- **Custom install directory**: `gh skill install --dir` is the supported destination override; SkillView does not use a `--repo-path` capability.
- **Terminal.Gui v2**: defaulted via `SkillView.Core.csproj` to `2.4.17`.
Keep the modern `Application.Create().Init()` +
`IApplication.Dispose()` lifecycle, tie async UI work to app/dialog lifetime
cancellation, and track any real remaining framework workarounds with
`// TODO(tg2): upstream`. `SkillView.GhExtension` still retains its current
project-level TG2-related `NoWarn` entries pending separate re-evaluation.
# GitHub Copilot instructions

Read [`../AGENTS.md`](../AGENTS.md) before reviewing or changing this repository.
It is the source of truth for project structure, supported versions, safety
invariants, build commands, and focused guides in `agent_docs/`. Do not copy
general project facts into this file; use the current code and `AGENTS.md` when
they disagree with an old comment, issue, or generated summary.

## For automated reviews

- Prioritize correctness, security, data loss, deadlocks, cross-platform
behavior, Native AOT compatibility, and user-visible regressions. Give the
exact triggering path or sequence, impact, and a small actionable fix.
- Review changed code against adjacent call sites and tests. Trace async
ownership through cancellation, queued UI dispatch, completion, and disposal.
A completed worker does not imply its UI completion callback has run.
- Treat removal as a destructive security boundary. Apply the complete
`AGENTS.md` removal contract before suggesting a simpler path or symlink
implementation. Do not recommend path-recursive deletion as a fallback.
- Check adapters against the **minimum supported** `gh` version as well as the
newest release. The minimum-version check replaces old per-flag capability
probing. Flag removals, JSON schema changes, and agent selector changes need
explicit evidence and focused tests.
- For Terminal.Gui updates, inspect lifecycle, UI-thread dispatch, key input,
scrolling, modal ownership, trimming, and both executable hosts. Do not
reintroduce obsolete `ConfigurationManager` APIs or static app lifecycle.
- For dependency PRs, read upstream release notes and the lock-file diff;
identify API or behavior changes that affect this code. A package version
change with no relevant change warrants a short review, not speculative
migration work. Never suggest a version bump that is already present.
- Review workflow edits for least-privilege permissions, pinned action SHAs,
untrusted PR input in shell expressions, concurrency, checksum integrity,
and whether a failure can silently pass. Release assets must remain
immutable once published.
- Confirm a finding against the current diff. Cite the file and line, explain
an observable failure, and separate proven defects from questions. Avoid
style-only comments already enforced by `dotnet build` and `dotnet format`.

## For automated coding agents

- Make the smallest change that solves the requested issue. Do not edit
`AGENTS.md` or other guidance solely to accommodate a speculative review.
- Preserve the existing public CLI, JSON, exit-code, and safety contracts
unless the request explicitly changes them. Add focused regression tests
for changed behavior, especially cancellation and removal boundaries.
- Run the narrow relevant tests first, then `dotnet restore --locked-mode`,
`dotnet build`, and `dotnet test --no-build` before reporting completion.
Run the two product publishes **sequentially** when a change can affect AOT.
- For a package update, update `packages.lock.json` through an intentional
restore, then verify locked restore. Keep the dependency monitor's version
parser, package pins, and version-sensitive tests in sync.
- For a GitHub CLI or Terminal.Gui compatibility issue, use the issue's
checklist as a starting point, inspect upstream evidence, and propose or
implement focused tests. Do not increase the `gh` minimum automatically.
- Do not execute filesystem removal against a developer's real skill roots
while validating a change. Follow the isolated PTY procedure in
`agent_docs/tui-pty-testing.md` when interactive testing is needed.
- In the PR description, state what changed, what was run, and any remaining
platform or live-terminal verification limits. Never claim a release or
deployment succeeded from a local build alone.

## For Copilot issue and PR automation

- Dependency monitor issues and Dependabot PRs are review inputs, not
authorization to merge or publish. Link the relevant issue and upstream
release notes when proposing a compatibility change.
- Keep bot-generated suggestions concrete: name affected SkillView code paths,
expected behavior, and a test that would detect a regression. Do not invent
breaking changes from a version number alone.
- 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.
43 changes: 34 additions & 9 deletions .github/dependabot.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,41 @@
version: 2
updates:
- package-ecosystem: "nuget"
directory: "/"
# A root NuGet scan discovers all five nested project files. Keep the two
# Terminal.Gui packages together while reviewing their API changes manually.
- package-ecosystem: nuget
directory: /
schedule:
interval: "weekly"
interval: weekly
day: monday
time: "09:00"
timezone: America/Chicago
allow:
- dependency-type: all
assignees: [harder]
open-pull-requests-limit: 10
groups:
terminal-gui:
patterns: ["Terminal.Gui", "Terminal.Gui.Editor"]
test-platform:
patterns: ["xunit*", "Microsoft.NET.Test.Sdk"]

- package-ecosystem: "github-actions"
directory: "/"
- package-ecosystem: github-actions
directory: /
schedule:
interval: "weekly"
interval: weekly
day: monday
time: "10:00"
timezone: America/Chicago
assignees: [harder]
open-pull-requests-limit: 10
groups:
github-actions:
patterns:
- "*"
github-maintained:
patterns: ["actions/*", "github/*"]

# global.json is a dependency too; keep SDK roll-forwards reviewable.
- package-ecosystem: dotnet-sdk
directory: /
schedule:
interval: monthly
assignees: [harder]
open-pull-requests-limit: 5
Loading
Loading