Skip to content

ORB-262: make orchestrate own final-head PR readiness - #694

Merged
thomasluizon merged 39 commits into
mainfrom
chore/orchestrate-ready-lifecycle
Aug 8, 2026
Merged

ORB-262: make orchestrate own final-head PR readiness#694
thomasluizon merged 39 commits into
mainfrom
chore/orchestrate-ready-lifecycle

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Aug 7, 2026

Copy link
Copy Markdown
Owner

DEGRADED: same-vendor review

Summary

Makes /orchestrate own the complete same-head/base readiness lifecycle and makes PR size advisory rather than a correctness gate. It also closes the mid-run gaps around connector issue comments, bounded process cleanup, observable waits, and ORB-260 parity guidance.

Linked Linear work: ORB-262 and ORB-260. API canonical-review companion: thomasluizon/orbit-api#463 (ORB-261).

The only product-tree change is the mandatory generated Zod contract artifact required by live API-main drift; no hand-written product behavior changed. No gate or baseline was weakened or reseeded. This delivery process never merges.

Observed failure -> fix -> red-capable regression -> external evidence

Observed failure Fix Red-capable regression Confirmed interface evidence
A correct 14-file/700-line PR, migration plus Designer output, lockfiles, codemods, or other mandatory artifacts could be rejected without a marker. Deleted override-only code and every size rejection/terminal verdict. Counts remain advisory. /ticket splits only at real behavior/deployment boundaries and keeps required output with its source. 14 files/700 lines and 355 generated files deliver without an override; EF migration plus Designer output stays executable; marker absence never rejects; advisory output cannot change the verdict; ticket prose tests separable versus atomic behavior. No external field is used for the decision. GitHub additions, deletions, and changedFiles are retained only as numeric review information.
Review, CI, connector, threads, base, and Linear could refer to different commits while the PR was reported ready, and a partial first page could hide later threads. Added a repository-qualified receipt keyed to current base/head with explicit stale verdicts. The connector fully paginates review threads and the receipt refuses any artifact not marked complete; aggregation rereads live PR/base, newest required CI, current connector, complete threads, and Linear state. Head/base advances, same-SHA failed CI rerun, dismissed connector, reopened or later-page thread, stale Linear, and incomplete-page artifact all prevent READY. PR/check, connector pagination, and compare shapes below.
Thomas had to repeatedly settle CI, fix Codex threads, update main, rerun reviews, and sync Linear. Canonical /orchestrate now runs one bounded proactive loop until simultaneous READY or a genuine permission/external/human-only blocker. It never merges. Receipt aggregation requires every artifact to name one current head/base; run-state stores repo key, PR number, and receipt path, never a bare number. GitHub and Linear shapes below.
A PR behind main could still deliver. verify-delivery reads live compare behind_by; positive values return OUT_OF_DATE with base/head/count. behind_by=1 cannot deliver and preserves exact values. Complete compare shape below; code reads only numeric behind_by.
Same-number API/UI PRs could resolve through caller cwd, and API review could run from UI. Bare PR numbers and review launches require a known --repo; full URLs must map unambiguously; API review cwd is configured API primary main. Same-number two-repo query; missing/unknown repo; API primary cwd; linked review worktree refused. Repository slug comes from live git remote get-url origin, consumed as one string.
UI and API silently loaded divergent review contracts. UI skill plus rubric are canonical; launch hashes both and fails on drift. API #463 installs the exact pair. Deliberate API fixture drift fails. SHA-256 is produced by the installed Node crypto implementation; no response object field is parsed.
The canonical review contract could miss invented external fields, could not inspect an API contract's shipped UI consumer, and could return CLEAN while a newly admitted round-two blocker remained open. Added Blocking external-interface evidence dimension 13; a bounded dimension-7 exception for targeted sibling-primary/paired-PR contract evidence; and an invariant that every admitted round-two blocker is appended OPEN and participates in the verdict. API #463 mirrors the exact bytes. Hook regressions assert all three clauses; launcher parity still fails on any UI/API drift. These changes read no external response field. Current UI/API SHA-256 pairs are exact; reproduction is the hash command in API #463.
A round-two receipt could erase its frozen round-one blockers by rewriting findings: []. Round 1 persists the exact ordered frozenFindingIds; round 2 cannot change it, and readiness verifies every frozen ID remains a Blocking finding. Empty/dropped frozen lists are REVIEW_STALE; a preserved CLOSED list can become READY. Harness-owned receipt shape only; the canonical UI/API contract bytes remain exact.
API-relative paths, API's P0/P1 floor, required review OIDs, and the frozen rubric source were underspecified. Canonical review now interprets src/ and tests/ against the API target, drops Medium/Low/Info before receipts/tickets, requests both OIDs, and snapshots the rubric from captured baseRefOid. Hook regressions assert every clause; UI/API byte parity remains exact. Complete selected gh pr view key/type evidence and reproduction appear here and in API #463.
gh auth switch mutated global account state, and the thread resolver later reproduced the same class by inheriting the globally active account. Every GitHub path, including reply/resolve mutations, requires an explicit repository; owner token is selected without printing it and exists only in each child GH_TOKEN; inherited token variables are replaced; errors redact secrets. Concurrent different-owner launches stay isolated; resolver missing/unknown repo fails; parent env is unchanged; token patterns redact; source scan contains no switch path. gh auth token --user <TARGET_OWNER> returns one token string; the first live resolver attempt failed before mutation, proving the wrong global account could not be relied upon.
Broad staging could absorb unrelated residue; tracked .orca could be silently discarded; aliases, automatic commit staging, and Git pathspec magic could bypass the hook. Worker hooks block broad add/stage forms, commit -a/--all, indirect or abbreviated pathspec-file flags, and all non-literal magic including :, :/, and empty :(literal). Named literal bracketed paths remain valid, and attached -S<keyid> values are consumed rather than misread as staging flags. Exact red cases cover every broad form plus safe named literals and git commit -Sapi; salvage also rejects unselected source. Installed Git 2.52 dry-runs proved the synonyms, abbreviations, commit-all flags, and magic forms matched the whole tree; installed git commit -h proves -S[<keyid>] is value-bearing.
Codex-only bodies relied on memory for the degradation banner; launcher/recorder edits did not invalidate old green CI; linked worktrees could not share an invalidation; and post-worker GitHub calls ran after both launcher clocks and the wake source were cleared. Before launcher, delivery, or receipt aggregation edits the body, one shared helper persists the exact head/base and pre-edit Guards identities under the repository's Git common directory. Every checkout refuses readiness until newer instances register. Bounded launcher children keep the wake source live. Launcher and recorder edits create exact receipts before gh pr edit; unchanged old rollups stay CI_PENDING/CI_STALE; a later process with newer Guards instances settles; hanging post-worker descendants are removed. Complete live launcher PR-list shape plus installed Git common-directory evidence below.
Red CI output lost inspectable identity, and a completed conclusion outside the old failure set (for example STALE) could be treated as green. Failed/pending entries retain check/run ID, details URL, workflow, name, status, and conclusion; newest rerun wins. Completed checks pass only on the confirmed SUCCESS, NEUTRAL, or SKIPPED allowlist; every other or future value fails closed. Exact failed metadata, later-rerun supersession, live STALE, and an unknown future conclusion. Complete selected check shapes and live enum introspection below.
Salvage could stage or push untested work, inherit unrelated pre-staged files, test a larger dirty tree than the committed subset, push a caller-named branch different from the checked-out worktree branch, require a PR number before one existed, and then disappear from readiness. Salvage proves the symbolic branch exactly matches --branch, refuses protected main, inventories the tree, requires a real green test receipt, rejects every unselected source path while allowing only untracked .orca/ residue, and refuses any staged path outside the exact named set. Stale/protected branch, failed receipt, unrelated index, or omitted dirty source all stop before push; pre-PR salvage and bracketed literals succeed on the exact declared branch. Installed Git branch/refspec evidence plus test child outcomes appear below.
CI polling could validate head/base once, then emit DELIVERED after a push or base advance; a partial rollup could pass before independent required workflows registered. Every poll refresh revalidates head, draft, compare behind_by, and the protected base branch's required-check inventory. Every missing required context is NOT_REGISTERED/CI_PENDING. Branch advances during the poll => STALE_PR; empty and one-fast-check partial rollups remain pending until every required context registers. Complete selected PR/check, branch-protection, and compare shapes below.
Review receipts accepted self-review/arbitrary rounds, Linear receipts could identify another ticket/repo/PR, and visible tickets could never leave the readiness loop while correctly remaining In Progress. READY requires reviewerKind: independent, rounds 1..2, and exact Linear issue/repository/PR identity. A synchronized visual/In Progress pair can reach technical READY with visualCheckOwed:true; premature visual/In Review remains stale. Self/round 9 => REVIEW_STALE; wrong Linear identity is rejected; visual/In Progress => READY; wrong visual status => LINEAR_STALE. Linear artifacts are harness-produced from the confirmed Orca issue state below.
Repository-qualified tests passed only on Thomas's Windows checkout because they loaded absolute real-config paths. GitHub-consuming tests now run through staged configs pointing at hermetic UI/API Git fixtures. Full tools gate exercises verify-delivery, list-bot-threads, and resolver through those fixtures. Repository owner is derived from each fixture's real git remote get-url origin string.
test-tools hung for over ten minutes and parent termination left a Node descendant; Linux PID 1 can leave a killed descendant as a defunct zombie that kill(pid, 0) still sees. Windows avoids the unbounded WSL/app-alias Bash probe; Git Bash probes are bounded; timeout kills the full process tree; Linux assertions treat /proc/<pid>/stat state Z as terminated. All four hanging-child cases spawn descendants; the bound fires and no running descendant remains, including an unreaped zombie. Live Node child shape below; Linux reproduction is included with the process evidence.
list-bot-threads exceeded 120 seconds without useful output and could orphan descendants. Every GitHub operation has a hard command timeout; timeouts terminate the full child tree; the overall connector wait emits structured progress on stderr. A hanging GitHub stub spawns a descendant; the read times out, reports the bound, and both processes are gone. Initial progress is asserted. Live Node child shape and connector GraphQL shape below.
A current-head connector Review in live state PENDING or DISMISSED could be accepted as a completed pass. Review evidence now uses the proved allowlist APPROVED, CHANGES_REQUESTED, or COMMENTED; pending and dismissed Reviews remain NO_REVIEW. Red cases for both PENDING and DISMISSED; issue-comment evidence remains separately head-pinned. Complete live PullRequestReviewState enum and reproduction below.
Delivery verification itself could hang forever even though the surrounding readiness loop was described as bounded. Every Git and GitHub child in verify-delivery now uses the full-tree bounded runner with a configurable hard timeout. A hanging GitHub stub creates a descendant; the verifier times out in one second and proves the descendant gone. Live Node child shape below.
A clean connector result can be an issue comment rather than a Review object. Accept the measured clean bot issue comment only when its Reviewed commit SHA prefix matches the full current head; stale comments are NO_REVIEW. Current CHANGES_REQUESTED Reviews still block. Current comment is REVIEWED; old-head comment is NO_REVIEW; a human copy is ignored. Live PR #690 response proves comments.nodes and a 10-character reviewed SHA prefix; shape below.
Linear state/comment drift was manual and spammed duplicates; caller state could disagree with the live visible-effect label in either direction, and Orca reads/writes were unbounded. Working/blocked maps to In Progress. For readiness, the live label is authoritative in both directions: visible work remains In Progress and ordinary work moves to In Review. Every Orca child has a hard timeout and full-tree cleanup. Mistaken ready and mistaken visual requests are both corrected from the live label; duplicate suppression, LINEAR_STALE, and a hanging Linear descendant removed after timeout. Complete live --full Orca issue shape below; writes depend only on exit status.
ORB-260 CI help still said “platform adapters only” after narrow layout-shell exceptions were allowed. Root contract, both one-sided CI errors, and the live parity:exempt label now name the exact closed exception list. Gate logic is unchanged. Exact guidance must occur twice; obsolete text is absent; label bypass expression and both one-sided count failures remain exact. Live label selected shape below.
API main advanced while this PR was open, making Contract Drift fail on an optional habitIds request field. Ran the repository-prescribed generator and committed the exact required Zod artifact with its API source contract; no gate/baseline edit. Contract Drift's own red diff is exactly the generated two-line optional field; final CI re-runs on this head. Generator: npm run generate:zod -w @orbit/shared using Orval 8.20.0 against current API main.

Verification

  • node tools/test-tools.mjs: PASS on final harness code; 19 scripts and 8 libraries structurally covered, all decision paths green, completed in 216.7 seconds.
  • node .claude/hooks/test-hooks.mjs: PASS on final local head, including ORB-260 guidance and unchanged gate assertions.
  • node tools/arch-map.mjs: completed; architecture.json and architecture.html had no content drift.
  • Pre-commit dash/root-allowlist hooks and pre-push protected-target hook: PASS.

Final-head GitHub CI, independent pr-review, current connector result, thread count, base freshness, and Linear receipt are reacquired after every push; their current status is not inferred from these local results.

Current pushed UI head: 7b5b647a8e5db76591da3a178db829cfc22ff9d0. Current base: 868cd816b7f609318ac37d9dbf6f3ee925d7df30. Current canonical hashes: SKILL.md 3FBF6D133B4A7BAFBE8DA8345EA10D45FF96AC6B83D6C78F348651FA3C830059; rubric.md 672F14194F2BC7834828D434BABDAD152D5B361B2059B75F34F5B0D370BE72B6.

Installed Git 2.52 accepts the unambiguous long-option abbreviations git add --a and git add --up; a live temporary-repository reproduction exited zero and respectively selected all tracked/untracked changes and all tracked updates. git add -h exposes the complete relevant option spellings as --[no-]all and --[no-]update. Reproduction and observed behavior: current-head connector evidence. The hook therefore compares every supplied long-option prefix against the complete dangerous allowlist instead of assuming callers use the documented full spelling.

Confirmed external response shapes

Values below are replaced by type names. Tokens, credentials, personal data, and account values are omitted. Commands derive credentials into child scope and remove them immediately.

GitHub PR, checks, and PR list

Reproduce:

$env:GH_TOKEN = gh auth token --user <TARGET_OWNER>
gh pr view <PR> --repo <OWNER/REPO> --json number,baseRefName,baseRefOid,headRefOid,isDraft,statusCheckRollup
gh pr view <PR> --repo <OWNER/REPO> --json number,baseRefName,baseRefOid,headRefOid,isDraft,body,statusCheckRollup
gh pr view <PR> --repo <OWNER/REPO> --json number,baseRefOid,headRefOid,isDraft
gh pr list --repo <OWNER/REPO> --state all --limit 1 --json number,url,headRefOid,additions,deletions,title,body,changedFiles
gh pr list --repo <OWNER/REPO> --head <BRANCH> --json number,body,baseRefOid,headRefOid,statusCheckRollup
gh run list --repo <OWNER/REPO> --workflow guards.yml --commit <HEAD_SHA> --limit 100 --json databaseId,createdAt,headSha,status,conclusion
Remove-Item Env:GH_TOKEN

Complete selected shapes:

PR object {
  baseRefName:string, baseRefOid:string, headRefOid:string, isDraft:boolean, number:number,
  statusCheckRollup:array<CheckRun|StatusContext>
}
Record-readiness PR object {
  baseRefName:string, baseRefOid:string, body:string, headRefOid:string,
  isDraft:boolean, number:number, statusCheckRollup:array<CheckRun|StatusContext>
}
Stop-hook live PR object {
  baseRefName:string, baseRefOid:string, headRefOid:string, isDraft:boolean, number:number,
  statusCheckRollup:array<CheckRun|StatusContext>
}
CheckRun {
  __typename:string, completedAt:string, conclusion:string, detailsUrl:string, name:string,
  startedAt:string, status:string, workflowName:string
}
StatusContext {
  __typename:string, context:string, startedAt:string, state:string, targetUrl:string
}
PR list: array<{
  additions:number, body:string, changedFiles:number, deletions:number,
  headRefOid:string, number:number, title:string, url:string
}>
Launcher PR list: array<{
  baseRefOid:string, body:string, headRefOid:string, number:number,
  statusCheckRollup:array<CheckRun|StatusContext>
}>
Guards workflow runs: array<{
  conclusion:string, createdAt:string, databaseId:number, headSha:string, status:string
}>

Observed __typename: CheckRun, StatusContext. Live GraphQL introspection returned the complete CheckConclusionState enum: ACTION_REQUIRED, TIMED_OUT, CANCELLED, FAILURE, SUCCESS, NEUTRAL, SKIPPED, STARTUP_FAILURE, STALE. Only SUCCESS, NEUTRAL, and SKIPPED pass; every other or future completed value fails closed. The complete StatusState enum is EXPECTED, ERROR, FAILURE, PENDING, SUCCESS; only SUCCESS passes, EXPECTED/PENDING remain pending, and the rest fail.

Reproduce the enum proof:

$env:GH_TOKEN = gh auth token --user <TARGET_OWNER>
gh api graphql -f query='{check:__type(name:"CheckConclusionState"){kind name enumValues{description name}} status:__type(name:"StatusState"){kind name enumValues{description name}}}'
Remove-Item Env:GH_TOKEN

Complete introspection result shape:

object { data:{
  check:{kind:string,name:string,enumValues:array<{description:string,name:string}>},
  status:{kind:string,name:string,enumValues:array<{description:string,name:string}>}
}}

GitHub required-status inventory

Reproduce:

$env:GH_TOKEN = gh auth token --user <TARGET_OWNER>
gh api repos/<OWNER>/<REPO>/branches/<BASE_BRANCH>/protection/required_status_checks
Remove-Item Env:GH_TOKEN

Complete live response shape for both protected main branches:

object {
  url:string, strict:boolean, contexts:array<string>, contexts_url:string,
  checks:array<{context:string,app_id:number}>
}

verify-delivery reads only contexts. The live UI list contains 20 names and the API list 15;
missing names are reported individually as NOT_REGISTERED, while registered advisory checks are
still evaluated and cannot hide a failure.

GitHub compare

Reproduce:

$env:GH_TOKEN = gh auth token --user <TARGET_OWNER>
gh api repos/<OWNER>/<REPO>/compare/<BASE>...<HEAD>
Remove-Item Env:GH_TOKEN

Complete top-level live key/type set:

object {
  url:string, html_url:string, permalink_url:string, diff_url:string, patch_url:string,
  base_commit:object, merge_base_commit:object, status:string,
  ahead_by:number, behind_by:number, total_commits:number, commits:array, files:array
}

The code reads only numeric behind_by; no compare status value is assumed.

GitHub Codex Review and issue-comment surfaces

Reproduce the exact query from tools/list-bot-threads.mjs:

$env:GH_TOKEN = gh auth token --user <TARGET_OWNER>
gh api graphql -F owner=<OWNER> -F repo=<REPO> -F pr=<N> -f query='<QUERY_FROM_FILE>'
Remove-Item Env:GH_TOKEN

The exact query was run live against UI PR #690. Complete selected shape:

object { data:{ repository:{ pullRequest:{
  number:number, isDraft:boolean, baseRefOid:string, headRefOid:string,
  reviews:{nodes:array<{
    author:{login:string}, state:string, submittedAt:string, body:string,
    commit:{oid:string}|null
  }>},
  comments:{nodes:array<{
    author:{login:string}, body:string, createdAt:string, url:string
  }>},
  reviewThreads:{pageInfo:{hasNextPage:boolean,endCursor:string|null},nodes:array<{
    id:string, isResolved:boolean, isOutdated:boolean, path:string, line:number|null,
    comments:{nodes:array<{author:{login:string},body:string}>}
  }>}
}}}}

Measured clean issue comment: GraphQL author login chatgpt-codex-connector (REST exposes the same bot as chatgpt-codex-connector[bot]), body begins Codex Review: Didn't find any major issues. and contains Reviewed commit with a 10-hex-character prefix; createdAt is ISO-8601 and url is a string. PR #690 returned head 445962dc803eaff136458e4d50464bbc91eac64e and clean comment prefix 445962dc80, proving the prefix/full-head relationship used by the tool. Only those two measured bot aliases are accepted.

Live GraphQL introspection returned the complete PullRequestReviewState enum: PENDING, COMMENTED, APPROVED, CHANGES_REQUESTED, DISMISSED. Only APPROVED, CHANGES_REQUESTED, and COMMENTED are completed connector Review evidence; CHANGES_REQUESTED blocks, while PENDING and DISMISSED cannot clear NO_REVIEW.

Reproduce:

$env:GH_TOKEN = gh auth token --user <TARGET_OWNER>
gh api graphql -f 'query=query { __type(name: "PullRequestReviewState") { kind name enumValues { name description isDeprecated deprecationReason } } }'
Remove-Item Env:GH_TOKEN

Complete selected response shape:

object { data:{ __type:{
  kind:string, name:string,
  enumValues:array<{name:string,description:string,isDeprecated:boolean,deprecationReason:string|null}>
}}}

GitHub label

Reproduce:

$env:GH_TOKEN = gh auth token --user <TARGET_OWNER>
gh label list --repo <OWNER/REPO> --search 'parity:exempt' --json name,description,color,id
Remove-Item Env:GH_TOKEN

Complete selected live shape:

array<{color:string,description:string,id:string,name:string}>

The live description is now Parity exemption: platform adapter or enumerated layout-shell divergence; justify in PR body.

Linear issue read and writes

Reproduce with the pinned installed binary after orca skills get orca-linear:

& '<ORCA_EXE>' linear issue ORB-262 --full --json

Complete selected live shape:

object {
  id:string, ok:boolean,
  result:{
    issue:{
      id:string, identifier:string, title:string, url:string, description:string,
      state:{id:string,name:string,type:string,color:string},
      team:{id:string,name:string,key:string,color:string},
      project:{id:string,name:string,color:string}|null,
      cycle:object|null, assignee:object|null,
      labels:array<{id:string,name:string,color:string}>, priority:number,
      estimate:number|null, dueDate:string|null, branchName:string,
      createdAt:string, updatedAt:string
    },
    meta:{requested:object,resolved:object,partial:boolean,includeErrors:array,sections:object},
    comments:array, children:array, attachments:array, relations:array, activity:array
  },
  _meta:{runtimeId:string|null}
}

The sync tool reads only result.issue.state.name, .type, and every labels[].name; labels is confirmed as an array above. Compared closed types are the complete set completed, canceled, duplicate. Status, comment, and attachment writes depend only on command exit status, not an invented response field. Installed response forwarding source: C:\Users\thoma\AppData\Local\Programs\orca\resources\app.asar.unpacked\out\cli\handlers\linear.js:100 and :149.

Git metadata and commit option parsing

Reproduce against installed Git 2.52 from the linked worktree:

git rev-parse --git-common-dir
git commit -h
git symbolic-ref --quiet --short HEAD
git push -h

The first command exited 0 in both the primary checkout and linked worktree and returned one non-empty path string resolving to the same repository Git common directory; no structured field is parsed. The installed commit usage includes -S[<keyid>], proving that the remainder of an attached -S token is a key ID value rather than more short options. git symbolic-ref --quiet --short HEAD exited 0 with the single checked-out branch-name string, and installed push usage identifies the final operand as <refspec>, proving why HEAD:<caller branch> must be rejected unless the caller branch equals the symbolic branch. The persisted invalidation JSON is harness-owned and contains exactly repositoryKey:null|string, prNumber:number, headSha:string, baseSha:string, editedAt:string, guardsRuns:array<{name:string,startedAt:string}>, and preEditWorkflowRuns:array<{conclusion:string,createdAt:string,databaseId:number,headSha:string,status:string}>. A null repository key is safe for launcher-created receipts because the common directory is repository-qualified and the file is PR-qualified; verifier/recorder receipts also store and validate the configured key. A marker clears only for a new run ID absent from the pre-edit snapshot, created at/after the edit for the exact head, and completed successfully; an opened-event job that starts late cannot qualify.

Node child-process termination

Reproduce against the installed Node binary:

node -e "const {spawn}=require('node:child_process'); const c=spawn(process.execPath,['-p','1'],{stdio:['ignore','pipe','pipe']}); const keys=Object.keys(c).sort(); let chunkType=null; c.stdout.once('data',b=>chunkType=b.constructor.name); c.once('close',(code,signal)=>console.log(JSON.stringify({childKeys:keys,pidType:typeof c.pid,stdoutChunkType:chunkType,closeCodeType:typeof code,closeSignalType:signal===null?'null':typeof signal})))"

Complete observed result:

childKeys: [_closesGot,_closesNeeded,_events,_eventsCount,_handle,_maxListeners,connected,
  exitCode,killed,pid,signalCode,spawnargs,spawnfile,stderr,stdin,stdio,stdout]
pidType:number, stdoutChunkType:Buffer, closeCodeType:number, closeSignalType:null

The tool reads only the numeric pid, stdout/stderr byte streams, and close code/signal. Windows uses installed taskkill /T /F /PID; POSIX uses a detached process group and negative-PID kill. Both real regressions prove the descendant is absent after timeout. On Linux, reproduce the defunct-state check with node tools/test-tools.mjs; the helper reads the installed procfs line /proc/<pid>/stat in its literal <pid> (<comm>) <state> ... form and accepts only state Z as terminated after kill(pid, 0) succeeds.

Latest Codex findings addressed

  • Validate the Linear receipt identity before readiness: fixed in 09940dd3; exact issue/repository/PR mismatch regression; replied and resolved.
  • Document the required Linear synchronization flags: fixed in 09940dd3; README now matches --help; replied and resolved.
  • Treat terminated zombie descendants as no longer alive: fixed in 09940dd3; all four process-tree regressions share the Linux-aware helper; replied and resolved.
  • Wait for every required check to register: fixed in 09940dd3; protected-branch inventory plus partial-rollup regression; replied and resolved.
  • Independent review F1-F3 on the previous head: removed the remaining numeric planner policies, reconciled failed-worker prose with the bounded readiness loop, and blocked update/glob staging while preserving literal bracketed names. These changes are on 09940dd3; the superseded review receipt remains BLOCKING by design and a fresh final-head review is reacquired.
  • Allow salvage before a PR number exists: fixed in c8375839; --pr is optional until creation and the result records pending readiness registration.
  • Include the repository in the retry command: fixed in c8375839; the emitted --resolve-only command retains the exact repository key.
  • Independent review F1-F4 on 5ee1c951: fixed in c8375839; salvage refuses unrelated staged paths, connector Review states use a proved allowlist, codex-only body enforcement runs again at delivery and receipt aggregation, and every delivery child has bounded full-tree cleanup. The round-one receipt remains BLOCKING by design until round-two verification on the final head.
  • Derive visual state from the live ticket label: fixed in 312e765c; a live visible-effect label mechanically overrides a mistaken ready request and keeps the ticket In Progress.
  • Bound every Linear CLI invocation: fixed in 312e765c; reads and writes use the shared bounded runner with full descendant-tree cleanup and a red hanging-child regression.
  • Prove the exact non-full Linear response: fixed in 312e765c by removing that interface read; synchronization now requests the already evidenced complete --full shape and validates its state and labels before use.
  • Independent review F1 on 312e765c: fixed in da738111; worker staging recognizes git stage and rejects indirect pathspec-file staging.
  • Bound the post-worker GitHub calls: fixed in da738111; degradation-marker calls are bounded, kill descendants, and remain covered by the wake source.
  • Derive visual state solely from the live label: fixed in da738111; the authoritative label corrects mistaken caller state in both directions.
  • Test the exact subset that salvage commits: fixed in da738111; every unselected dirty source path is rejected before commit/push.
  • Independent review round-two abbreviation proof: fixed in 5be49769; the hook rejects the complete --pathspec... option family, including installed Git's accepted --pathspec-from-f abbreviation, with the exact red case.
  • Independent review F1-F3 on 5be49769: fixed in f09476ac; all root/bare/empty pathspec magic is blocked, resolver GraphQL children are bounded with descendant cleanup, and the documented Linear stdin sentinel works.
  • Block broad staging through git commit: fixed in f09476ac; worker commit -a, -am, and --all are rejected.
  • Paginate review threads before declaring readiness: fixed in f09476ac; the connector follows live pageInfo to exhaustion and readiness requires a complete artifact.
  • Bound the bot-thread GraphQL calls: fixed in f09476ac; reply, resolve, and resolve-only reads/mutations use the shared bounded runner with a hang/tree-kill regression.
  • Independent round-two F4: fixed in d8187b2d; commit-time auto, interactive, patch, dot, glob/magic, and indirect pathspec staging are blocked while explicit named literal commit paths remain allowed.
  • Independent round-two F5: fixed in d8187b2d; thread pagination has one total deadline, a 100-page ceiling, repeated-cursor refusal, and structured per-page progress with a red cursor-cycle test.
  • Fresh independent F1 on d8187b2d: fixed in a413a826; the worker hook prefix-matches Git's dangerous long commit-staging options, so installed Git's accepted --intera and --patc abbreviations are blocked. Exact red cases cover both spellings while explicit named commit paths remain allowed.
  • Derive review cleanliness from blocker entries: fixed in cef4f90a; readiness preserves the findings array and refuses a nominally CLEAN artifact with any OPEN blocking entry.
  • Bound Git operations during salvage and Kill the complete timed-out test tree: fixed in cef4f90a; every Git and workspace-test child uses the shared bounded runner, with real hanging pre-commit and workspace-test descendant cleanup regressions.
  • Define the connector fixer bound: fixed in cef4f90a; one positive caps.connectorFixAttempts value is validated and governs both connector sections.
  • Prove body on the exact PR-view invocation: fixed in this PR body from the live exact number,baseRefName,baseRefOid,headRefOid,isDraft,body command; the complete selected key/type set is recorded above.
  • Reverify CI after mutating the PR body: fixed in cef4f90a; a marker-restoring edit mechanically invalidates the delivery CI artifact until delivery is rerun.
  • Gate queue completion on the readiness receipt: fixed in cef4f90a; run-state maintains an append-only repository-qualified readiness ledger within the exact current session and the stop hook opens every receipt before allowing completion.
  • Validate the frozen rubric OID before readiness: fixed in cef4f90a; rubric base/path evidence is preserved and must match the live base.
  • Revalidate live Linear state during aggregation: fixed in cef4f90a; aggregation rereads the confirmed full issue state/label shape and invalidates stale status or visible-effect artifacts.
  • Parse attached -m values before broad-stage flags: fixed in cef4f90a; attached message/file values are consumed before short staging flags, with safe -mapi and -Fpath-to-message regressions.
  • Independent F1 and connector Recheck CI after restoring the degraded marker: fixed in 9d5c30a1; a body edit returns CI_PENDING immediately and only a later delivery invocation can settle edited-event checks.
  • Independent F2: fixed in 9d5c30a1; the stop hook boundedly rereads the exact live GitHub PR and full Linear issue shapes before accepting a cached READY receipt.
  • Reject malformed blocker flags: fixed in 9d5c30a1; every finding must carry a boolean blocking value and every true blocker must be CLOSED.
  • Match each ledger receipt to its PR identity: fixed in 9d5c30a1; repository key, PR number, head/base/draft, ticket, Linear status, and visible-effect state must all match live values.
  • Independent round-two F3 and connector Revalidate CI and review threads before allowing stop: fixed in 0ee82f9d; the stop hook boundedly rereads newest required CI, current connector evidence, complete thread inventory, and Linear state before accepting cached READY. Same-SHA failed-rerun, dismissed-review, and reopened-thread regressions are green.
  • Persist body-edit invalidation until replacement CI registers: fixed in 0ee82f9d, completed in 2543b28e; delivery stores pre-edit Guards identities in shared repository Git metadata and every checkout remains pending until strictly newer instances appear.
  • Consume attached GPG key IDs before scanning staging flags: fixed in 0ee82f9d; the hook treats the remainder of -S<keyid> as a value, with git commit -Sapi covered while broad commit staging remains blocked.
  • Revalidate GitHub readiness before recording READY: fixed in abc4d2e3; aggregation now rereads newest required CI, current connector evidence, and the complete thread inventory, and combines them with the exact-head input artifacts. Red same-SHA failed-rerun, dismissed-review, and reopened-thread cases all prevent READY.
  • Verify the salvage branch before pushing: fixed in 2e3051c2; salvage resolves the checked-out symbolic branch before testing, rejects any caller mismatch and protected main, then pushes only the already-proved exact branch. Both refusal cases leave the change uncommitted and unpushed.
  • Persist CI invalidation for launcher body edits: fixed in c56770e0; launcher and verifier now share one repository-local receipt implementation, and the launcher records the exact pre-edit head/base and newest Guards runs before gh pr edit. The launcher regression proves the persisted receipt; the verifier regressions prove old green results stay stale until replacement runs register.
  • Reset the ledger when a new session starts: fixed in 2543b28e; prior identities are unioned only when the exact non-empty sessionId matches, while a new session's regression proves the ledger begins empty.
  • Persist invalidation before the recorder edits the body: fixed in 2543b28e; receipt aggregation writes the same common-directory head/base/Guards baseline before gh pr edit, rejects unchanged old runs as CI_STALE, and clears the marker only after newer runs register.
  • Preserve the frozen blocker list in round two: fixed in d9bb423e; canonical UI/API receipts carry immutable frozenFindingIds, and readiness rejects an absent, empty, duplicate, or dropped round-two list.
  • Identify the edited-event Guards run before clearing invalidation: fixed in d9bb423e; every editor snapshots live workflow run IDs and only a new completed successful same-head run created after the edit clears the marker. The zero-baseline regression proves a late-starting opened-event run remains pending.
  • Live final-gate poll crash after async invalidation: fixed in acade79b; the CI loop awaits each refreshed invalidation result. The strengthened test enters a real one-second pending poll and would reproduce the prior rollup.failing TypeError without the fix.
  • Compare complete frozen blocker list: fixed in c2937f06; round two names and SHA-256-verifies the immutable round-one artifact, then readiness requires the exact ordered blocker IDs. A red artifact that drops F2 from both the final ID list and findings returns REVIEW_STALE.
  • Reject closed blocker statuses in round one: fixed in c2937f06; round-one CLEAN is valid only with an empty frozen ID list and no blocking findings. A red round-one CLOSED blocker cannot persist READY.
  • Recheck PR identity before writing READY: fixed in c2937f06; aggregation repeats the confirmed live number,baseRefOid,headRefOid,isDraft query immediately before persisting. A head/base change during connector or Linear reads fails before the receipt write.
  • Verify tests do not mutate staged tree: fixed in c2937f06; salvage fingerprints every explicitly named path before and after the caller-specified green test and refuses mutation before staging or push. The red test mutates a named file while exiting zero and proves nothing is committed or pushed.
  • Block abbreviated broad git-add flags: fixed in a6105a7a; the guard now rejects every unambiguous prefix of --all, --update, and --renormalize, including the live-proved --a and --up, while explicit named literal paths remain allowed. Hook regressions cover both abbreviations.
  • Recheck volatile GitHub state in the closing read: fixed in a6105a7a; immediately before receipt evaluation, aggregation rereads the complete selected PR/status-check shape and the current connector/thread inventory, then replaces the cached values. Red sequence tests turn CI red, dismiss the connector review, and reopen a thread during aggregation; each prevents READY.
  • Independent F1 / connector Reject closed statuses for newly admitted blockers: fixed in 6627da40; only frozen round-one blockers may close in round two. Any newly admitted blocker marked CLOSED is REVIEW_STALE, and an OPEN one keeps the verdict blocking because no third fixer exists.
  • Independent F2 Block literal directory pathspecs: fixed in 6627da40; the worker guard resolves every literal named path and rejects directories for both git add and commit-time staging, preventing subtree sweeps while preserving individual literal files.
  • Connector Persist the authoritative round-one hash before round two: fixed in 6627da40; round-one BLOCKING artifacts are mechanically registered before the fixer under repository Git state with exact path, SHA-256, base/head, and frozen IDs. Round two must match that independent ledger; a forged reduced artifact plus matching caller hash is rejected.
  • Capped-review F2 Block fully deleted directory pathspecs: fixed in 7b5b647a; for a missing literal path the guard boundedly asks the Git index for exact matches. One different descendant or multiple matches identifies a directory/subtree sweep; one exact deleted file remains an allowed named path. The real linked-worktree regression deletes .claude and proves git add .claude is blocked.
  • Capped-review F3 / connector Make round-one registration immutable: fixed in 7b5b647a; an identical registration is idempotent, while any non-identical attempt for the same repo/PR/reviewed head fails without writing. Red tests prove a reduced blocker list cannot replace the ledger.

Deliberately deferred

  • API canonical file parity lives in companion PR fix(ci): restore SonarCloud coverage (turbo test-cache replayed empty lcov) #463 because API repository files cannot belong in this UI PR.
  • The readiness loop stops at genuine permission, external-service, or human-only blockers and reports the exact decision; it never bypasses them.
  • Visible-effect tickets remain In Progress until human visual acceptance. These harness/policy tickets have no visible effect.
  • No PR is merged by this change or delivery process.

@vercel

vercel Bot commented Aug 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
orbit-ui-mobile-web Ignored Ignored Aug 8, 2026 2:51am

Request Review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 50d3a3082b

ℹ️ 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".

Comment thread tools/record-readiness.mjs
Comment thread tools/verify-delivery.mjs Outdated
@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f16d0057ac

ℹ️ 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".

Comment thread tools/record-readiness.mjs Outdated
Comment thread tools/verify-delivery.mjs Outdated
Comment thread tools/__tests__/verify-delivery.mjs Outdated
@thomasluizon

Copy link
Copy Markdown
Owner Author

DEGRADED: same-vendor review

BLOCKING: independent pr-review round 1 on f16d0057ac1c6d0538740aad5e2da71f4cf722e5 found seven Blocking defects. The finding list is frozen.

Blocking findings

  • F1, High, tools/record-readiness.mjs:130: CI evidence is relabeled with the later live head/base rather than retaining the delivery artifact's SHAs. Green CI from head A can therefore satisfy head B after a push.
  • F2, High, tools/lib/readiness-receipt.mjs:45: any string reviewer kind and any positive round count pass. Runtime verification showed reviewerKind: "self" plus rounds: 9 returns READY.
  • F3, High, .claude/hooks/_lib/rules-orchestrator.mjs:41: the staging hook blocks the literal git add . but runtime verification showed it allows both git add -- . and git add ".".
  • F4, High, tools/salvage-worker.mjs:60: salvage accepts broad-equivalent Git pathspecs such as sub/.. and *, so it can stage beyond the caller's named files.
  • F5, High, tools/lib/github-auth.mjs:29: the token-selection operation uses execFileSync timeout and does not use the full-process-tree termination path promised for every GitHub child.
  • F6, High, tools/list-bot-threads.mjs:258: the live proof establishes a 10-hex reviewed-commit prefix, but the parser accepts 7 through 40 hex characters and can accept a non-identical commit sharing a shorter prefix.
  • F7, High, tools/verify-delivery.mjs:328: an empty statusCheckRollup is treated as settled and green, allowing readiness before required checks register.

Follow-up tickets filed

  • None. All seven findings break harness correctness or source-integrity guarantees and are Blocking.

Dimensions marked N/A

  • 4 No-workaround/root cause: no product/API workaround was introduced.
  • 6 Security: no product authorization, session, webhook, key, validation, or leakage surface changed; credential/process isolation was assessed under correctness.
  • 7 Contract alignment/backward compatibility: no request, response, endpoint, DTO, or schema contract changed.
  • 8 Cross-platform parity: no apps/web or apps/mobile product surface changed.
  • 9 i18n: no user-facing locale string changed.
  • 10 Design: no UI file changed.
  • 11 Backend hard rules: no orbit-api source file changed.
  • 12 FEATURES.md: no user-facing feature surface changed.

Dimensions 1, 2, 3, 5, and 13 were walked across the complete 3,902-line harness diff. No Non-blocking findings survived verification. A machine never merges.

@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@thomasluizon

Copy link
Copy Markdown
Owner Author

DEGRADED: same-vendor review

BLOCKING: independent pr-review round 2 on 0ad30d5acbf7489994b55bd5ec57106f44dd8563 closed frozen F1-F7 and admitted one new Blocking defect on the mechanical round-two line set. The two-round review is complete; there is no round 3.

Frozen findings

  • F1 CLOSED: tools/record-readiness.mjs:130 now preserves state.headSha and state.baseSha on CI evidence.
  • F2 CLOSED: tools/lib/readiness-receipt.mjs:47-48 now requires reviewerKind === "independent" and rejects more than two rounds.
  • F3 CLOSED: .claude/hooks/_lib/rules-orchestrator.mjs:41 now catches dot staging after --, quoted dot, slash-dot, and a dot mixed with named paths.
  • F4 CLOSED: tools/salvage-worker.mjs:61-65,108-129 canonicalizes paths, requires exact dirty-file membership, and stages normalized paths.
  • F5 CLOSED: tools/lib/github-auth.mjs:27-34 now selects the token through the full-tree bounded runner, and every touched caller awaits it.
  • F6 CLOSED: tools/list-bot-threads.mjs:258-261 now accepts exactly the measured 10-hex connector prefix and rejects shorter or longer shapes.
  • F7 CLOSED: tools/verify-delivery.mjs:344-346 now records an empty rollup as pending CI registration rather than green.

Admitted round-two blocker

  • F8 OPEN, High, tools/salvage-worker.mjs:62: the new glob-character rejection also rejects valid explicit filenames containing brackets. The frozen rubric itself names the real path apps/web/app/api/[...path]/route.ts; the new predicate evaluates true for that path. Salvage therefore cannot stage a real named route file. Preserve the exact dirty-file membership check and pass paths to Git with literal pathspec semantics instead of outlawing filename characters.

Follow-up tickets filed

  • None. F8 breaks required salvage behavior and is Blocking.

Review scope

  • Rechecked only frozen F1-F7 and the mechanical f16d0057...0ad30d5a touched-line set supplied by the caller.
  • No full-diff rescan and no finding outside that touched-line set.
  • Focused tests and the two complete harness suites were reported green by the fixer; the reviewer independently inspected the touched diff and verified the bracket-path predicate.

A machine never merges.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0ad30d5acb

ℹ️ 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".

Comment thread tools/lib/readiness-receipt.mjs Outdated
@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

console.log(JSON.stringify({ threadId, replied: true, resolved: false, error: resolved.detail, note: `the reply landed; retry with --resolve-only, do not repost the reply`, retry: `node tools/resolve-bot-thread.mjs --thread ${threadId} --resolve-only` }, null, 2))

P2 Badge Include the repository in the generated retry command

When the reply succeeds but the resolve mutation fails, the structured retry command omits the now-required --repo argument. Executing the recovery command immediately exits with usage code 2, leaving the thread unresolved; include the current repoKey in this command.

ℹ️ 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".

Comment thread tools/record-readiness.mjs
Comment thread tools/README.md Outdated
Comment thread tools/__tests__/bounded-process.mjs Outdated
@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@thomasluizon

Copy link
Copy Markdown
Owner Author

DEGRADED: same-vendor review

Verdict: BLOCKING on head 3d5dc3b530335548974be459ee6c5b2ec6271b42 against base f131ba4201174f1fc4ae328686fe33de16e38708 (round 1, independent session).

Blocking findings:

  1. F1 High: the hard 400-line planning rule still survives. At .claude/skills/ticket/SKILL.md:90, the diff says file and line estimates are planning signals, but CLAUDE.md:11 still requires every PR to be under 400 lines, .claude/agents/product-manager.md:21 still requires under about 400, and .claude/skills/_shared/audit-to-tickets.md:32 still targets under 400. Those always-loaded/planning instructions can still force the arbitrary numeric split this change is meant to retire.

  2. F2 High: the proactive readiness lifecycle is contradicted by the queue's no-retry rule. At .claude/skills/orchestrate/SKILL.md:760, the diff begins the bounded repair loop for every existing PR. The same skill later says at lines 883-885 that a failed ticket is skipped with "No retry" and that salvage is the only exception. That permits the old behavior for CI, review, connector, thread, or freshness failures and leaves Thomas to repeat the lifecycle manually.

  3. F3 High: the named-path staging hook still admits broad staging. At .claude/hooks/_lib/rules-orchestrator.mjs:41, the new regex recognizes only -A, --all, and dot pathspecs. Direct execution against the changed function returns allowed for git add -u, git add --update, git add :(glob)**, and git add src/*.ts. Those commands can sweep unrelated tracked source, including tracked .orca changes, despite the stated named-path-only rule.

No non-blocking findings were retained, so no follow-up Linear tickets were filed.

Rubric coverage:

  • Correctness: BLOCKING (F1-F3).
  • Dead/stale code: no retained finding.
  • SOLID/clean architecture: no retained finding.
  • Root cause/no-workaround: no retained finding.
  • Test quality: the added regressions do not cover the three failing paths above; this is evidence for F1-F3, not separate findings.
  • Security: no retained credential or authorization finding.
  • Contract alignment/backward compatibility: N/A; the current base already carries the generated optional habitIds field, so it is no longer in this PR diff.
  • Cross-platform parity: N/A; no apps/web or apps/mobile files changed.
  • i18n: N/A; no user-facing locale surface changed.
  • Design: N/A; no application UI files changed.
  • Backend hard rules: N/A; this is the UI repository and no backend diff is paired into this PR.
  • FEATURES.md: N/A; no user-facing feature surface changed.

The external-interface evidence in the PR body was also checked against the changed reads; no additional retained finding was found. Rubric snapshot: C:\Users\thoma\orca\reviews\ui-pr-694-f131ba4-rubric.md. Receipt: C:\Users\thoma\orca\reviews\ui-pr-694-3d5dc3b-findings.json.

A machine does not merge.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d5dc3b530

ℹ️ 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".

Comment thread tools/verify-delivery.mjs Outdated
@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2e3051c236

ℹ️ 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".

Comment thread tools/launch-worker.mjs Outdated
@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c56770e0a7

ℹ️ 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".

Comment thread tools/lib/run-state.mjs Outdated
Comment thread tools/record-readiness.mjs
@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2543b28ec4

ℹ️ 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".

Comment thread tools/lib/readiness-receipt.mjs Outdated
Comment thread tools/lib/body-edit-invalidation.mjs Outdated
@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d9bb423e28

ℹ️ 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".

Comment thread tools/lib/readiness-receipt.mjs Outdated
Comment thread tools/lib/readiness-receipt.mjs
@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: acade79b0f

ℹ️ 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".

Comment thread tools/record-readiness.mjs
Comment thread tools/salvage-worker.mjs
@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c2937f068b

ℹ️ 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".

Comment thread .claude/hooks/_lib/rules-orchestrator.mjs Outdated
Comment thread tools/record-readiness.mjs Outdated
@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6105a7a5f

ℹ️ 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".

Comment thread tools/lib/readiness-receipt.mjs Outdated
Comment thread tools/record-readiness.mjs
@thomasluizon

Copy link
Copy Markdown
Owner Author

DEGRADED: same-vendor review

Verdict: BLOCKING (round 1)

Blocking findings:

  1. F1 — High — tools/lib/readiness-receipt.mjs:68
    Round-two readiness accepts any blocking finding marked CLOSED, including a newly admitted round-two blocker. The capped contract requires such a finding to be appended OPEN, because there is no later head/round that could have closed it; the current validator nevertheless returns READY for a valid frozen F1 plus an extra blocking F2 marked CLOSED. Require every blocking finding outside frozenFindingIds to remain OPEN (and add the corrupt-receipt regression case).

  2. F2 — High — .claude/hooks/_lib/rules-orchestrator.mjs:58
    isBroadPathspec rejects dot/glob/magic forms but accepts literal directory pathspecs. Consequently git add apps/web, git stage tools, and git commit tools -m x all bypass the worker guard while recursively sweeping unrelated changes/residue from the named subtree. Resolve literal pathspecs and reject directories, with red cases for add/stage/commit.

Follow-up tickets: none.

Dimensions marked N/A:

  • 7 Contract alignment/backward compatibility — no shared API endpoint, Zod schema, or DTO surface changed.
  • 8 Cross-platform parity — no web/mobile product implementation changed.
  • 9 i18n — no user-facing product copy changed.
  • 10 Design — no product UI changed.
  • 11 Backend hard rules — this is the UI repository and no backend code changed.
  • 12 FEATURES.md gating — no user-facing feature, gate, platform availability, or locale behavior changed.

Dimensions 1–6 were applied. Verification on the exact reviewed head passed: node tools/test-tools.mjs (212.1s) and node .claude/hooks/test-hooks.mjs (3.2s).

Canonical receipt:

{
  "reviewerKind": "independent",
  "verdict": "BLOCKING",
  "rounds": 1,
  "reviewedHeadOid": "a6105a7a5f949b3ad4fcbf04f19c2dd8511bcec5",
  "baseSha": "868cd816b7f609318ac37d9dbf6f3ee925d7df30",
  "rubricBaseOid": "868cd816b7f609318ac37d9dbf6f3ee925d7df30",
  "rubricArtifactPath": "C:/Users/thoma/AppData/Local/Temp/orbit-pr-review-694-a6105a7a/pr-694-rubric.md",
  "artifactPath": "C:/Users/thoma/AppData/Local/Temp/orbit-pr-review-694-a6105a7a/findings.json",
  "frozenFindingIds": ["F1", "F2"],
  "findings": [
    {
      "id": "F1",
      "severity": "High",
      "file": "tools/lib/readiness-receipt.mjs",
      "line": 68,
      "claim": "Round-two readiness accepts newly admitted blocking findings as CLOSED, so a final receipt can hide a blocker that the capped review contract requires to remain OPEN and still become READY.",
      "blocking": true
    },
    {
      "id": "F2",
      "severity": "High",
      "file": ".claude/hooks/_lib/rules-orchestrator.mjs",
      "line": 58,
      "claim": "Literal directory pathspecs bypass the broad-staging guard, so commands such as git add apps/web or git commit tools can still sweep unrelated worker residue into a commit.",
      "blocking": true
    }
  ]
}

@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6627da4014

ℹ️ 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".

Comment thread tools/record-readiness.mjs
@thomasluizon

Copy link
Copy Markdown
Owner Author

DEGRADED: same-vendor review

Verdict: BLOCKING (round 2; review cap reached)

Frozen finding disposition:

  1. F1 — CLOSED — tools/lib/readiness-receipt.mjs:82
    The validator now requires every non-frozen blocking finding to remain OPEN; the added regression case exercises the previously accepted false-closure shape.

  2. F2 — OPEN — .claude/hooks/_lib/rules-orchestrator.mjs:66
    The fix rejects directories only while they exist in the working tree. Git also accepts an absent directory pathspec to stage tracked deletions: against the exact-head index, git add --dry-run -- .claude with an empty work tree emitted recursive removals, while checkBroadStaging("git add .claude", ...) returned null. Resolve pathspecs against tracked index entries as well as the filesystem, and cover a fully deleted directory.

Admitted touched-line blocker:

  1. F3 — OPEN — Critical — tools/record-readiness.mjs:119
    writeFileSync(path, ..., "utf8") uses installed Node's default flag: "w", so rerunning --register-round-one overwrites the supposedly immutable pre-fixer ledger. A post-fixer caller can register a rewritten round-one file, then submit matching path/hash/IDs and defeat the new trust boundary. Create the ledger exclusively (wx) and fail if it already exists; add a test proving re-registration cannot replace it.

Follow-up tickets: none.

Dimensions marked N/A remain unchanged from round 1:

  • 7 Contract alignment/backward compatibility — no shared API endpoint, Zod schema, or DTO surface changed.
  • 8 Cross-platform parity — no web/mobile product implementation changed.
  • 9 i18n — no user-facing product copy changed.
  • 10 Design — no product UI changed.
  • 11 Backend hard rules — this is the UI repository and no backend code changed.
  • 12 FEATURES.md gating — no user-facing feature, gate, platform availability, or locale behavior changed.

Canonical receipt:

{
  "reviewerKind": "independent",
  "verdict": "BLOCKING",
  "rounds": 2,
  "reviewedHeadOid": "6627da4014d29ba43b37f6a92990d08d8c6e5a80",
  "baseSha": "868cd816b7f609318ac37d9dbf6f3ee925d7df30",
  "rubricBaseOid": "868cd816b7f609318ac37d9dbf6f3ee925d7df30",
  "rubricArtifactPath": "C:/Users/thoma/AppData/Local/Temp/orbit-pr-review-694-a6105a7a/pr-694-rubric.md",
  "artifactPath": "C:/Users/thoma/AppData/Local/Temp/orbit-pr-review-694-6627da40/findings.json",
  "roundOneArtifactPath": "C:/Users/thoma/AppData/Local/Temp/orbit-pr-review-694-a6105a7a/findings.json",
  "roundOneArtifactSha256": "fb3230b6c3346cf06fd8492a225047a966224fb46dae6855d48e7528e9490dcb",
  "frozenFindingIds": ["F1", "F2"],
  "findings": [
    {
      "id": "F1",
      "severity": "High",
      "file": "tools/lib/readiness-receipt.mjs",
      "line": 68,
      "claim": "Round-two readiness accepts newly admitted blocking findings as CLOSED, so a final receipt can hide a blocker that the capped review contract requires to remain OPEN and still become READY.",
      "blocking": true,
      "status": "CLOSED"
    },
    {
      "id": "F2",
      "severity": "High",
      "file": ".claude/hooks/_lib/rules-orchestrator.mjs",
      "line": 58,
      "claim": "Literal directory pathspecs bypass the broad-staging guard, so commands such as git add apps/web or git commit tools can still sweep unrelated worker residue into a commit.",
      "blocking": true,
      "status": "OPEN"
    },
    {
      "id": "F3",
      "severity": "Critical",
      "file": "tools/record-readiness.mjs",
      "line": 119,
      "claim": "Round-one registration overwrites an existing ledger, so a post-fixer caller can replace the supposedly immutable pre-fixer identity and make a forged round-two receipt pass readiness.",
      "blocking": true,
      "status": "OPEN"
    }
  ]
}

@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@sonarqubecloud

sonarqubecloud Bot commented Aug 8, 2026

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7b5b647a8e

ℹ️ 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".

new Set(blockingIds).size !== blockingIds.length ||
JSON.stringify(blockingIds) !== JSON.stringify(registered.frozenFindingIds)
) fail("round-one registration requires one valid BLOCKING receipt with its exact ordered frozen IDs")
const path = roundOneLedgerPath(registered.reviewedHeadOid)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Include the base SHA in the round-one ledger key

When the target branch advances while the PR head remains unchanged, the review contract requires a fresh round one against the new base, but this path still resolves to the prior head-only ledger file. The new receipt has a different baseSha, so the immutable-ledger comparison rejects it as a replacement and leaves the PR unable to register the required fresh review. Key registrations by both base and head while preserving each existing record's immutability.

Useful? React with 👍 / 👎.

@thomasluizon
thomasluizon merged commit ea7f7c3 into main Aug 8, 2026
32 checks passed
@thomasluizon
thomasluizon deleted the chore/orchestrate-ready-lifecycle branch August 8, 2026 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant