fix: address CodeRabbit review comments — add grep patterns, fix docs, improve evals

- operator-review: replace fragile grep with rg -U for multi-line O1 check;
  add concrete grep/rg detection patterns for O2 and O6
- security-review: add executable bash detection patterns for S1–S6 so
  automated agents can produce pass/fail signals
- cypress-demo: fix incomplete curl URL (add :3000 port); clarify helpers
  must be embedded in each demo file (not in sessions.cy.ts); qualify
  'complete working example' claim about sessions.cy.ts
- pr-fixer: document that gh pr view returns uppercase state (OPEN/CLOSED/MERGED)
  and that non-existent PRs produce non-zero exit + stderr (not JSON)
- pr-fixer/evals: add URL-form and punctuation-trailing PR number fixtures
- scaffold/evals: include integration target in expected_args for natural-
  language PagerDuty case so routing failures don't silently pass
- unleash-flag: replace unsafe inline token export with non-echoing read
  prompt guidance; add note to log len(token) not the value itself
- jira.log.md: add issue_type normalization note (Story|Bug|Task allowlist,
  normalize case, reject unrecognized values)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: Ambient Code Bot

.claude/agents/operator-review.md

@@ -28,17 +28,29 @@ Load these files before running checks:
 ### O1: OwnerReferences on child resources (Blocker)
 
 ```bash
-grep -rn "Jobs\?\|Secrets\?\|PersistentVolumeClaims\?" components/operator/ --include="*.go" | grep -i "create"
+# Find client.Create calls (handles multi-line builder chains)
+rg -U "\.Create\s*\(" components/operator/ --glob="*.go" -l
+# Then for each file, check whether OwnerReferences is set before Create
+rg "OwnerReferences|controllerutil\.SetControllerReference" components/operator/ --glob="*.go"
 ```
 
-Cross-reference each create call with `OwnerReferences` in the same function. See `DEVELOPMENT.md` for the required pattern.
+For each `Create(` call in `components/operator/`, verify the created object's `metadata.OwnerReferences` is populated (or `controllerutil.SetControllerReference` is called) in the same function. See `DEVELOPMENT.md` for the required pattern.
 
 ### O2: Proper reconciliation patterns (Critical)
 
 - `errors.IsNotFound` → return nil (resource deleted, don't retry)
 - Transient errors → return error (triggers requeue with backoff)
 - Terminal errors → update CR status to "Failed", return nil
 
+```bash
+# Find IsNotFound usages — verify they return nil, not an error
+rg -n "errors\.IsNotFound" components/operator/ --glob="*.go" -A 2
+# Find status updates that set Failed but still return errors (bad pattern: should return nil)
+rg -U "status.*Failed|Failed.*status" components/operator/ --glob="*.go" -A 3
+# Find Reconcile/reconcile functions for manual review of error return paths
+rg -n "^func.*[Rr]econcile" components/operator/ --glob="*.go"
+```
+
 ### O3: SecurityContext on Job pod specs (Critical)
 
 ```bash
@@ -65,6 +77,15 @@ grep -rn "panic(" components/operator/ --include="*.go" | grep -v "_test.go"
 
 Error paths must update the CR status to reflect the error.
 
+```bash
+# Find status update calls
+rg -n "status\.Update|Status\.Conditions|SetCondition|UpdateStatus" components/operator/ --glob="*.go" | grep -v "_test.go"
+# Find error returns in Reconcile functions without preceding status update (flag for manual review)
+rg -n "return.*ctrl\.Result|return.*err" components/operator/ --glob="*.go" | grep -v "_test.go"
+```
+
+For each error return path in `Reconcile`, verify a status update (setting condition to `Failed` or similar) occurs before returning.
+
 ### O7: No context.TODO() (Minor)
 
 ```bash

.claude/agents/security-review.md

@@ -27,26 +27,70 @@ Load these files before running checks:
 
 Handlers must use `GetK8sClientsForRequest(c)` for user-initiated operations. Service account only for privileged operations after RBAC validation.
 
+```bash
+# Find handler functions using service account clients without RBAC validation
+rg -n "GetK8sClientsForRequest|serviceAccountClient|saClient" components/ --glob="*.go" | grep -v "_test.go"
+# Flag handlers that don't call GetK8sClientsForRequest (manual review for user-flow endpoints)
+rg -n "func.*Handler\|func.*Handle" components/backend/ --glob="*.go" -A 5 | grep -v "GetK8sClientsForRequest" | grep "func.*Handler"
+```
+
 ### S2: RBAC before resource access (Critical)
 
 `SelfSubjectAccessReview` (or equivalent authz check) should precede user-scoped resource access.
 
+```bash
+# Find SelfSubjectAccessReview usage — flag user-resource endpoints that lack it
+rg -n "SelfSubjectAccessReview|SubjectAccessReview" components/ --glob="*.go" | grep -v "_test.go"
+# Find resource access patterns without preceding RBAC check (flag for manual review)
+rg -n "client\.Get\|client\.List\|client\.Create" components/backend/ --glob="*.go" -B 10 | grep -v "SelfSubjectAccessReview"
+```
+
 ### S3: Token redaction in all outputs (Blocker)
 
 No tokens in logs, errors, or API responses. Use `len(token)` for logging.
 
+```bash
+# Find token variables logged directly (should use len(token) instead)
+rg -n "log.*[Tt]oken\|Sprintf.*[Tt]oken\|Error.*[Tt]oken" components/ --glob="*.go" | grep -v "_test.go" | grep -v "len(token)"
+# Find token values in response bodies
+rg -n '"token"\s*:\s*[a-zA-Z]' components/ --glob="*.go" | grep -v "_test.go"
+```
+
 ### S4: Input validation (Major)
 
 DNS labels validated, URLs parsed, no raw newlines for log injection.
 
+```bash
+# Find URL construction without url.Parse validation
+rg -n "url\.Parse\|url\.ParseRequestURI" components/ --glob="*.go" | grep -v "_test.go"
+# Find log statements with user-controlled input (potential log injection via newlines)
+rg -n 'log\.(Info|Error|Warn).*(name|label|input|param|query)' components/ --glob="*.go" | grep -v "_test.go"
+# Find DNS label handling without regex validation
+rg -n "IsDNSLabel\|dns1123\|ValidateName" components/ --glob="*.go" | grep -v "_test.go"
+```
+
 ### S5: SecurityContext on pods (Critical)
 
 `AllowPrivilegeEscalation: false`, `Capabilities.Drop: ["ALL"]`.
 
+```bash
+# Find PodSpec/container definitions — verify SecurityContext is set
+rg -n "AllowPrivilegeEscalation\|Capabilities" components/ --glob="*.go" | grep -v "_test.go"
+# Flag pod/container specs missing AllowPrivilegeEscalation
+rg -U "corev1\.Container\{[^}]*\}" components/ --glob="*.go" --multiline | grep -v "AllowPrivilegeEscalation"
+```
+
 ### S6: OwnerReferences on Secrets (Critical)
 
 Secrets created by the platform must have OwnerReferences for cleanup.
 
+```bash
+# Find Secret create calls
+rg -n "corev1\.Secret\|&v1\.Secret\|Secrets\(\)" components/ --glob="*.go" | grep -v "_test.go"
+# Find Secret creation sites — verify OwnerReferences is set
+rg -n "client\.Create.*[Ss]ecret\|Create.*corev1\.Secret" components/ --glob="*.go" -B 10 | grep -v "_test.go" | grep -v "OwnerReferences"
+```
+
 ### S7: No hardcoded credentials (Blocker)
 
 ```bash

.claude/commands/jira.log.md

@@ -22,7 +22,7 @@ Extract the following from `$ARGUMENTS`:
 
 - **Summary** (required): The title/summary of the issue
 - **Description** (optional): Detailed description of the work
-- **Issue Type** (optional): Defaults to `Story`, but can be `Bug` or `Task`. Tasks are tech debt related and not user facing
+- **Issue Type** (optional): Defaults to `Story`, but can be `Bug` or `Task`. Tasks are tech debt related and not user facing. Normalize case before use (e.g. "bug" → "Bug", "TASK" → "Task") and validate against this allowlist; reject unrecognized values with a prompt to the user.
 - **Priority** (optional): Defaults to `Normal`
 
 If the user provides a simple sentence, use it as the summary. If they provide multiple lines, use the first line as summary and the rest as description.
@@ -119,7 +119,7 @@ Use the `mcp__jira__jira_create_issue` tool with:
 {
   "project_key": "RHOAIENG",
   "summary": "[user provided summary]",
-  "issue_type": "[parsed issue type from step 1]",
+  "issue_type": "[validated issue type: normalize to Story|Bug|Task — default Story if unrecognized]",
   "description": "[structured description from template]",
   "components": "Agentic",
   "additional_fields": "{\"labels\": [\"team:ambient\"]}"

.claude/skills/cypress-demo/SKILL.md

@@ -47,15 +47,20 @@ When invoked, create a Cypress test file in `e2e/cypress/e2e/` that records a de
 - Verify `e2e/.env.test` or `e2e/.env` exists with `TEST_TOKEN`
 - Check if `ANTHROPIC_API_KEY` is available (needed for Running state)
 - Verify the kind cluster is up: `kubectl get pods -n ambient-code`
-- Verify the frontend is accessible: `curl -s -o /dev/null -w "%{http_code}" http://localhost`
+- Verify the frontend is accessible: `curl -s -o /dev/null -w "%{http_code}" http://localhost:3000`
 
 ### 3. Create the demo test file
 
 Create `e2e/cypress/e2e/<feature-name>-demo.cy.ts` using the helpers below.
 
 #### Required helpers
 
-Copy the demo helpers (cursor, caption, click ripple, timing constants) from the reference implementation at `e2e/cypress/e2e/sessions.cy.ts` into each new demo file. The helpers are: `caption()`, `clearCaption()`, `initCursor()`, `moveTo()`, `moveToText()`, `clickEffect()`, `cursorClickText()`, plus timing constants (`LONG`, `PAUSE`, `SHORT`, `TYPE_DELAY`).
+Each demo file must define the following helpers inline (they are not shared utilities — embed them directly in the test file):
+
+- **Functions**: `caption()`, `clearCaption()`, `initCursor()`, `moveTo()`, `moveToText()`, `clickEffect()`, `cursorClickText()`
+- **Timing constants**: `LONG`, `PAUSE`, `SHORT`, `TYPE_DELAY`
+
+See the patterns table below for usage. If a prior demo file already exists in `e2e/cypress/e2e/`, copy the helpers from there; otherwise implement them fresh using the patterns below.
 
 ### 4. Key patterns
 
@@ -77,4 +82,4 @@ npx cypress run --no-runner-ui --spec "cypress/e2e/<name>-demo.cy.ts"
 
 ### 6. Reference implementation
 
-See `e2e/cypress/e2e/sessions.cy.ts` for a complete working example.
+`e2e/cypress/e2e/sessions.cy.ts` is a reference for test structure and Cypress patterns in this repo. Note that sessions.cy.ts is a functional test, not a demo test — it does not contain the cursor/caption/click-ripple helpers listed above. Use it as a guide for selectors, workspace setup, and polling patterns; implement the demo-specific helpers fresh in your new file.

.claude/skills/pr-fixer/SKILL.md

@@ -25,9 +25,14 @@ The PR number is required. Example: `/pr-fixer 1234`
    - Confirm the repo has a `pr-fixer.yml` workflow
 
 2. **Pre-flight PR check**
-   - Verify the PR exists and is open: `gh pr view <N> --repo <owner/repo> --json state --jq .state`
-   - If PR is closed/merged, abort with: "PR #N is already <state>. Nothing to fix."
-   - If PR doesn't exist, abort with: "PR #N not found in <owner/repo>."
+   - Check the PR exists and its state:
+     ```bash
+     gh pr view <N> --repo <owner/repo> --json state --jq .state
+     ```
+   - `gh pr view` returns state in **uppercase**: `OPEN`, `CLOSED`, or `MERGED`. A non-zero exit code (with stderr error) means the PR does not exist.
+   - If the exit code is non-zero: abort with "PR #N not found in <owner/repo>."
+   - If state is `CLOSED` or `MERGED`: abort with "PR #N is already <state>. Nothing to fix." (use the actual uppercase value returned)
+   - Only proceed if state is exactly `OPEN`.
 
 3. **Dispatch the workflow**
    ```bash

.claude/skills/pr-fixer/evals/evals.json

@@ -17,6 +17,18 @@
     "expected_args": { "skill": "pr-fixer", "args": "987" },
     "description": "Natural language with hash prefix on PR number"
   },
+  {
+    "input": "fix https://github.com/org/repo/pull/1234",
+    "expected_tool_call": "Skill",
+    "expected_args": { "skill": "pr-fixer", "args": "1234" },
+    "description": "URL-form PR reference — extracts PR number from full GitHub URL"
+  },
+  {
+    "input": "please fix PR 1234.",
+    "expected_tool_call": "Skill",
+    "expected_args": { "skill": "pr-fixer", "args": "1234" },
+    "description": "Punctuation-trailing PR number — trailing period must not be captured"
+  },
   {
     "input": "/pr-fixer",
     "expected_tool_call": "None",

.claude/skills/scaffold/evals/evals.json

@@ -8,8 +8,8 @@
   {
     "input": "add a new integration for PagerDuty",
     "expected_tool_call": "Skill",
-    "expected_args": { "skill": "scaffold" },
-    "description": "Natural language integration scaffolding request"
+    "expected_args": { "skill": "scaffold", "args": "integration PagerDuty" },
+    "description": "Natural language integration scaffolding request — must extract integration name"
   },
   {
     "input": "/scaffold endpoint audit-logs",

.claude/skills/unleash-flag/SKILL.md

@@ -169,12 +169,19 @@ This returns 404 (not 403) when the flag is off, so the endpoint appears not to
 
 ### E2E Testing with Feature Flags
 
-When writing E2E tests for flagged features, set the admin token via environment variable (obtain from Unleash UI > API Access, or from deployment secrets):
+When writing E2E tests for flagged features, the Unleash admin token must be set as an environment variable. **Never paste or export the token inline in a shell session** — this risks exposing it in shell history, logs, or terminal recordings.
+
+Obtain the token from Unleash UI > API Access or from deployment secrets, then set it using a non-echoing prompt or a secret manager:
 ```bash
-export CYPRESS_UNLEASH_ADMIN_TOKEN='<your-unleash-admin-token>'
+# Non-echoing prompt (token not stored in shell history)
+read -rs CYPRESS_UNLEASH_ADMIN_TOKEN && export CYPRESS_UNLEASH_ADMIN_TOKEN
 ```
 
-Alternatively, map it in `e2e/cypress.config.ts`:
+In CI, inject it via a secret reference (e.g. GitHub Actions secret, Vault, or sealed secret) — never hard-code or `echo` the value.
+
+> **Note**: When logging token-related errors, use `len(token)` to confirm presence without exposing the value. Never include the full token in error messages, API responses, or commit history.
+
+Map it in `e2e/cypress.config.ts`:
 ```typescript
 env: { UNLEASH_ADMIN_TOKEN: process.env.CYPRESS_UNLEASH_ADMIN_TOKEN }
 ```