Repository navigation
fix(ship-check): require exact model ids in attribution #23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,12 +45,20 @@ orchestrator triage, pr-monitor replies). | |
|
|
||
| - `<component>` is the phase or role: `pr-review`, `code-quality`, `test-audit`, | ||
| `bug-check`, `pr-monitor`, or `triage` (for orchestrator inter-phase triage posts). | ||
| - `<model-id>` is the poster's own model ID, read from the system prompt ("You are | ||
| powered by the model named..."). Agents self-identify — the orchestrator does not | ||
| look up or pass model IDs for them. | ||
| - `<model-id>` is the poster's exact runtime model ID, including version and variant | ||
| suffixes (for example, `gpt-5.6-sol`). Never shorten it to a family label such as | ||
| `gpt-5`, translate it to a display name, or guess. Read it from runtime system | ||
| context. In Codex, when visible system prose is generic, first obtain the current | ||
| thread or session ID from runtime-provided task metadata, match it exactly to | ||
| `session_meta.payload.id` in the rollout metadata, then read | ||
| `session_meta.payload.base_instructions.provenance.model`. Never select a rollout by | ||
| recency, cwd, or display name. If the runtime does not expose the current ID or no | ||
| exact model ID can be verified, stop before posting and report the attribution | ||
| blocker. Agents self-identify — the orchestrator does not look up or pass model IDs | ||
| for them. | ||
| - **The orchestrator's own PR-level comments** (non-inline findings, deferred items | ||
| posted via `gh pr comment`) use the orchestrator's own model ID with component | ||
| `triage` or `ship-check`. | ||
| posted via `gh pr comment`) use the orchestrator's own verified exact runtime model | ||
| ID with component `triage` or `ship-check`. | ||
|
|
||
| A comment posted without a footer is indistinguishable from the repo owner's manual | ||
| comments and misattributes automated output to a human. Every `gh api` and | ||
|
|
@@ -70,7 +78,8 @@ commit): | |
| When committing, add this trailer to every commit message (after the body, before | ||
| any trailers the harness adds): | ||
| Ship-Check: PHASE_NAME · YOUR_MODEL_ID | ||
| Read YOUR_MODEL_ID from your system prompt ("You are powered by the model named..."). | ||
| Use your verified exact runtime model ID for YOUR_MODEL_ID. Preserve version and | ||
| variant suffixes; never abbreviate, translate, or guess it. | ||
|
Comment on lines
+81
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Define unverifiable-ID behavior for commit trailers 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 fixMirror 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 |
||
| ``` | ||
|
|
||
| The harness appends its own `Co-Authored-By` trailer automatically; the `Ship-Check` | ||
|
|
@@ -122,7 +131,8 @@ mode" section in your preloaded skill for the gh api template. Only post a revie | |
| you have findings — skip the API call for 0 findings. | ||
| Repo: OWNER_REPO | ||
| Append a footer to the review body AND every inline comment body using your own | ||
| model ID (from your system prompt): | ||
| exact runtime model ID, including version and variant suffixes. Never abbreviate, | ||
| translate, or guess it: | ||
| \n\n---\n*🔍 ship-check · PHASE_NAME · YOUR_MODEL_ID* | ||
| ``` | ||
|
|
||
|
|
@@ -170,7 +180,7 @@ gh repo view --json nameWithOwner -q .nameWithOwner | |
| ``` | ||
|
|
||
| Pass it as `Repo: owner/repo` in the dispatch prompt. In comment mode, agents | ||
| include their own model ID in the footer (they know it from their system prompt) — | ||
| include their own verified exact runtime model ID in the footer — | ||
| the orchestrator does not need to look it up or pass it. | ||
|
|
||
| ## Local review mode (`--local`) | ||
|
|
@@ -358,7 +368,8 @@ Append it to every phase that commits (phases 1, 3-5): | |
| ``` | ||
| When committing, add this trailer to every commit message: | ||
| Ship-Check: PHASE_NAME · YOUR_MODEL_ID | ||
| Read YOUR_MODEL_ID from your system prompt ("You are powered by the model named..."). | ||
| Use your verified exact runtime model ID for YOUR_MODEL_ID. Preserve version and | ||
| variant suffixes; never abbreviate, translate, or guess it. | ||
| ``` | ||
|
|
||
| ### Phase 1: PR Review | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
umm-actually · deepseek/deepseek-v4-flash-0731