[codex] Treat unavailable e2e assets as advisory verification - #5
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a distinction between controlling and advisory evidence during MoonSpec verification in both moonspec.verify.md and SKILL.md. Controlling evidence (such as unit tests and repo-local hermetic checks) is required to verify a feature, whereas advisory evidence (such as integration or e2e tests requiring unavailable external resources) is non-blocking. The reviewer pointed out an inconsistency in SKILL.md where an unqualified reference to 'integration tests' still remains in the definition of FULLY_IMPLEMENTED and should be updated to 'repo-local hermetic checks' to align with the new guidelines.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| - `BLOCKED`: the execution environment prevents trustworthy verification, such as `ENVIRONMENT_CONTAMINATED_BY_SKILL_PROJECTION` from the workspace projection preflight. Include the diagnostic, whether it is recoverable in the current runtime, and the minimum repair needed. | ||
|
|
||
| Prefer `ADDITIONAL_WORK_NEEDED` over `NO_DETERMINATION` when a concrete missing code or test gap is visible. Use `BLOCKED` only for environment failures; it says nothing about implementation completeness. | ||
| Do not choose a blocking verdict solely because advisory integration/e2e/map/deployment validation cannot run or fails due to unavailable non-repo assets, services, credentials, or tooling. Use `FULLY_IMPLEMENTED` when all controlling evidence verifies; otherwise gate only on concrete implementation or controllable verification gaps. |
There was a problem hiding this comment.
The definition of FULLY_IMPLEMENTED on line 282 still lists unqualified 'integration tests' as a requirement for verification. To ensure consistency with the new controlling vs. advisory split introduced in this PR (and to match the updated definition in moonspec.verify.md), please update line 282 to specify 'repo-local hermetic checks' instead of unqualified 'integration tests'.
![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: f88f99177b
鈩癸笍 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".
| - `BLOCKED`: the execution environment prevents trustworthy verification, such as `ENVIRONMENT_CONTAMINATED_BY_SKILL_PROJECTION` from the workspace projection preflight. Include the diagnostic, whether it is recoverable in the current runtime, and the minimum repair needed. | ||
|
|
||
| Prefer `ADDITIONAL_WORK_NEEDED` over `NO_DETERMINATION` when a concrete missing code or test gap is visible. Use `BLOCKED` only for environment failures; it says nothing about implementation completeness. | ||
| Do not choose a blocking verdict solely because advisory integration/e2e/map/deployment validation cannot run or fails due to unavailable non-repo assets, services, credentials, or tooling. Use `FULLY_IMPLEMENTED` when all controlling evidence verifies; otherwise gate only on concrete implementation or controllable verification gaps. |
There was a problem hiding this comment.
In the scenario this change targets鈥攃ontrolling checks pass but integration/e2e/map assets are unavailable鈥攖his new instruction says to use FULLY_IMPLEMENTED, while the same Verdict section still defines FULLY_IMPLEMENTED as requiring integration tests to verify. That leaves the skill with mutually exclusive criteria and can cause verifiers to keep blocking on the advisory suites this patch meant to make non-blocking; update the verdict definition here to match the controlling/advisory split.
Useful? React with 馃憤聽/ 馃憥.
1 participant
Summary
Root Cause
A verifier could classify missing runtime assets for e2e/map smoke checks as a blocking verification failure even when implementation and hermetic evidence were otherwise sufficient.
Validation
python3 tools/validate_bundle.pypytest