Harden portable skill protocol, installer safety, and quality gates - #2
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (19)
Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour. 📝 WalkthroughWalkthroughThe repository adds a Python installer, package validator, evidence-based skill guidance, evaluation scenarios, documentation, regression tests, and CI controls. The shell entry point now delegates installation to Python. ChangesRepository hardening
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to No unresolved material merge risk remains. Sequence Diagram(s)sequenceDiagram
participant Developer
participant skills.sh
participant install.py
participant Filesystem
Developer->>skills.sh: run installation command
skills.sh->>install.py: validate Python and forward arguments
install.py->>Filesystem: validate metadata and destinations
install.py->>Filesystem: create or remove owned links
Filesystem-->>Developer: return operation status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 2.04% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 49 functions across 6 files. (13 skipped: 13 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scope and delivered state
Review and harden code-max without prescribing an application architecture, language, framework, or agent host. Inspected all nine tracked entries at baseline
3a7bc44658ff91e57d4a0609734832b627e9e4d0.Four focused commits cover the portable protocol, conflict-safe optional installer, package/test/CI enforcement, and current Node 24-compatible Actions:
5c21e23refactor(skill): define portable evidence-first engineering gates73068ecfix(installer): preserve existing entries and require explicit targets47d4f8atest(quality): gate package integrity and portable utility regressions9478bf2ci: pin verified Node 24-compatible action releasesFinal head:
9478bf2f2e56732e2f818d2a027738edd6eb61e0.The local candidate exactly matches the published Git tree:
249db1abeac2d8a6744fb14a2f5052235d057281.Confirmed defects addressed
ln -sfninstaller replacing a regular file and nesting a link inside an existing directory.The isolated baseline reproduction verified installer bytes against its Git blob SHA before execution.
docs/hardening.mdrecords findings, severity, evidence, sources, trade-offs, and limits.Implementation
docsaggregate. SHA-pinned checkout v7.0.1 and setup-python v7.0.0, read-only contents, disabled persisted checkout credentials, timeouts, and concurrency cancellation.Observed verification
Final-head CI run 33936477669 completed successfully:
docscheckLocal environment: Linux, Bash 5.2, Python 3.13.5.
python3 -B scripts/validate.py: PASS, 10 Markdown documents and 16 scenario definitions.python3 -B -m unittest discover -s tests -v: PASS, 30 tests after final implementation edits.bash -n skills.sh: PASS.git diff --cached --check: PASS.git write-tree: exact match with the published final tree.The fourth commit changed only the two Action pins after initial CI exposed deprecated Node 20 declarations; the full matrix then passed again on the final head. ShellCheck was unavailable locally but passed remotely. No independent reviewer or real model/host behavioral evaluation was performed. Utility tests do not establish model compliance or universal correctness. Windows-native installation remains unvalidated.
Migration, observability, and rollback
The skill remains instruction-only. Optional installer/validation tooling needs Python 3.10+; no third-party Python packages, hooks, network calls, or application framework were added.
Intentional CLI safety change: bare
./skills.shexits without writes. Select--agent, an absolute parent--target, or explicitly--all. Old Codex/Pi links are not automatically deleted. Dry-run and owned-link uninstall provide an explicit adoption/reversal path.Each operation reports its result. Preflight is not a multi-target transaction: a later I/O failure can leave earlier reported operations completed. Use trusted, user-owned directories; this is not protection against hostile concurrent filesystem replacement. Retry idempotently or remove only successful links to the same checkout. Use a clean reviewed checkout for version rollback; never reset user-owned changes.
No deployment, database migration, or branch-protection changes. License, artwork, historical research, and the CLAUDE.md symlink remain unchanged. This PR is open for review; main has not been changed by this work.