Repository navigation
fix(ship-check): require exact model ids in attribution - #23
Conversation
![umm-actually[bot]](https://avatars.githubusercontent.com/in/4233716?s=60&v=4){kind=small}
| stop before posting. No exceptions — a comment without a footer misattributes | ||
| automated output to the repo owner. |
There was a problem hiding this comment.
Report attribution blockers instead of stopping silently
Medium severity · correctness · medium confidence
The new stop-condition halts phase agents before posting when the exact runtime model ID cannot be verified, yet the comment-mode summary templates only support a 0-findings reason for not posting, and the four agent files and four phase skills — unlike pr-monitor and the ship-check orchestrator — omit any instruction to report the blocker. A phase that stops for attribution reasons therefore reports indistinguishably from a clean 0-findings run, letting the orchestrator advance the pipeline and potentially declare the PR merge-ready without that phase's review.
Failure scenario: A Codex run where the runtime does not expose the current session ID, or where it cannot be matched to session_meta.payload.id. The phase agent stops before posting per the new rule and returns its summary with the only not-posted state the template provides (0 findings), so the orchestrator records the phase as clean, dispatches the next phase, and may report the PR merge-ready without this phase's review having landed.
Suggested fix
Extend the stop-condition in the four agent files and four phase skills to "stop before posting and report the attribution blocker to the orchestrator" (matching pr-monitor/SKILL.md and ship-check/SKILL.md), and add a blocked state to the comment-mode summary templates (e.g. "Review posted: no (attribution blocker)") so a stopped phase is distinguishable from one with no findings.umm-actually · deepseek/deepseek-v4-flash-0731
| Use your verified exact runtime model ID for YOUR_MODEL_ID. Preserve version and | ||
| variant suffixes; never abbreviate, translate, or guess it. |
There was a problem hiding this comment.
Define unverifiable-ID behavior for commit trailers
Low severity · correctness · medium confidence
The commit-trailer instructions now require a verified exact runtime model ID but define no behavior when verification fails, while the footer rules in the same file do (stop before posting, report the blocker). A phase agent that cannot verify its ID must choose between committing with a guessed or family-level trailer — explicitly forbidden — or silently dropping the trailer, both of which violate the PR's stated requirement that every ship-check commit trailer carry the exact ID.
Failure scenario: A phase agent in default mode fixes bugs on a Codex run but cannot verify its exact runtime model ID. With no instruction covering the commit path, it either commits with a guessed family-level ID in the Ship-Check trailer (forbidden by the new rule) or omits the trailer, leaving the commit misattributed either way.
Suggested fix
Mirror the footer rule: state that when the exact runtime model ID cannot be verified the agent must stop before committing and report the attribution blocker to the orchestrator, rather than committing with an unverified or omitted trailer.umm-actually · deepseek/deepseek-v4-flash-0731
|
umm-actually reviewed at 2 new finding(s) posted (2 tracked finding(s) across all runs). Context notes
umm-actually · deepseek/deepseek-v4-flash-0731 |
1 participant
Summary
Validation
git diff --check