=== ID 3431396021 | .claude/settings.json:8 | user=gautam-chatgpt[bot] | reply_to=null
[blocking] Please do not commit a repo-level Claude `PreToolUse` Bash hook that runs `rtk hook claude` for every shell command. This is arbitrary code execution from repository config and will also fail or alter behavior for reviewers/operators who do not have `rtk` installed. Keep personal Claude hooks out of the repo, or document an opt-in setup outside tracked settings.

=== ID 3431396028 | tools/ringcentral-admin/server.py:573 | user=gautam-chatgpt[bot] | reply_to=null
[blocking] `update_call_queue_members` performs a RingCentral write when `dry_run=false` and `EXULT_RC_ALLOW_WRITES=1`, but it does not require exact Gautam approval text for this specific queue/member change. The repo convention requires exact wording and exact destination confirmed in the current conversation before any RC PUT/POST/DELETE. Add an `approval_text` gate that includes queue id plus add/remove ids, similar to the stricter `create_ai_scheduling_extension` flow.

=== ID 3431396034 | tools/ringcentral-admin/server.py:631 | user=gautam-chatgpt[bot] | reply_to=null
[blocking] `set_call_queue_ring_type` can PUT queue answering-rule changes without a per-call exact approval string. This can alter live call routing, so `EXULT_RC_ALLOW_WRITES` plus `dry_run=false` is not sufficient under the repository's RC write policy.

=== ID 3431396038 | tools/ringcentral-admin/server.py:708 | user=gautam-chatgpt[bot] | reply_to=null
[blocking] `set_call_queue_member_order` can PUT fixed-order agents without exact current-conversation approval. This changes queue routing behavior and needs the same exact approval gate with queue id, rule id, and ordered extension ids before allowing the write.

=== ID 3431397856 | tools/curogram-mcp/curogram-auth.ts:237 | user=gautam-claude[bot] | reply_to=null
[blocking] **Infinite relogin loop on persistent 401.** `fetch()` calls `relogin()` on 401, which clears the session and calls `ensureSession()` → `login()`. If login succeeds but the new session is immediately 401'd again (revoked creds, IP block, account disabled), this loops forever. Add a relogin attempt counter or a cooldown:

```ts
private reloginAttempts = 0;
private static MAX_RELOGIN = 3;

async fetch(url: string, init: RequestInit = {}): Promise<Response> {
  // ...
  if (res.status === 401) {
    if (this.reloginAttempts >= CurogramAuth.MAX_RELOGIN) {
      throw new Error('curogram-auth: max relogin attempts exceeded');
    }
    this.reloginAttempts++;
    const fresh = await this.relogin();
    res = await doFetch(fresh);
  }
  this.reloginAttempts = 0; // reset on success
  return res;
}
```

=== ID 3431397859 | supervisors/run-curogram-mcp.sh:34 | user=gautam-claude[bot] | reply_to=null
[blocking] **Race condition in PID-file lock.** The header comment says "Locked with flock so multiple invocations are safe" but the actual implementation is a check-then-write PID file with a TOCTOU window. Two processes can both pass the `kill -0` check and both write their PID. Use actual `flock`:

```bash
exec 9>"$LOCK_FILE"
if ! flock -n 9; then
  echo "[$(date)] another instance holds the lock. Exiting." >&2
  exit 0
fi
```

This also eliminates the need for the `cleanup` trap to remove the lock file (flock is released on process exit).

=== ID 3431397862 | tools/curogram-mcp/curogram-auth.ts:79 | user=gautam-claude[bot] | reply_to=null
[suggestion] **No max-age / expiry tracking on CDP-harvested cookies.** When using the CDP fallback (`opts.cookie` + `opts.xsrf`), the session is accepted unconditionally and never validated. If those cookies are already expired, every request will 401 → relogin → same stale cookies → loop (compounded by the relogin issue above). Consider at minimum a staleness check or a flag that skips relogin when using static CDP cookies.

=== ID 3431397864 | tools/curogram-mcp/server.ts:191 | user=gautam-claude[bot] | reply_to=null
[suggestion] **No input sanitization on `conversation_id` beyond `encodeURIComponent`.** The value is interpolated into a URL path. While `encodeURIComponent` prevents path traversal, Curogram ObjectIds should be alphanumeric hex strings — a regex check (`/^[a-f0-9]{24}$/`) would reject garbage early and provide a better error message. Same applies to `patient_id` and `message_id`.

=== ID 3431397869 | supervisors/run-curogram-mcp.sh:73 | user=gautam-claude[bot] | reply_to=null
[suggestion] **Exported env vars may be unset.** Lines 73-74 do `export CUROGRAM_PRACTICE_ID CUROGRAM_COOKIE CUROGRAM_XSRF_TOKEN` unconditionally, but these are documented as optional. If they're not set in the env file, this exports empty strings into the child process, which could shadow intentional absence checks (e.g., `process.env.CUROGRAM_COOKIE` will be `""` instead of `undefined`). Use `export ${VAR:+CUROGRAM_PRACTICE_ID}` or only export if set.

=== ID 3431397878 | supervisors/run-curogram-mcp.sh:24 | user=gautam-claude[bot] | reply_to=null
[suggestion] **Hardcoded `SERVER_DIR` path.** The path `$HOME/repos/exult-agent/tools/curogram-mcp` assumes a specific repo checkout location. Other supervisors in this repo may use the same pattern, but it's fragile — consider deriving it relative to the script's own location (`$(dirname "$0")/../tools/curogram-mcp`) or from an env var.

=== ID 3431397883 | tools/curogram-mcp/server.ts:293 | user=gautam-claude[bot] | reply_to=null
[suggestion] **`handleSendText` leaks message length in dry-run output.** The dry-run response includes `message_length`. While not PHI, it could be combined with other metadata to narrow down message content. Consider whether this is needed — the caller already has the message text.

=== ID 3431397886 | tools/ringcentral-admin/server.py:110 | user=gautam-claude[bot] | reply_to=null
[suggestion] **Token cache now stores the full OAuth response body.** `_token_cache.update(body)` merges the entire token response including `access_token` and `refresh_token` into the cache dict. While `_get_auth_context` filters to safe keys, any future code that logs or serializes `_token_cache` will leak secrets. Consider explicitly picking only the keys you need:

```python
for key in ['access_token', 'token_type', 'expires_in', 'scope', 'owner_id', 'endpoint_id', 'expireTime']:
    if key in body:
        _token_cache[key] = body[key]
```

=== ID 3431397895 | tools/ringcentral-admin/server.py:843 | user=gautam-claude[bot] | reply_to=null
[suggestion] **`create_ai_scheduling_extension` uses string equality on approval_text but the expected text is reconstructable.** An attacker who can read the source can reconstruct the approval text. This is defense-in-depth (the write gate is the real guard), but the approval text provides no additional security beyond what `EXULT_RC_ALLOW_WRITES=1` already provides. If the intent is human-in-the-loop confirmation, consider a nonce or timestamp-based challenge instead.

=== ID 3431397897 | tools/ringcentral-admin/server.py:573 | user=gautam-claude[bot] | reply_to=null
[suggestion] **No `_validate_id` call on `queue_id` in several new write tools before string interpolation into API paths.** `update_call_queue_members` does validate, but make sure `_validate_id` is consistent. I see it is used — good. Just confirming the pattern holds across all new tools.

=== ID 3431397903 | tools/curogram-mcp/server.ts:459 | user=gautam-claude[bot] | reply_to=null
[nitpick] The `serveMcpOverHttp` call is wrapped in try/catch, but the server startup log on line 463 runs regardless of whether `serveMcpOverHttp` actually binds the port (if it's async and deferred). Verify that `serveMcpOverHttp` is synchronous or throws synchronously on port conflicts.

=== ID 3431397908 | tools/teams-channel/server.ts:2160 | user=gautam-claude[bot] | reply_to=null
[nitpick] The `HTTP_ONLY_MODE` guard is clean but produces 4 separate log lines when enabled. Consider consolidating to a single startup banner: `teams-channel: HTTP_ONLY mode — stdio, webhook, RC notifications, and polling disabled`.

=== ID 3431398573 | supervisors/run-curogram-mcp.sh:5 | user=gautam-gemini[bot] | reply_to=null
[nitpick] The comment mentions 'Locked with flock' but the script actually implements manual PID-file based locking. Consider updating the comment to accurately reflect the implementation.