Fix Git evidence, updater lifecycle, and release validation

.claude-plugin/marketplace.json

@@ -8,7 +8,7 @@
       "name": "code-review-ad",
       "source": "./",
       "description": "code-review-ad",
-      "version": "1.0.6"
+      "version": "1.0.7"
     }
   ]
 }

.claude-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "code-review-ad",
-  "version": "1.0.6",
+  "version": "1.0.7",
   "description": "Independent Toolkit, Standards, Spec, and conditional Performance review.",
   "author": {
     "name": "Dmitry Kropachev"

.codex-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "code-review-ad",
-  "version": "1.0.6",
+  "version": "1.0.7",
   "description": "Independent Toolkit, Standards, Spec, and conditional Performance review.",
   "author": {
     "name": "Dmitry Kropachev"

PROVENANCE.json

@@ -40,6 +40,17 @@
         "tests/test_aspect_driver.py"
       ],
       "description": "Codex issue discovery from the first prompt and one role-specific deeper-look driver call in the same session, without intermediate observation or finding-collection payloads; read-only pins and parent verification retained."
+    },
+    {
+      "version": "1.0.7",
+      "paths": [
+        "common/codex/scripts",
+        "scripts/update.py",
+        "scripts/check.py",
+        "scripts/version.py",
+        "tests"
+      ],
+      "description": "Ten separate-agent correctness review passes: hardened frozen Git evidence, updater installation identity and lifecycle failure handling, and release validation; added focused regression coverage."
     }
   ]
 }

README.md

@@ -1,6 +1,6 @@
 # code-review-ad
 
-Independent MIT plugin for Claude Code and Codex. Version 1.0.6.
+Independent MIT plugin for Claude Code and Codex. Version 1.0.7.
 
 ## Install
 

common/codex/scripts/aspect-driver.py

@@ -19,7 +19,7 @@ def git(*args):
 
 
 def drive(args):
-    if not all(re.fullmatch(r"[a-f0-9]{40}", pin) for pin in (args.base, args.head)):
+    if not all(re.fullmatch(r"(?:[a-f0-9]{40}|[a-f0-9]{64})", pin) for pin in (args.base, args.head)):
         raise ValueError("full base/head OIDs are required")
     if git("rev-parse", "HEAD") != args.head:
         raise ValueError("checkout HEAD differs from the pinned head")

common/codex/scripts/prepare-review.js

@@ -54,26 +54,33 @@ function prepareReview({ repo = '.', base, head }) {
     }
   }
   repo = path.resolve(repo)
+  // Git's tree/pathspec commands otherwise interpret root-relative inventory
+  // paths from the caller's subdirectory and silently miss their blob entries.
+  // Remove only Git's terminator: a repository directory may end in whitespace.
+  repo = git(repo, 'rev-parse', '--show-toplevel').replace(/\n$/, '')
   base = git(repo, 'rev-parse', '--verify', `${base}^{commit}`).trim()
   head = git(repo, 'rev-parse', '--verify', `${head}^{commit}`).trim()
   if (git(repo, 'rev-parse', 'HEAD').trim() !== head) throw new Error('checkout HEAD does not match the frozen head')
   const mergeBase = git(repo, 'merge-base', base, head).trim()
   const range = `${mergeBase}...${head}`
-  const files = changedFiles(git(repo, 'diff', '--no-ext-diff', '--no-textconv', '--find-renames', '--name-status', '-z', range))
+  const files = changedFiles(git(repo, 'diff', '--no-ext-diff', '--no-textconv', '--ignore-submodules=none', '--find-renames', '--name-status', '-z', range))
   const comments = [], nearby = [], risks = []
   for (const file of files) {
     if (file.status === 'D') continue
+    // Gitlinks refer to commits in a different object database. Inspect the
+    // tree entry rather than dereferencing their possibly unavailable objects.
+    const entry = git(repo, '--literal-pathspecs', 'ls-tree', '-z', head, '--', file.path)
+    if (!/^[0-7]+ blob /.test(entry)) continue
     // The old pathname may now be a directory containing unrelated new files.
     // Compare renamed blobs directly so its descendants cannot enter this diff.
     const scope = file.oldPath
       ? [`${mergeBase}:${file.oldPath}`, `${head}:${file.path}`]
       : [range, '--', file.path]
     const diff = git(repo, '--literal-pathspecs', 'diff', '--no-ext-diff', '--no-textconv', '--no-color',
-      '--find-renames', '--unified=0', ...scope)
+      '--ignore-submodules=none', '--find-renames', '--unified=0', ...scope)
     const added = addedLines(diff)
-    if (!added.length) continue // Binary files, submodules and pure renames stay in the file list.
+    if (!added.length) continue // Binary files and pure renames stay in the file list.
     const object = `${head}:${file.path}`
-    if (git(repo, 'cat-file', '-t', object).trim() !== 'blob') continue
     const lines = git(repo, 'cat-file', 'blob', object).split('\n')
     const addedNumbers = new Set(added.map((row) => row.line))
     const nearbyNumbers = new Set()
@@ -101,8 +108,8 @@ function prepareReview({ repo = '.', base, head }) {
   const commits = git(repo, 'rev-list', '--reverse', `${mergeBase}..${head}`).trim().split('\n').filter(Boolean).map((oid) => {
     const parent = git(repo, 'show', '-s', '--format=%P', oid).trim().split(' ')[0] || null
     const changed = parent
-      ? git(repo, 'diff', '--no-ext-diff', '--no-textconv', '--name-only', '-z', parent, oid)
-      : git(repo, 'diff-tree', '--root', '--no-commit-id', '--name-only', '-r', '-z', oid)
+      ? git(repo, 'diff', '--no-ext-diff', '--no-textconv', '--ignore-submodules=none', '--name-only', '-z', parent, oid)
+      : git(repo, 'diff-tree', '--ignore-submodules=none', '--root', '--no-commit-id', '--name-only', '-r', '-z', oid)
     return {
       oid, filesComparedWith: parent,
       message: git(repo, 'show', '-s', '--format=%B', oid).trimEnd(),
@@ -112,7 +119,7 @@ function prepareReview({ repo = '.', base, head }) {
   return {
     schemaVersion: 1, base, head, mergeBase,
     workingTreeStatus: git(repo, 'status', '--porcelain=v1'),
-    diffCommand: `git diff --no-ext-diff --no-textconv --find-renames ${range}`,
+    diffCommand: `git diff --no-ext-diff --no-textconv --ignore-submodules=none --find-renames ${range}`,
     files, commits, comments, nearby, risks,
   }
 }

scripts/check.py

@@ -5,7 +5,7 @@
 ROOT=Path(__file__).resolve().parent.parent
 
 def version(v):
-    if not re.fullmatch(r"[0-9]+\.[0-9]+\.[0-9]+",v):raise ValueError('Stable semantic version required')
+    if not re.fullmatch(r"(?:0|[1-9][0-9]*)\.(?:0|[1-9][0-9]*)\.(?:0|[1-9][0-9]*)",v):raise ValueError('Stable semantic version required')
     return tuple(map(int,v.split('.')))
 
 def check(base=None):
@@ -28,6 +28,8 @@ def check(base=None):
     for host,filename in [('codex','.agents/plugins/marketplace.json'),('claude','.claude-plugin/marketplace.json')]:
         catalog=json.loads((ROOT/filename).read_text());assert catalog['name']==manifests[0]['name']
         assert [p['name'] for p in catalog['plugins']]==[manifests[0]['name']]
+        if host=='claude':
+            assert catalog['plugins'][0]['version']==manifests[0]['version'],'Claude marketplace version must match host manifests'
     for p in ROOT.rglob('*'):
         if '.git' in p.parts or '__pycache__' in p.parts or '.pytest_cache' in p.parts:continue
         if p.is_symlink():assert p.resolve().is_relative_to(ROOT),str(p)

scripts/update.py

@@ -2,6 +2,7 @@
 """Self-contained plugin lifecycle and release policy (Python standard library)."""
 import argparse
 from contextlib import contextmanager
+import errno
 import json
 import os
 from pathlib import Path
@@ -39,8 +40,18 @@ def locked(path):
     with path.open('a+b') as stream:
         if os.name == 'nt':
             import msvcrt
-            stream.write(b'0'); stream.flush(); stream.seek(0)
-            msvcrt.locking(stream.fileno(), msvcrt.LK_LOCK, 1)
+            stream.seek(0, 2)
+            if stream.tell() == 0:
+                stream.write(b'0'); stream.flush()
+            stream.seek(0)
+            while True:
+                try:
+                    msvcrt.locking(stream.fileno(), msvcrt.LK_NBLCK, 1)
+                    break
+                except OSError as exc:
+                    if exc.errno not in (errno.EACCES, errno.EAGAIN, errno.EDEADLK):
+                        raise
+                    time.sleep(0.05)
         else:
             import fcntl
             fcntl.flock(stream, fcntl.LOCK_EX)
@@ -54,23 +65,34 @@ def locked(path):
 
 
 def version(value):
-    match = re.fullmatch(r'v?(\d+)\.(\d+)\.(\d+)', value or '')
+    match = re.fullmatch(r'v?(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)', value or '')
     return tuple(map(int, match.groups())) if match else None
 
 
-def run(argv):
-    proc = subprocess.run(argv, text=True, capture_output=True, timeout=90)
+def run(argv, *, cwd=None):
+    proc = subprocess.run(argv, text=True, capture_output=True, timeout=90, cwd=cwd)
     if proc.returncode:
         raise RuntimeError(proc.stderr.strip() or proc.stdout.strip() or 'Host command failed')
     return proc.stdout
 
 
+def project_path(record):
+    if record.get('scope') not in ('project', 'local'):
+        return None
+    value = record.get('projectPath')
+    if isinstance(value, str) and Path(value).is_absolute():
+        return str(Path(value).resolve())
+    return None
+
+
 def installed(root, host):
+    root = root.resolve()
     manifest = read(root / f'.{host}-plugin/plugin.json')
     name = manifest['name']
     marketplace = None
     pinned = False
     scope = 'user'
+    project = None
     records = []
     if host == 'claude':
         home = Path(os.environ.get('CLAUDE_CONFIG_DIR', Path.home() / '.claude'))
@@ -81,7 +103,23 @@ def installed(root, host):
         known = read(home / 'plugins/known_marketplaces.json')
     else:
         data = json.loads(run(['codex', 'plugin', 'list', '--json']))
-        records = [(r['pluginId'], r) for r in data.get('installed', []) if r.get('name') == name]
+        home = Path(os.environ.get('CODEX_HOME', Path.home() / '.codex'))
+        for record in data.get('installed', []):
+            if record.get('name') != name or record.get('installed') is False:
+                continue
+            identity = record.get('pluginId', '')
+            plugin_name, separator, catalog_name = identity.partition('@')
+            if not separator or plugin_name != name or not catalog_name:
+                continue
+            # Listing a plugin with the same name does not register this script's
+            # checkout. Current Codex lists omit installPath, so also recognize
+            # the host's versioned cache directory (including an older loaded
+            # version after the registry has advanced).
+            install_path = record.get('installPath')
+            registered_root = (Path(install_path) if install_path else
+                               home / 'plugins/cache' / catalog_name / name / manifest['version'])
+            if registered_root.resolve() == root:
+                records.append((identity, record))
         known = {}
     if len(records) > 1:
         raise ValueError('Ambiguous installed plugin registry identity; no automatic update')
@@ -90,13 +128,17 @@ def installed(root, host):
         manifest['version'] = record.get('version', manifest['version'])
         marketplace = identity.split('@', 1)[1]
         scope = record.get('scope', 'user')
+        if host == 'claude':
+            project = project_path(record)
+            pinned |= scope in ('project', 'local') and project is None
         source = record.get('source') or known.get(marketplace, {}).get('source', {})
         if isinstance(source, str):
             source = {'source': source}
         marketplace_source = record.get('marketplaceSource') or {}
         remote_marketplace = marketplace_source.get('sourceType') == 'git'
-        pinned = bool(record.get('pinned') or source.get('sha') or source.get('ref') not in (None, 'main')
+        pinned |= bool(record.get('pinned') or source.get('sha') or source.get('ref') not in (None, 'main')
                       or marketplace_source.get('sha') or marketplace_source.get('ref') not in (None, 'main')
+                      or marketplace_source.get('sourceType') in ('local', 'directory', 'file')
                       or (source.get('source') == 'local' and not remote_marketplace)
                       or source.get('sourceType') in ('local', 'directory')
                       or source.get('source') in ('directory', 'file'))
@@ -113,7 +155,7 @@ def installed(root, host):
         pinned = True  # Direct checkout, plugin-dir, or unregistered installation.
     return {'name': name, 'host': host, 'marketplace': marketplace, 'version': manifest['version'],
             'repository': 'dk-code-factory/' + name, 'pinned': pinned, 'scope': scope,
-            'root': str(root)}
+            'root': str(root), 'project_path': project}
 
 
 def latest(repository):
@@ -150,9 +192,16 @@ def mode(settings, name):
     return value
 
 
+def installation_key(identity):
+    return json.dumps([identity[key] for key in ('host', 'name', 'marketplace', 'scope', 'root')]
+                      + [identity.get('project_path')])
+
+
 def update(identity, target):
     name, marketplace, host = (identity[k] for k in ('name', 'marketplace', 'host'))
-    if identity['pinned'] or not marketplace:
+    if (identity['pinned'] or not marketplace
+            or host == 'claude' and identity['scope'] in ('project', 'local')
+            and not identity.get('project_path')):
         return {'status': 'pinned', 'message': 'Pinned or unregistered installation; update manually.'}
     if version(identity['version']) and version(identity['version']) >= version(target):
         return {'status': 'current', 'version': identity['version'], 'reload_required': True,
@@ -165,14 +214,17 @@ def update(identity, target):
                     ['codex', 'plugin', 'add', name + '@' + marketplace, '--json']]
     try:
         for command in commands:
-            run(command)
+            run(command, cwd=identity.get('project_path') if host == 'claude' else None)
         if host == 'codex':
             rows = json.loads(run(['codex', 'plugin', 'list', '--json'])).get('installed', [])
-            versions = [r['version'] for r in rows if r['pluginId'] == name + '@' + marketplace]
+            versions = [r['version'] for r in rows if r['pluginId'] == name + '@' + marketplace
+                        and r.get('installed') is not False
+                        and r.get('scope', 'user') == identity['scope']]
         else:
             home = Path(os.environ.get('CLAUDE_CONFIG_DIR', Path.home() / '.claude'))
             rows = read(home / 'plugins/installed_plugins.json').get('plugins', {}).get(name + '@' + marketplace, [])
-            versions = [r['version'] for r in rows if r.get('scope') == identity['scope']]
+            versions = [r['version'] for r in rows if r.get('scope') == identity['scope']
+                        and project_path(r) == identity.get('project_path')]
         if len(versions) != 1 or not version(versions[0]) or version(versions[0]) < version(target):
             raise RuntimeError('Host did not confirm installation of the released version')
         return {'status': 'updated', 'version': versions[0], 'reload_required': True,
@@ -184,12 +236,9 @@ def update(identity, target):
 def lifecycle(root, host, phase, *, token=None, parent=None, now=None):
     now = time.time() if now is None else now
     config_path, state = paths(root, Path.cwd())
-    settings = read(config_path)
     with locked(state / '.lock'):
         data = read(state / 'state.json', {'checks': {}, 'notices': [], 'invocations': {}, 'sessions': {}})
         if phase == 'entry':
-            identity = installed(root, host)
-            policy = mode(settings, identity['name'])
             token = uuid.uuid4().hex
             if parent:
                 if parent not in data['invocations'] or data['invocations'][parent]['complete']:
@@ -201,11 +250,27 @@ def lifecycle(root, host, phase, *, token=None, parent=None, now=None):
             session = data['sessions'][session_id]
             session['active'].append(token)
             data['invocations'][token] = {'session': session_id, 'complete': False}
+            # Host/configuration failures must not hide a live workflow from
+            # other invocations that are waiting to install an update.
+            try:
+                identity = installed(root, host)
+                if not version(identity['version']):
+                    raise ValueError('Installed plugin version is not stable semantic versioning')
+                policy = mode(read(config_path), identity['name'])
+            except Exception as exc:
+                write(state / 'state.json', data)
+                return {'status': 'unavailable', 'action': 'continue', 'message': str(exc),
+                        'non_blocking': True, 'token': token}
             result = {'status': 'current', 'action': 'continue', 'message': '', 'token': token,
                       'identity': identity, 'mode': policy}
             if policy != 'off':
                 cache = data['checks'].get(identity['repository'], {})
-                if now - cache.get('checked', -DAY) >= DAY:
+                checked = cache.get('checked') if isinstance(cache, dict) else None
+                valid_cache = (isinstance(checked, (int, float)) and not isinstance(checked, bool)
+                               and 0 <= now - checked < DAY
+                               and ('error' in cache or isinstance(cache.get('version'), str)
+                                    and version(cache['version'])))
+                if not valid_cache:
                     try:
                         cache = {'checked': now, 'version': latest(identity['repository'])}
                     except Exception as exc:
@@ -220,7 +285,7 @@ def lifecycle(root, host, phase, *, token=None, parent=None, now=None):
                         status, message = 'pinned', f'{identity["name"]} {target} is available; this installation is pinned or unregistered.'
                     elif policy == 'auto':
                         status, message = 'deferred', f'{identity["name"]} {target} will update after the outer workflow completes.'
-                        session['pending'][host + ':' + identity['name']] = {'identity': identity, 'target': target}
+                        session['pending'][installation_key(identity)] = {'identity': identity, 'target': target}
                     else:
                         status, message = 'available', f'{identity["name"]} {target} is available (installed {identity["version"]}).'
                     result['status'] = status
@@ -243,15 +308,18 @@ def lifecycle(root, host, phase, *, token=None, parent=None, now=None):
                 # pending work only when all recorded invocations have finished.
                 pending_updates = {}
                 for row in data['sessions'].values():
-                    pending_updates.update(row['pending'])
+                    # Derive keys from saved identities so older host:name keys
+                    # remain readable without merging distinct installations.
+                    for pending in row['pending'].values():
+                        pending_updates[installation_key(pending['identity'])] = pending
                 for pending in pending_updates.values():
                     identity = pending['identity']
                     # Recheck policy and pins at completion; changing settings must cancel a pending auto-update.
-                    if mode(settings, identity['name']) != 'auto':
-                        continue
                     try:
+                        if mode(read(config_path), identity['name']) != 'auto':
+                            continue
                         current = installed(Path(identity['root']), identity['host'])
-                        if current['marketplace'] != identity['marketplace']:
+                        if installation_key(current) != installation_key(identity):
                             raise ValueError('Installation identity changed during workflow')
                         outcome = update(current, pending['target'])
                     except Exception as exc: