Correct Code Review acceptance gates and token evidence checks

docs/reviews/2026-10-28-cr6-native-review.md

@@ -195,10 +195,17 @@ Code Review → paste the PR link Val named → **Review with Claude** → read
 
 **Closing checks:**
 
+Run from the same `apps/desktop` directory. The token check prints only a status: exit 0 means the token was absent from all required evidence, exit 1 means it was found, and exit 2 means the check was incomplete. Failed authentication, missing or unreadable evidence, and symlinks never count as a clean scan. Resolve an incomplete check before recording acceptance.
+
 ```sh
-grep -E -- '--method (POST|PUT|PATCH|DELETE)' "$SMOKE/gh-argv.log" && echo WRITE-FOUND || echo no-github-write
+if grep -qE -- '--method (POST|PUT|PATCH|DELETE)' "$SMOKE/gh-argv.log"; then
+  echo WRITE-FOUND
+else
+  scan_status=$?
+  if [ "$scan_status" -eq 1 ]; then echo no-github-write; else echo write-check-incomplete; fi
+fi
 gh api user --jq .login | diff - "$SMOKE/login-before.txt" && echo login-unchanged
-gh auth token | grep -rlF -f - "$SMOKE/home" "$SMOKE/claude-argv.log" "$SMOKE/electron-stderr.log" && echo TOKEN-FOUND || echo token-absent
+python3 ../../docs/reviews/code-review-token-check.py "$SMOKE/home" "$SMOKE/claude-argv.log" "$SMOKE/electron-stderr.log"
 ```
 
 Do not press **Add to review draft** → submit: findings reach GitHub only through CR-7's previewed draft, and

docs/reviews/2026-11-01-cr-gate-corrections.md

@@ -0,0 +1,9 @@
+# Code Review acceptance and canary corrections
+
+This candidate follows PR #472 at `245be00b2976e0c58b7fe036515feb2206a2160b` and preserves its full-suite failure, isolated retry, hosted pass and seed-launch evidence. It marks AC2 partial because attachment C58 and reasoning effort C59 lack an implementation or approved deviation, and includes those omissions in the enablement and decision gates. The existing current-run acceptance row now distinguishes the failed local E2E run from separate evidence.
+
+The CR-6 and CR-7 procedures previously piped `gh auth token` into `grep` and treated any nonzero result as absence. Synthetic probes reproduced a false clean result for failed authentication and missing evidence. Both procedures now invoke a shared status-only scanner: exit 0 means absent, 1 means found, and 2 means incomplete. It refuses failed authentication, empty or malformed token output, missing or unreadable paths, and symlinks. The CR-6 write-trace check also distinguishes no matching write from a failed scan.
+
+Run `python3 docs/reviews/code-review-token-check.test.py` from the repository root. All nine synthetic tests passed: clean evidence, a nested token match, failed authentication, empty token, missing evidence, symlink, unreadable file, missing gh and no paths. The fixture replaces PATH with a synthetic gh executable and asserts that no token text is printed. No real credential, provider call, live canary or GitHub write from the app was used.
+
+Build passed. Independent review found no blockers and confirmed status-only scanner output, failure handling, documented invocation paths, and the unresolved C58/C59 acceptance gates. Full unit validation passed: 2,277 passes, 26 skips, zero failures. The human and named-repository gates remain open.

docs/reviews/2026-11-01-cr7-live-canary.md

@@ -89,12 +89,14 @@ proved. A step that fails is recorded as failed with its screenshot; the canary
 
 ## Closing checks
 
+Run from the same `apps/desktop` directory. The token check prints only a status: exit 0 means the token was absent from all required evidence, exit 1 means it was found, and exit 2 means the check was incomplete. Failed authentication, missing or unreadable evidence, and symlinks never count as a clean scan. Resolve an incomplete check before recording acceptance.
+
 ```sh
 gh api user --jq .login | diff - "$CANARY/login-before.txt" && echo login-unchanged
 grep -E -- '--method (POST|PUT|PATCH|DELETE)' "$CANARY/gh-argv.log"   # main's argv form; expect 9 lines (8 if step 8 was skipped), each matching the table above
 grep -cE -- '--method DELETE' "$CANARY/gh-argv.log"                     # must be 0
-# The token must appear nowhere Modex wrote (prints file names only, never the token):
-gh auth token | grep -rlF -f - "$CANARY/home" "$CANARY/electron-stderr.log" "$CANARY/steps.md" && echo TOKEN-FOUND || echo token-absent
+# The token must appear nowhere Modex wrote (prints status only, never the token):
+python3 ../../docs/reviews/code-review-token-check.py "$CANARY/home" "$CANARY/electron-stderr.log" "$CANARY/steps.md"
 ```
 
 Then copy `$CANARY/steps.md`, the screenshots and the argv log (they contain only the disposable repository's

docs/reviews/2026-11-01-cr8-acceptance-matrix.md

@@ -143,7 +143,7 @@ Decisions 2–4 (C21, C31, C35, C38, C43's +) were settled by Val on 2026-10-25
 | #321 AC3 | Revoked, denied, malformed, timeout, rate limit, partial, omitted patch, no secret leakage | `U`; `F-prod` "incomplete data" (gh stderr sentinel never reaches the renderer or a reply) | `A-349` | — | met |
 | #321 AC4 | Head changes invalidate or label old diff/check data | `U`; `F-prod` "revision" (before the page: `head-changed`, files list never requested; during the page: `stale head-changed`) | — | — | met. The UI labelling landed with #324 (#404 `e3527f4`; `code-review-diff.spec.ts` "a new head cannot be overwritten by a delayed file page") |
 | #321 AC5 | Authenticated read-only smoke, kept separate | — | `A-321`, `A-349`, `A-main` | — | met, and re-run on main |
-| #321 AC6 | Existing Changes, connection and login tests green; no new permissions or writes | full suites (see Repository suites) | — | — | met at merge. Re-checked in this branch's full run |
+| #321 AC6 | Existing Changes, connection and login tests green; no new permissions or writes | full suites (see Repository suites) | — | — | met at merge. The current local full-suite re-check failed in E2E; the isolated retry and earlier hosted pass are recorded separately (`S-1101`) |
 | #322 AC1 | R1 and R4 through real controls; PR and tab identity correct | `F-nav`; `F-prod` journey (rail → row → split, with captures at the references' CSS sizes and geometry within 2 px of CR-3's table) | `A-322`, `A-main`, `A-main-2` | — | met for the host; R1 checks and R4 diff populated since #464 (`3d72c6e`). **Original-reference comparison is open** (see Visual comparison) |
 | #322 AC2 | Rapid switching shows no stale reply; back/forward don't replay | `F-nav` ×3; `F-prod` "identity" ×2, journey (no re-read) | `A-322` (back/forward) | — | met |
 | #322 AC3 | Scroll regions independent; resize and reopen keep controls in the window | `F-nav` keyboard test; `code-review-scroll.spec.ts` ×2 (#438 `afec85b`) | — | — | met at fixture: list, overview, diff and tree scroll alone and keep focus at 1440×900 and 900×600 (#438) |
@@ -181,7 +181,7 @@ Decisions 2–4 (C21, C31, C35, C38, C43's +) were settled by Val on 2026-10-25
 | AC | Where it stands on `0a8c513` |
 | --- | --- |
 | AC1 R1–R4 in real Electron | All four reached through real controls over production reads with synthetic `gh` (`F-1101` journey). **Open (Val):** comparison with the original references, which are gone; needs re-captured references or Val's decision to accept re-captures. |
-| AC2 every visible control has behaviour or an explicit unavailable state | Met at fixture: every control is built, a recorded deviation, or a named open decision (table above). Seven built controls have no asserting spec (follow-ups below). |
+| AC2 every visible control has behaviour or an explicit unavailable state | Partial: C58 attachment and C59 reasoning effort remain omitted without an approved deviation or numbered decision. The other controls are mapped above; seven built controls also lack an asserting spec (follow-ups below). |
 | AC3 isolation, injection, incomplete data, at-most-once | Met at fixture: read regressions, native denial on 2.1.316 (`N`), agent stop/unknown/restart, write at-most-once and reconciliation (`F-1101`). **Open:** the live smoke (#325) and live canary (#326). |
 | AC4 local Changes, navigation, conversation and approvals intact | `S-1101`: unit green; the local E2E had one non-Code-Review failure that passed alone, so it is not counted as green. The hosted required check passed on `c1f74c7`. |
 | AC5 performance, keyboard/contrast; human screen-reader gate | Baseline measurements and contrast stand for `0e674e7`; `F-1101` re-ran measure and a11y green. **Open (Val):** the [VoiceOver checklist](2026-10-22-cr8-voiceover/checklist.md), Result row empty. |
@@ -197,7 +197,7 @@ it, and main's channels answer regardless of whether the destination is visible.
 - **Stage 0, now:** hidden in packaged builds. Val (or a tester) turns it on per home from DevTools.
 - **Stage 1, after the live smoke and canary pass on repositories Val names:** still hidden by default; testers
   use it on their own repositories. Writes stay preview-bound and at-most-once.
-- **Stage 2, after VoiceOver and Val's call on the references and decisions 1, 5, 6, 7:** remove the switch (or
+- **Stage 2, after VoiceOver and Val's call on the references, decisions 1, 5, 6, 7, and the C58/C59 omissions:** remove the switch (or
   invert it) in a product PR that Val approves. That PR updates `docs/status.md` and this matrix.
 
 **Rollback:** setting the value to anything but `"on"` (or clearing site data for that home) hides the
@@ -220,7 +220,8 @@ the destination is visible. A product rollback (removing a capability) is a code
 4. **CR-7 live canary (#326 AC6):** name a disposable repository on #326 and say go;
    [procedure](2026-11-01-cr7-live-canary.md).
 5. **Decisions 1 (avatars C16/C81), 5 (Viewed C65), 6 (new tab C07), 7 (access mode C82).**
-6. **The `modex.codeReview` packaged-build switch:** when it goes (stages above).
+6. **C58 attachment and C59 reasoning effort:** decide whether to implement each omission or record an approved deviation; neither currently has a numbered decision.
+7. **The `modex.codeReview` packaged-build switch:** when it goes (stages above).
 
 ## Follow-ups found in this refresh (agent work, not gates)
 
@@ -230,7 +231,6 @@ the destination is visible. A product rollback (removing a capability) is a code
   controls are not in its required Tab set (C76).
 - No asserting spec for C50's text, C51's Copy comment link, C63's toggles, C66's menu items, C69's text, C71's
   horizontal scroll or C75's resize.
-- C58 (Attach) and C59's effort are left out with no numbered decision; Val may want them recorded as D-rows.
 - C61's clearance was planned for a floating composer; CR-6 built Ask inline and clearance was not re-measured.
 - CR-4's and CR-5's authenticated evidence is service-level on pre-merge candidates (`A-323`, `A-324`); an
   authenticated Electron read of the overview and diff on `main` needs Val's authorization (GET only).

docs/reviews/code-review-token-check.py

@@ -0,0 +1,43 @@
+import os
+from pathlib import Path
+import subprocess
+import sys
+
+
+def incomplete():
+    print("token-check-incomplete", file=sys.stderr)
+    raise SystemExit(2)
+
+
+try:
+    result = subprocess.run(["gh", "auth", "token", "--hostname", "github.com"], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, check=False)
+    token = result.stdout.strip()
+    if result.returncode or not token or b"\n" in token or b"\r" in token or not sys.argv[1:]:
+        incomplete()
+    files = []
+    for value in sys.argv[1:]:
+        root = Path(value)
+        if root.is_symlink() or not root.exists():
+            incomplete()
+        if root.is_dir():
+            def walk_error(error):
+                raise error
+            for directory, directories, names in os.walk(root, onerror=walk_error):
+                for name in directories + names:
+                    if (Path(directory) / name).is_symlink():
+                        incomplete()
+                files.extend(Path(directory) / name for name in names)
+        elif root.is_file():
+            files.append(root)
+        else:
+            incomplete()
+    found = False
+    for path in files:
+        if not path.is_file():
+            incomplete()
+        found = token in path.read_bytes() or found
+except OSError:
+    incomplete()
+
+print("TOKEN-FOUND" if found else "token-absent")
+raise SystemExit(1 if found else 0)

docs/reviews/code-review-token-check.test.py

@@ -0,0 +1,79 @@
+"""Synthetic-only regression checks; never invoke the signed-in gh executable."""
+import os
+from pathlib import Path
+import subprocess
+import sys
+import tempfile
+import unittest
+
+SCRIPT = Path(__file__).with_name("code-review-token-check.py")
+TOKEN = "synthetic-review-token-not-a-credential"
+
+
+class TokenCheck(unittest.TestCase):
+    def setUp(self):
+        self.temp = tempfile.TemporaryDirectory()
+        self.addCleanup(self.temp.cleanup)
+        self.root = Path(self.temp.name)
+        self.bin = self.root / "bin"
+        self.bin.mkdir()
+        self.gh = self.bin / "gh"
+        self.gh.write_text('#!/bin/sh\n[ "$1 $2 $3 $4" = "auth token --hostname github.com" ] || exit 9\n[ "$FAKE_AUTH_FAIL" = 1 ] && exit 1\nprintf "%s" "$FAKE_TOKEN"\n')
+        self.gh.chmod(0o700)
+        self.evidence = self.root / "evidence"
+        self.evidence.mkdir()
+        (self.evidence / "result.txt").write_text("clean fixture result")
+        self.env = {**os.environ, "PATH": str(self.bin), "FAKE_TOKEN": TOKEN, "FAKE_AUTH_FAIL": "0"}
+
+    def check(self, expected, *paths):
+        result = subprocess.run([sys.executable, str(SCRIPT), *map(str, paths)], env=self.env, capture_output=True, text=True)
+        self.assertEqual(result.returncode, expected, result.stderr)
+        self.assertNotIn(TOKEN, result.stdout + result.stderr)
+        if expected == 2:
+            self.assertEqual(result.stdout, "")
+            self.assertEqual(result.stderr.strip(), "token-check-incomplete")
+        else:
+            self.assertEqual(result.stdout.strip(), "TOKEN-FOUND" if expected else "token-absent")
+
+    def test_clean_complete_evidence(self):
+        self.check(0, self.evidence)
+
+    def test_token_in_nested_evidence(self):
+        nested = self.evidence / "nested"
+        nested.mkdir()
+        (nested / "log").write_text("prefix " + TOKEN + " suffix")
+        self.check(1, self.evidence)
+
+    def test_failed_authentication(self):
+        self.env["FAKE_AUTH_FAIL"] = "1"
+        self.check(2, self.evidence)
+
+    def test_empty_token(self):
+        self.env["FAKE_TOKEN"] = ""
+        self.check(2, self.evidence)
+
+    def test_missing_required_evidence(self):
+        self.check(2, self.evidence, self.root / "missing")
+
+    def test_symlink_is_not_clean_evidence(self):
+        (self.evidence / "link").symlink_to(self.evidence / "result.txt")
+        self.check(2, self.evidence)
+
+    def test_unreadable_file(self):
+        file = self.evidence / "result.txt"
+        file.chmod(0)
+        try:
+            self.check(2, self.evidence)
+        finally:
+            file.chmod(0o600)
+
+    def test_missing_gh(self):
+        self.gh.unlink()
+        self.check(2, self.evidence)
+
+    def test_no_required_paths(self):
+        self.check(2)
+
+
+if __name__ == "__main__":
+    unittest.main()