Correct workspace projection acceptance boundary (#187) - #197
Conversation
|
@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: 602b298c69
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 310f2a62eb
ℹ️ 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".
| ); | ||
| if (!REVISION_PATTERN.test(sourceRevision)) | ||
| throw new Error("acceptance-source-revision-invalid"); | ||
| const inspected = await inspectPackage(sourceRevision); |
There was a problem hiding this comment.
When this workspace-only runner is invoked against a current-head package that acquired an extra file after packaging, prepareWorkspacePackage still accepts it: unlike preparePackage, it does not rebuild via packageAcceptance, and inspectPackage reads only the manifest, foundation evidence, and executable without enumerating the actual package tree. The resulting evidence can therefore claim unexpectedFiles: 0 and packageTestHooks: 0 for a package that contains an unmanifested hook or other unexpected resource; rebuild the package or compare its complete tree with the manifest before launching it.
AGENTS.md reference: AGENTS.md:L162-L167
Useful? React with 👍 / 👎.
| Promise.all( | ||
| prepared.internal.workspaceRoots.map((root) => snapshotDirectory(root)), | ||
| ), |
There was a problem hiding this comment.
When the permission-denial scenario accidentally changes the denied fixture—for example, the packaged app chmods or writes it under the same user—the safeguard still reports unexpectedWorkspaceMutations: 0: the before/after snapshots cover only runtimeWorkRoot and the four successful workspaceRoots, while deniedWorkspaceRoot is omitted and later reset and deleted during cleanup. Capture mutation evidence for the denied fixture as well so the permission-denial checkpoint cannot conceal filesystem effects.
AGENTS.md reference: AGENTS.md:L144-L146
Useful? React with 👍 / 👎.
| const result = await waitForTracerAccessibilityAction({ | ||
| ...request, | ||
| binary: resources.adapter.binary, | ||
| pid: child.pid, | ||
| }); |
There was a problem hiding this comment.
When a paired adapter invocation performs its action but cannot observe the projection before returning missing-or-ambiguous-semantic-target, this call goes through waitForTracerAccessibilityAction, whose retry loop invokes the entire activating action again rather than retrying only observation. That can press canvas navigation, picker cancellation, workspace selection, or turn cancellation multiple times and then retain timing from a later attempt, even though the emitted safeguard unconditionally claims hiddenRetries: 0; execute each activating action once or distinguish post-action observation failures so they cannot re-enter the action.
AGENTS.md reference: AGENTS.md:L162-L165
Useful? React with 👍 / 👎.
| const sourceRevision = await run( | ||
| "git", | ||
| hardenedGitArguments(["rev-parse", "HEAD"]), | ||
| { timeoutMs: acceptanceSubprocessTimeouts.inspection }, |
There was a problem hiding this comment.
When tracked or untracked quality files are modified locally, this runner executes those uncommitted harness changes while recording only git rev-parse HEAD; unlike the native package gate, it never applies the repository's exact-head guard or checks git status before and after execution. A modified validator or adapter can therefore emit apparently exact-head workspace evidence for code that is not contained in the recorded revision. Require a clean exact-head guard for the entire run, including a final unchanged check before evidence is persisted.
AGENTS.md reference: AGENTS.md:L149-L153
Useful? React with 👍 / 👎.
|
Stopped and superseded by the accepted runtime-verification blocker #198; this PR is being closed unmerged and its branches are preserved. Exact stopped delivery evidence:
The four fresh Codex review findings remain unresolved and are retained for the future fresh #187 replay after #198 merges:
#198 v1 is the official blocker and is implementation-ready under #198 (comment) ( |
1 participant
Scope
v3codex/187-workspace-projection-recovery-602bepic/98-codex-tracer12ed4a0225fdf1fac2a75731fdbb25e949cf61349cc0a7ab7d8bad09c0eab7a3defectquality/.Issue #187 v3 readiness fingerprint:
c66599671f59b04b27bac84263e5219c99e4e8458084a7213c84f4831a20b062.Product and architecture alignment
CONTEXT.md, accepted ADRs, and the issue Quality Plan.No durable architecture change was required. The workspace runner is quality-only, reuses the accepted packaged-app and Accessibility primitives, measures successful projection after AX action return, retains native action separately, and never subtracts durations.
Acceptance criteria and evidence
Evidence identifies exact commands, artifacts, and platform results at the current head.
29/32/32/49ms; p9549ms (<=100). Native actions retained separately:28/20/20/10ms. Workspace evidence SHA-256978c7447f8350ba5f6050d323ff4a234503abe24bc24ff287624721fa1f86fbf.310f2a62eb220a22d7333717cb6bc9a9c4e18d9fkeiko-native-codex-tracer-workspace-acceptance/v1; no product hook or duration subtraction.310f2a62eb220a22d7333717cb6bc9a9c4e18d9f498ms (<=750). Malformed, stale, ambiguous, invalid-UTF-8, environment-drift, cleanup, package, and redaction states fail closed.310f2a62eb220a22d7333717cb6bc9a9c4e18d9f555561fde31abbf7b22eb88a70330e897cd3c7f217fe0a9a32ca23766aae82f3, and audit receipt SHA-25679ef54a467c93aa30873673e8bf3be3c436d76361f304965452a979b5588b26fcomplete. Full canonical intentionally not run because it remains #104-owned.310f2a62eb220a22d7333717cb6bc9a9c4e18d9fAcceptance journey evidence
Requirednpm run acceptance:macos;node quality/codex-tracer-workspace-acceptance.mjs; checkpointsapplication-launch,canvas-presentation310f2a62; package executable SHA-25624ef95c3…workspace-picker-open,workspace-picker-cancellation,workspace-permission-denial; 20 fresh cancellation launches, p95498ms29/32/32/49ms, p9549ms; native action separateapplication-quit; cleanup71ms; residue/mutations/redaction/repository bytes all0Quality Plan settlement
The runner reports no hidden retries, mock-only claims, package test hooks, raw path bytes, repository bytes, runtime repository context, redaction matches, residual processes, or unexpected workspace mutations.
Verification
npm ci --ignore-scriptsnpm run qualitynpm audit --audit-level=highAdditional affected checks and concise results:
One earlier
acceptance:macosinvocation failed before Cargo/test execution because its PATH omitted Cargo (ENOENT). It produced no success evidence and was classified as invocation-only. The accepted run used the full pinned Node 24/npm 11.16/Cargo/Homebrew/system PATH and passed. A separateverify-receipt.sh --helpprobe failed closed under system npm 11.12.1 because that script has no help mode; it wrote no receipt. The corrected pinned invocation completed green and wrote the SHA-bound receipt.The full
npm run acceptance:codex-tracer:macoscanonical was not rerun or claimed for #187 v3. The complete runtime/turn/cancellation journey remains owned by #104; #187 v3 authorizes the bounded workspace-only runner above.Independent audit and findings
310f2a62eb220a22d7333717cb6bc9a9c4e18d9f602b298; failure-first timeout testa2af734f; 15 s outer envelope only, with independent 5 s phase caps and<=100ms retained projection budget602b298; environment-drift testsa2af734f; normalized environment validated before first timing and after all measurements/cleanup602b298; hostile adapter testa2af734f; rejects before activation/actionf291318and310f2a6; one post-cleanup closed-enum stderr record, no raw error/path/PID/timingIntegrated epic acceptance
Required310f2a62.complete; strict schema/budget/p95/binding/package/redaction/safeguard validator failures[]; full local green bar and SHA-bound receipts complete.Delivery
child issue -> epic branchReady for Human Reviewbefore every required Acceptance Journey result and exact-head gate was complete.For a child-issue pull request targeting its designated epic branch:
This authority exists only for a fully eligible child-issue pull request targeting its exact accepted
epic/**branch. PR #197 remains draft while fresh remote checks and review settle; no merge is being attempted.PR #190 was closed unmerged because its delivery branch had an authenticated prohibited force update. Its branch remains available for transparent history. PR #197 is the sole active delivery PR. The recovery branch was created at unchanged signed
602b298…; this publication was one ordinary fast-forward push602b298…310f2a62. No force push, rewrite, gate bypass, or evidence substitution occurred on this branch.For an epic or standalone pull request targeting
dev, complete only by Niko or Oscharko. This child PR does not targetdev; the human-only subsection is not applicable.Residual risks and follow-ups
1128ms; the accepted independent 20-sample nearest-rank p95 is498ms. The complete ordered distribution is retained and validated rather than hiding the cold-start sample.310f2a62before delivery eligibility.Closes #187