Skip to content

test(harness): make permission-mode model evidence-based - #2542

Merged
Chris0Jeky merged 3 commits into
mainfrom
issue-2537/permission-mode-positive-control
Sep 4, 2026
Merged

test(harness): make permission-mode model evidence-based#2542
Chris0Jeky merged 3 commits into
mainfrom
issue-2537/permission-mode-positive-control

Conversation

@Chris0Jeky

Copy link
Copy Markdown
Owner

Summary

  • removes the unverified claim that auto is a protected project/local permission mode
  • cites the measured settings: stop pinning a project defaultMode so the user's mode wins #2395 boundary and the harness-refresh record's built-in-default note in the model comment
  • adds a positive project-scope acceptEdits control while preserving the bypass, allow-rule, trust, and command-line precedence checks

Closes #2537

Verification

  • PowerShell AST parse — passed
  • focused headless-permission-contract case — 1/1
  • full powershell -NoProfile -ExecutionPolicy Bypass -File scripts/git/Test-New-CodexIssueWorktree.ps1 — 28/28
  • git diff --check — passed

Boundaries

Test-model and regression coverage only. No project settings, runtime permissions, or production behavior changed.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

The connector usage-limit notice is informational, not a review finding. This PR is being checked through the independent coordinator review path; no code action is requested from this notice.

@Chris0Jeky Chris0Jeky left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Independent coordinator review of exact head 5998724: no CRITICAL/HIGH correctness, security, or data-loss finding. The change removes an unsupported auto assumption, anchors the remaining protected-mode model to #2395 and the repo refresh record, and adds a discriminating project acceptEdits control. AST, focused, full 28/28 self-test, and diff checks are green. Hosted checks remain the required gate.

@Chris0Jeky

Copy link
Copy Markdown
Owner Author

Merging — and the red on the earlier head was never this PR

Review: fresh-context, verdict SHIP. One MEDIUM and three INFOs, all tracked rather than blocking. This PR is T1, not control-plane: scripts/git/** matches no entry in ci/policy.v1.json's 36 controlPaths (that list has scripts/ci/**, scripts/deploy/**, scripts/security/** and scripts/build-release.{ps1,sh}, but nothing for scripts/git/), so it is agent-mergeable rather than SC-10 material.

On the earlier red, recorded because it looked alarming and was not. The failing check was Frontend Unit / Frontend Unit (windows-latest) on a PR that changes one PowerShell file. The job never reached vitest: it died at step 4, Run source launcher regression suite (node --test … scripts/ci/dev-up.test.mjs), which runs before npm ci; steps 5–11, including step 10 Run frontend tests with coverage thresholds, are all skipped. So the original "frontend test failure" framing was wrong — no frontend test ran at all.

The actual failure was PowerShell: run identity changes after final state write at dev-up.test.mjs:1566, fail 1 of 85, on the third assertion (assertNoStateAndPortsReleased) — the launcher correctly detected the identity flip and correctly reported failure, and only the teardown was judged incomplete. Mechanism: Stop-LoadedStack retains PID state whenever cleanup cannot be proved, and its pre-taskkill identity probe (scripts/dev-up.ps1:401-406) returns false for Unknown as well as MismatchGet-ProcessCreationToken swallows any exception from $Process.StartTime and a live Windows process can transiently refuse it. The code already tolerates exactly this at :414-415, but only in the post-taskkill loop. Timing corroborates: the failing run took 2991 ms while a local pass of the same test takes 3482 ms, which rules out every bounded wait (10 s and 5 s).

This PR cannot reach that code. It changes only scripts/git/Test-New-CodexIssueWorktree.ps1, which CI executes in exactly one place — .github/workflows/reusable-docs-governance.yml:25, a different job, which passed.

Deliberately not filed as "known flake": it is distinct from the #2378/#2161 timeout cohort (those are ETIMEDOUT/cancelled shapes; this is a 2991 ms assertion), and 9 consecutive local reruns of the same test all passed. It is n=1 with an identified mechanism, now tracked as #2561. If that same test fails a second time it should be treated as a regression, not re-run.

Gate at the current head d78c31378: 23 pass / 11 skipping / 0 non-pass-non-skipping after gh pr update-branch. closingIssuesReferences is [2537], so the issue link is live.

Tracked, not fixed here: the MEDIUM is that .claude/README.md still asserts project-scope auto is ignored, which this PR's model change now contradicts — worth reconciling, but a doc statement elsewhere is not a merge blocker for a test-harness change. The reviewer also notes #2537's first MEDIUM (the model asserting its own filter) is untouched, so #2537 should not be treated as fully closed by this alone despite the Closes link.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

1 participant