[kentcdodds]

Summary

• stop rewriteCapabilitySecretError from scraping secret names out of the full wrapped execute bundle
• only derive capability-approval secret names from the current error payload (string or Error.message)
• add a regression test proving Secret "..." text inside user code does not inflate approval errors
• strengthen execute/search/docs guidance so agents do not copy {{secret:...}} placeholder text into visible content such as issue bodies, comments, prompts, logs, or returned strings
• remove low-value tests that only pinned instructional/configuration copy, and document that preference in the repo testing guidelines
• reduce repeated secret search warning text by keeping the warning in human-facing markdown while restoring concise structured usage output

Validation

• npm exec vitest run packages/worker/src/mcp/run-codemode-registry.node.test.ts
• npm exec vitest run packages/worker/src/mcp/fetch-gateway.node.test.ts
• npm exec vitest run packages/worker/src/mcp/tools/execute.node.test.ts
• npm exec vitest run packages/worker/src/mcp/capabilities/unified-search.workers.test.ts

Notes

• The actual fetch placeholder resolver remains scoped to request URL, headers, and body in fetch-gateway.ts
• The original approval-list bug came from over-broad secret-name extraction during capability-error rewriting, not execute state being accumulated by conversation
• Agent-facing guidance now warns against placing placeholder syntax into user-visible or third-party-visible prose
• docs/contributing/testing-principles.md now explicitly tells contributors not to add tests whose only value is pinning tool descriptions, usage hints, warnings, or other instructional copy
• Secret search structured output is concise again; the stronger warning remains in the search markdown/detail prose instead of being repeated in every usage field

  

Summary by CodeRabbit

• Documentation

  • Prohibit embedding literal secret placeholder tokens in user-visible or third-party-visible content; recommend obfuscation when mentioning placeholders.
  • Clarify approved contexts for placeholders (execute-time wiring and explicit capability inputs).
  • Added testing guidance to avoid tests that only assert/pin instructional prose.

• Bug Fixes

  • Improved secret error reporting to avoid exposing unrelated secret text.

• Tests

  • Updated/added tests to stop asserting literal placeholder formatting and to validate error conversion behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f3ce226b-7ba2-4939-97c3-c95e22c8c3f2

📥 Commits

Reviewing files that changed from the base of the PR and between edbfa81 and 284a9c3.

📒 Files selected for processing (6)

• docs/contributing/testing-principles.md
• packages/worker/src/mcp/capabilities/unified-search.ts
• packages/worker/src/mcp/execute-modules/codemode-utils.node.test.ts
• packages/worker/src/mcp/execute-modules/codemode-utils.ts
• packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts
• packages/worker/src/mcp/run-codemode-registry.node.test.ts

✅ Files skipped from review due to trivial changes (6)

• docs/contributing/testing-principles.md
• packages/worker/src/mcp/execute-modules/codemode-utils.ts
• packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts
• packages/worker/src/mcp/run-codemode-registry.node.test.ts
• packages/worker/src/mcp/capabilities/unified-search.ts
• packages/worker/src/mcp/execute-modules/codemode-utils.node.test.ts

---

📝 Walkthrough

Walkthrough

Updated docs and runtime text to forbid embedding literal secret placeholders ({{secret:...}}) in user-visible or third-party-visible content; extended secret-usage guidance and search-formatting text; refactored secret-name extraction to parse only structured capability error messages; and adjusted tests to avoid brittle literal-content assertions and to verify capability secret-access errors.

Changes

Cohort / File(s)	Summary
Documentation & Guidance docs/contributing/testing-principles.md, docs/use/execute.md, docs/use/secrets-and-values.md, packages/worker/src/mcp/tools/execute.ts	Added guidance forbidding literal {{secret:...}} in user-visible or third-party-visible content (issue bodies, comments, prompts, logs, returned strings) and instructing obfuscation when mentioning placeholders; added testing guideline to avoid tests that assert/pin configuration-style prose.
Search & Capability Usage Text packages/worker/src/mcp/tools/search-format.ts, packages/worker/src/mcp/capabilities/unified-search.ts	Extended formatted guidance and buildSecretUsage text to restrict secret-placeholder placement to execute-time fetch URL/header/body fields or explicit capability inputs, and to prohibit placement in visible content.
Secret Extraction & Runtime packages/worker/src/mcp/run-codemode-registry.ts	Stopped passing wrapped code into secret-error rewriting; removed parsing of wrapped code for secret names and now extract secret names solely from structured capability error messages via collectSecretNamesFromCapabilityError.
Tests packages/worker/src/mcp/capabilities/unified-search.workers.test.ts, packages/worker/src/mcp/run-codemode-registry.node.test.ts, packages/worker/src/mcp/capabilities/coding/coding-capabilities.node.test.ts, packages/worker/src/mcp/execute-modules/codemode-utils.node.test.ts, packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts	Removed brittle assertions checking for literal {{secret:; added test asserting capability secret access-denied batch error and that unrelated plaintext in user code is not leaked; wrapped coding-capabilities tests in a describe with timeout; minor test type/format tweaks.
Formatting / Minor Refactors packages/worker/src/mcp/capabilities/unified-search.ts, packages/worker/src/mcp/execute-modules/codemode-utils.ts, packages/worker/src/mcp/tools/search-format.ts	Non-functional formatting and small refactors (arrow-function simplifications, conditional reformatting, string join layout) and minor adjustments to lexical scoring expression grouping.

Sequence Diagram(s)

sequenceDiagram
  participant Runner as runCodemodeWithRegistry
  participant Registry as CapabilityRegistry
  participant Secrets as SecretResolver
  participant Executor as Executor
  participant Errors as ErrorParser

  Runner->>Registry: fetch capability registry
  Runner->>Secrets: resolve required secrets (stubbed in tests)
  Runner->>Executor: execute wrapped code
  Executor-->>Runner: returns capability secret access-denied error (structured)
  Runner->>Errors: collectSecretNamesFromCapabilityError(error)
  Errors-->>Runner: extracted secret names (from structured message only)
  Runner->>Runner: createCapabilitySecretAccessDeniedBatchMessage(extracted)
  Runner-->>Client: return batch error (no plaintext copied from wrapped code)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Add secure codemode secret_set capability #96: Overlaps secret-placeholder/redaction and run-codemode-registry secret extraction changes.
• feat: add secret-aware fetch gateway #61: Related edits to buildSecretUsage, execute tool docs, and unified-search secret guidance.
• docs: align testing principles with fewer longer tests #105: Related updates to testing-principles and removing brittle literal-content assertions.

Poem

🐰 Quiet paws, I tuck tokens tight,
No bare placeholders in plain sight.
Fetch keeps secrets where they belong,
Tests now check behavior—not a song.
Nibble, patch, and hop—code feels light!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the primary change: stopping the scanning of wrapped execute code for capability secret names, which is the core fix implemented in the PR.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch cursor/secret-placeholder-scope-f8c4

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

🔎 Preview deployed: https://kody-pr-129.kentcdodds.workers.dev

Worker: kody-pr-129
D1: kody-pr-129-db
KV: kody-pr-129-oauth-kv

Mocks:

• ai: https://kody-pr-129-mock-ai.kentcdodds.workers.dev/__mocks (kody-pr-129-mock-ai)
• cloudflare: https://kody-pr-129-mock-cloudflare.kentcdodds.workers.dev/__mocks (kody-pr-129-mock-cloudflare)
• resend: https://kody-pr-129-mock-resend.kentcdodds.workers.dev/__mocks (kody-pr-129-mock-resend)