Repository navigation
Align discovery onboarding with the CompleteTech Skills package - #1
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Second-pass alignment pushed at c2b3cdd: synchronized branding/handoff and maintainer guidance, contributor/PR/editor standards, stronger read-only file/link/PNG validation and Linux/Windows/macOS CI. Discovery's existing generators, templates, SKILL.md, dependencies and approval boundaries remain intact. Local common synthetic tests passed (10 parameterized methods); final CI and review must pass before merge. No visual-render verification is claimed. @codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2b3cdde35
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| elif kind == b"IDAT": | ||
| image = image or bool(length) |
There was a problem hiding this comment.
A PNG containing any nonempty, CRC-correct IDAT payload is accepted without checking that the payload is a valid zlib stream or obeys the image parameters declared by IHDR. For example, replacing the logo's image data with a one-byte IDAT payload and recomputing its CRC still makes validate_package.py report success, although PNG decoders cannot render the file. This allows a corrupted checkout logo to pass the package contract; validate the bounded compressed stream and required scanline structure before setting the image as valid.
Useful? React with 👍 / 👎.
| if not isinstance(repo, str) or repo not in (f"CompleteTech-LLC/{name}", f"CompleteTech-LLC/{name}-skill"): | ||
| errors.append("repository does not match skill_name") |
There was a problem hiding this comment.
When skill_name already ends in -skill, the second accepted repository form appends that suffix again. For this package, changing the manifest repository to CompleteTech-LLC/agentic-discovery-skill-skill still produces no validation errors, so a repository typo can pass the package contract and defeat the intended metadata synchronization. Only allow the suffixed form when the skill name does not already have that suffix, or encode the exceptional repository mapping explicitly.
Useful? React with 👍 / 👎.
| readme = local_file(root, "README.md").read_text(encoding="utf-8") | ||
| errors.extend(f"missing README navigation: {item}" for item in NAVIGATION if item not in readme) | ||
| validate_png(local_file(root, "assets/logo.png")) | ||
| for document in ("ONBOARDING.md", "CONTRIBUTING.md", "BRANDING.md", "AGENTS.md"): |
There was a problem hiding this comment.
The link scan excludes README.md, even though it is a required document and currently links to the workflow diagram and all three example artifacts. Deleting or renaming any of those targets still leaves validate_package.py green, so the package contract can approve a checkout whose primary user-facing page contains broken local links. Include at least the required Markdown documents, especially README and SKILL, in this scan.
Useful? React with 👍 / 👎.
| python scripts/validate_package.py | ||
| ``` | ||
|
|
||
| Also run the existing quality and regression commands in [README.md](README.md). Package checks are read-only and do not execute specialist generators. They validate metadata, required files, local documentation links and PNG chunk integrity, not decoded artwork or document truth. Do not weaken original Quality checks to obtain a green result. |
There was a problem hiding this comment.
This directs contributors to quality and regression commands in README.md, but that file contains no quality-validation command; the actual python3 scripts/validate_quality.py instruction is only in QUALITY.md. A contributor following the new verification section can therefore stop after the package checks and omit the required existing Quality gate. Link to QUALITY.md or include the command directly.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
2 participants
Summary
Coordinated with CompleteTech-LLC/agentic-services-orchestrator-skill#1. Add consistent family navigation, specialist onboarding, CONTRIBUTING.md, skill-package.json, the read-only standard-library validator, 17 synthetic regression tests, and separate least-privilege package CI. Includes the reviewed PNG chunk/CRC validation improvement.
The safe demonstration uses bundled Markdown and writes to output/onboarding-demo.pdf. Existing discovery templates and facts-versus-assumptions guidance are preserved.
Compatibility
No existing runtime code, template, SKILL.md, dependency, approval policy, brand asset, registry version, or repository visibility is changed. Original Quality CI is retained. Structural checks do not verify client facts or approve a proposal.
Validation
The shared 17-test synthetic suite passed locally as part of the 24-test hub suite. Full-checkout package validation and original Quality checks must pass on GitHub before merge. No local full-repository or visual-render verification is claimed; the execution environment cannot clone GitHub.