Skip to content

feat: tell Codex to run contextbridge open outside the sandbox#115

Merged
blimmer merged 6 commits into
mainfrom
feat/skill-template-rendering
May 14, 2026
Merged

feat: tell Codex to run contextbridge open outside the sandbox#115
blimmer merged 6 commits into
mainfrom
feat/skill-template-rendering

Conversation

@blimmer
Copy link
Copy Markdown
Contributor

@blimmer blimmer commented May 14, 2026

Summary

contextbridge open launches a local browser, which Codex's default sandbox blocks — $planbridge-open prints the listening URL but the browser never opens. The fix is content the agent reads: the Codex SKILL.md needs to tell it to set sandbox_permissions: "require_escalated" (with a justification and optionally a reusable prefix_rule) when invoking the command.

To deliver harness-specific content without leaking a hardcoded Codex appendix into the renderer, this PR makes SKILL.md bodies Handlebars templates with { harness } as the data context and a single eq helper, and introduces partials under packages/skills/sources/_partials/<harnessId>/<topic>.md. The codex/sandbox-escalation partial is the first consumer; open/SKILL.md gains one templated block at the call site. Rendered outputs are formatted through Prettier (markdown parser) before being written, which collapses the extra blank line Handlebars leaves where a non-matching conditional sat — keeping per-harness diffs clean and letting the harnessIntegrations/*/skills/**/SKILL.md exclusion drop from .prettierignore. Closes #110.

Review focus

  • Codex exec_command parameters cited in the partial are verified against the Codex sourcesandbox_permissions: "require_escalated", justification, and prefix_rule: ["contextbridge", "<subcommand>"] all come from codex-rs/core/src/tools/handlers/shell_spec.rs::create_approval_parameters. require_escalated is the right mode for a browser launch (not with_additional_permissions, which is for additive sandbox grants).
  • Render errors throw with the skill name + harness id, with the caught error attached as cause so the missing-partial Handlebars error surfaces with context: failed to render skill 'open' for harness 'codex': The partial codex/<missing> could not be found.
  • The partial is phrased generically ("this skill runs commands that require resources outside Codex's default sandbox") rather than hardcoding contextbridge open and a specific prefix_rule, so a future skill wrapping a different subcommand can pull the same partial in unchanged.
  • Prettier formatting runs inside targetsFor (via prettier.resolveConfig(path) + prettier.format(body, { ...config, filepath: path })) so config inherits from the destination's location in the repo. skills:check formats the same way, so byte-diff drift detection still works.

Commits

  • e66197d — Handlebars templating + _partials/ registry in @contextbridge/skills; codex/sandbox-escalation partial; open SKILL.md uses it; docs updated.
  • 0bc46bc — Format rendered SKILL.md through Prettier in targetsFor; drop the harnessIntegrations/*/skills/**/SKILL.md exclusion from .prettierignore (drift check still guards against hand-edits).
  • 1836cc9 — Rewrite the codex sandbox-escalation partial to drop contextbridge open-specific text so future skills with other commands can reuse it unchanged.
  • 56531a0 — PR feedback: hoist the local toError in renderTargets.ts to the shared @contextbridge/shared/errors implementation; replace the manual recursive readdirSync walk in handlebars.ts with Bun.Glob('**/*.md').scanSync().
  • c679f6c — Inject a minimal BaseContext (logger + telemetry-disabled) into the skills:generate and skills:check scripts so diagnostics flow through pino instead of console.*.

blimmer added 3 commits May 14, 2026 09:10
contextbridge open launches a local browser window, which is blocked by
Codex's default sandbox: $planbridge-open prints the listening URL but
the browser never opens automatically. The fix is content the Codex agent
reads — its SKILL.md needs to tell it to escalate the sandbox.

To deliver that content without leaking a hardcoded Codex appendix into
the renderer, this change makes SKILL.md bodies Handlebars templates:

* render() compiles the body and evaluates it with { harness } as data.
* One helper, eq (strict equality), enables call-site conditionals like
  {{#if (eq harness.id "codex")}}…{{/if}}.
* Reusable per-harness snippets live at sources/_partials/<harnessId>/
  <topic>.md and are registered as Handlebars partials named after their
  path under _partials/ minus the .md extension.
* loadAllFrom skips underscore-prefixed directories so the partials tree
  is invisible to the skill loader and orphan-detection pass.
* Render errors throw with the skill name + harness id, with the original
  error attached as cause.

The new codex/sandbox-escalation partial cites the parameters Codex's
exec_command/shell tool actually accepts — sandbox_permissions:
\"require_escalated\", a justification, and a suggested prefix_rule of
[\"contextbridge\", \"open\"] — verified against
codex-rs/core/src/tools/handlers/shell_spec.rs in the codex repo.

The open SKILL.md gains one templated block at the call site:

    {{#if (eq harness.id \"codex\")}}
    {{> codex/sandbox-escalation}}
    {{/if}}

Generated harnessIntegrations/<id>/skills/<n>/SKILL.md files are added
to .prettierignore — the renderer is the source of truth for their
shape, and we already follow this pattern for CHANGELOG.md,
projen-managed files, and harnessIntegrations/claude/.claude-plugin/
plugin.json.

Closes #110.
Run rendered outputs through prettier in `targetsFor` so the extra
blank line Handlebars leaves where a non-matching conditional sat is
collapsed; drop the harnessIntegrations/*/skills/**/SKILL.md exclusion
from .prettierignore since the generator now produces prettier-
conformant output (drift check still guards against hand-edits).
The partial referenced `contextbridge open`, its browser-window
behavior, and a hardcoded `prefix_rule: ["contextbridge", "open"]`.
Rewrite it to describe the codex escalation pattern generically so
future skills with other commands can pull it in unchanged.
Comment thread packages/skills/scripts/check.ts Outdated
const safeReaddir = fromThrowable((dir: string) => readdirSync(dir, { withFileTypes: true }));

function main(): void {
async function main(): Promise<void> {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

async now because we prettier-format the skills (which is an async call)

SKILL_RENDERABLE_HARNESSES.map(async (harness) => {
const path = join(outDirFor(harness), harness.skillRendering.installName(skill.frontmatter.name), 'SKILL.md');
const config = await prettier.resolveConfig(path);
const body = await prettier.format(render(skill, harness), { ...config, filepath: path });
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new partial was adding unneeded newlines, which prettier rightly removes.

@@ -0,0 +1,3 @@
## Running this from Codex
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many skills will need this, so introducing the idea of a partial

Comment thread packages/skills/AGENTS.md

All files are committed. `bun run skills:check` (wired into `just verify` and the lint CI job) regenerates in-memory and fails on byte-diff.

## Templating
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figure we'll have this with other harnesses eventually

@blimmer blimmer marked this pull request as ready for review May 14, 2026 15:31
@blimmer blimmer requested a review from jcarver989 as a code owner May 14, 2026 15:31
Comment thread packages/skills/src/handlebars.ts Outdated
Comment on lines +21 to +42
function walkPartials(rootDir: string): LoadedPartial[] {
return walk(rootDir).map((absolutePath) => ({
name: relative(rootDir, absolutePath).replace(/\.md$/, '').split(sep).join('/'),
source: readFileSync(absolutePath, 'utf8'),
}));
}

function walk(dir: string): string[] {
let entries;
try {
entries = readdirSync(dir, { withFileTypes: true });
} catch (err) {
if ((err as NodeJS.ErrnoException).code === 'ENOENT') return [];
throw err;
}
return entries.flatMap((entry) => {
const full = join(dir, entry.name);
if (entry.isDirectory()) return walk(full);
if (entry.isFile() && entry.name.endsWith('.md')) return [full];
return [];
});
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://bun.com/reference/node/fs/readdir -- Bun/Node have a built in fn to recursively list files, wondering if we could use that vs rolling our own visitor/walk pattern here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great callout! I wonder if we should have some agent skill that guides it to use more native bun solutions (like this, macros in your previous DB PR, etc.)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, worth trying!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed as #120 — we'll handle the agent rule there.

Comment on lines +21 to 30
function compileBody(env: HandlebarsEnv, skill: Skill, harness: HarnessDescriptor): string {
try {
return env.compile(skill.body, { noEscape: true })({ harness });
} catch (err) {
const message = err instanceof Error ? err.message : String(err);
throw new Error(`failed to render skill '${skill.frontmatter.name}' for harness '${harness.id}': ${message}`, {
cause: err,
});
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this return Result or did you prefer the throw style ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure we can neverthrow it. Do you think there's a way we can guide agents to do this? It's definitely not in their training data...

Copy link
Copy Markdown
Contributor

@jcarver989 jcarver989 May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea over in cb-bot, ContextBot realized we do this and wrote us a rule for it here: https://github.com/contextbridge/cb-bot/blob/91dc39e5d48316db36b657365fae0482c14c3373/.contextbridge/rules/neverthrow-error-handling.md -- so I suppose the easiest thing to do would be to copy that over into our Claude rules here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly, cb-bot did that here, too, but it's not being paid attention to. https://github.com/contextbridge/planbridge/blob/main/.contextbridge/rules/error-handling-neverthrow.md

I can't remember off the top of my head if we can edit these files? I don't think so.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah interesting ... Yea currently .contextbridge/ files are assumed to be owned by cb-bot and not-editable.

This is a good usecase for a janitor agent that reviews the diff and fixes it w/ a fresh context window. We could either do that locally or via cb-bot on a schedule (i.e. violation fix)

blimmer added 2 commits May 14, 2026 10:38
Scripts were using console.* directly. Wire them through a minimal
BaseContext (logger + telemetry-disabled) so diagnostics flow through
pino like the rest of the project.
@blimmer blimmer merged commit 6cff1e9 into main May 14, 2026
13 checks passed
@blimmer blimmer deleted the feat/skill-template-rendering branch May 14, 2026 16:57
blimmer pushed a commit that referenced this pull request May 14, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.6.0](v0.5.0...v0.6.0)
(2026-05-14)


### Features

* add global feedback submit shortcut
([#116](#116))
([21b14a1](21b14a1))
* allow configuring PlanBridge port
([#95](#95))
([bf53148](bf53148))
* tell Codex to run contextbridge open outside the sandbox
([#115](#115))
([6cff1e9](6cff1e9))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: contextbridge-pr-automation[bot] <259134118+contextbridge-pr-automation[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: tell Codex to run contextbridge open outside the sandbox

2 participants