Skip to content

Refactor skill discovery into a lightweight agent-led shelf - #17

Open
dd3ok wants to merge 3 commits into
mainfrom
refactor/agent-led-skill-selection
Open

dd3ok wants to merge 3 commits into
mainfrom
refactor/agent-led-skill-selection

Conversation

@dd3ok

@dd3ok dd3ok commented Oct 24, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Replace keyword scores, synonym rules, aliases, and routing tiers with exact refs, literal filters, and host-agent selection.
  • Keep one aggregate snapshot and one hash-bound local classification file. Lifecycle mutations invalidate the snapshot; outside edits require refresh.
  • Add classify; use 50-card internal pages and 10 user-facing candidates. Preserve explicit selection before loading or executing instructions.
  • Keep guarded lifecycle ownership, provenance, deployment, recovery, and containment checks. Recycle discarded skill trees without a permanent-delete fallback.
  • Regenerate all adapters and simplify the canonical skill and documentation.

The PR contains the accumulated Stash work since e17206f, plus the focused review fixes in b1cec4f and dc0e347. Personal catalogs, installed skill contents, backups, and local configuration are not included.

Initial review findings — all fixed

  1. Scoped availability: collect per-catalog failures in the existing snapshot. Healthy scoped queries and exact refs remain usable; requests that require an unavailable catalog return catalog-unavailable, not a misleading complete count. Refresh retries failed roots and reports ok/partial/failed for its scope.
  2. Attribution preservation: omitted local attribution survives reclassification only while the inspected content hash is unchanged. Explicit replacement/clearing still works, stale attribution is not carried across changed content, and upstream provenance still wins.
  3. Disposable-cache validation: reject array-shaped sources and incorrectly typed optional id, displayName, or url fields, then rebuild normally. The guard covers visible records and folded copies.

The first follow-up CI run exposed an intermittent Windows EPERM while concurrent refreshes replaced the same snapshot. dc0e347 adds a Windows-only, EPERM-only publication retry (at most 5 attempts / 200 ms total delay). Persistent and unrelated errors still fail and preserve the previous cache; warm reads incur no retry or additional file I/O.

No new command, dependency, router, cache file, validation framework, or background service was added by these fixes. Runtime changes are limited to four source files (net +31 lines); the bundle grew from 458,711 to 460,543 bytes (+0.40%).

Boundary coverage

  • Missing roots and paths that are files; cold/warm scoped pagination; disabled roots; bare names versus exact refs; case/Unicode-normalized refs.
  • Unavailable-only and mixed scopes, explicit recovery, later disconnection, and cursor invalidation.
  • Reclassification with omitted, replaced, or explicitly cleared attribution; stale-hash rejection; changed-content invalidation; conflicting upstream provenance.
  • Null, string, array, number, and invalid nested source values in both visible and folded cache records; malformed cached failure metadata.
  • Transient Windows rename failures recover; permanent EPERM is bounded; other errors fail immediately without replacing the old cache. A native Windows stress check passed 80 refreshes (20 rounds of 4 concurrent writers).

Verification

  • Windows, Node v24.19.0: final npm run test:all passed — 99 source tests and 11 distribution tests passed; 1 existing platform-specific filename fixture skipped.
  • The workflow-focused suite passed all 42 cases after reproducing the original failures before the fix.
  • Generated-artifact consistency, TypeScript build, canonical/generated Codex skill validation, and git diff --check passed.
  • Package contents checked after the build: 109 files, 554,563 packed bytes / 2,874,427 unpacked bytes; no personal config/cache files detected.
  • Same 1,000-skill synthetic workload, 20 warm samples per run: baseline p50/p95 1.66/2.27 ms; final 1.60/2.40 ms; cold 524 → 542 ms. Post-fix p50 measurements ranged from 1.60 to 1.94 ms. The 50-card projection stayed 7,539 bytes. These local timings are not model latency, token counts, semantic accuracy, or a statistically significant performance claim.
  • Final CI on dc0e347 passed: Ubuntu Node 20, Ubuntu Node 24, Windows Node 24, and macOS Node 24. All three initial review threads are resolved.

Intentional breaking changes and limits

  • Package 0.3.0; config/result schema 2; disposable snapshot schema 4. Old caches rebuild automatically; no legacy command/config compatibility layer is supplied.
  • Remove search, index, installation aliases, relevance fields, and legacy configuration fields.
  • Outside edits and recovered folders require explicit refresh. Semantic selection still depends on metadata and the host agent.
  • Build/test cleanup and lifecycle recycling require the documented OS trash provider.
  • Fresh Claude/Antigravity host sessions and a separate Codex plugin-creator validator were not available; existing distribution metadata tests and available skill validators passed.

The focused re-review found no remaining blocker among the three reported issues. Live installed skills, active states, and user catalogs were not modified. No automatic merge was performed.

@dd3ok dd3ok left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Multi-angle review completed against the requested simplification and repository contracts. Functionality, data integrity, lifecycle safety, performance, selection UX, packaging and complexity were inspected. The existing local suite passes (81 passed, 1 Windows-only fixture skipped), but targeted public-interface probes reproduce the two P2 regressions below. Keep this PR in draft until those are addressed; the P3 cache validation issue is a small robustness follow-up. No production behavior was changed during this review. CI is running separately.

Comment thread src/internal/catalog-snapshot.ts Resolved
Comment thread src/stash-lifecycle.ts Resolved
Comment thread src/internal/catalog-snapshot.ts Resolved
@dd3ok
dd3ok marked this pull request as ready for review October 24, 2026 08:38
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review 🔄 Running since 2026-10-24T08:38:29.358451Z dc0e347 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

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.

1 participant