^D Contributor runbook depth: adapter READMEs + worked examples (E7)#387
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 054207e0bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 054207e0bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Fixed the breakglass claim (security-relevant): verified provider-wrapper.sh sets WORKCELL_WRAPPER_CONTEXT=1 so provider_policy_allows_breakglass is false in the wrapper — the unsafe-flag rejection is re-applied and holds in every mode including breakglass (container-smoke.sh asserts breakglass Claude/Codex overrides fail). Corrected all 5 instances to say the rejection is enforced in every mode and breakglass only raises the sandbox floor. (The Container smoke check failure was an unrelated flake on this docs-only PR — re-running.) |
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51869743cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 51869743cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Lines 47 to 50 in 4c8e30b
This makes promotion sound like only an internal/adapters/data.go row plus files under adapters/<name>/, but the launch path still has separate support gates: providerid.AllProviders controls IsValid, and scripts/workcell currently exits early for antigravity before runtime preparation unless its supported-agent case is changed. A contributor following this would create registry metadata without actually enabling a runnable provider.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15b588aee5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15b588aee5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Rebased onto main (now includes the Cloudflare CDN mirror fix #392) as a clean linear history (dropped the earlier main-sync merge commit) so CI builds on the reliable mirror. Docs content unchanged. |
15b588a to
490ed43
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 490ed435ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Fixed: (1) regenerated control-plane-manifest.json — the enriched adapter READMEs are manifest-tracked and the copilot README sha had gone stale (surfaced once the mirror fix let Validate actually run); (2) added the launcher/help/why/auth-status touchpoints to the 'add a provider' worked example, since scripts/workcell validates the agent set and a contributor completing only the registry edits would still hit |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09fb7eb82b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Added both touchpoints (verified against source): the new-adapter step now names the runtime dispatch surfaces (runtime/container/entrypoint.sh codex|claude|copilot|gemini cases, the Rust core-launcher, Dockerfile core-wrapper symlinks), and the credential-key step now calls out print_injection_status's hard-coded supported_credential_keys list in scripts/workcell so a new key isn't silently missing from --auth-status. markdownlint/links/hygiene green. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afccb9c918
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afccb9c918
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed the class rather than enumerating endlessly: added a 'the registration spans layers — grep to find them all' consolidating note to each checklist naming the auth-management touchpoints (canonicalCredentialDestinations, statusOrder, bootstrapSummaryForCredential), extended the resolver step to require the resolveCredential/ProbeResolverReadiness cases (allowedResolvers alone only passes policy validation), and added the provider-e2e live-certification gate before any supported-tier claim in provider-matrix.md. Every symbol grep-verified against source. markdownlint/links/hygiene green. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b004ae9bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b004ae9bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Fixed both: corrected the note to point at internal/authpolicy/manage.go where canonicalCredentialDestinations is declared (staging.go only consumes it), and added the control-plane manifest touchpoint to the seed step — adapter baseline files are checked by workcell_verify_control_plane_prefix and must be added to the metadatautil/core.go manifest source and regenerated/verified, else home seeding aborts. Grep-verified both. markdownlint/links green. |
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8394958bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… (enrich the 5 thin adapter READMEs - antigravity/claude/codex/copilot/gemini - with accurate auth methods, managed control-plane files, and behavior summaries grounded in internal/adapters/data.go, authresolve, provider-policy.sh, provider-wrapper.sh, and home-control-plane.sh; add docs/extending-adapters.md with two worked examples - add a credential type, extend/add an adapter - each referencing the invariants/threat-model items it touches; Example 1 dry-run-verified with go build+test and a negative control) (docs-only, no code change; user-visible contributor docs)
…Es and contributor guide said breakglass exempts the provider-native unsafe-flag rejection, but provider-wrapper.sh always sets WORKCELL_WRAPPER_CONTEXT=1, which makes provider_policy_allows_breakglass false, so the wrapper re-checks and rejects those flags even in breakglass - container-smoke.sh asserts breakglass Claude/Codex overrides still fail; fix all 5 instances to say the rejection holds in every mode and breakglass only raises the sandbox floor) (docs-only accuracy fix; corrects a security-relevant operator claim)
…(the Gemini adapter README carried the same incorrect 'breakglass exempts these' unsafe-flag claim as the others - missed in the prior commit; the wrapper re-checks arguments so the rejection holds in every mode including breakglass) (docs-only accuracy fix; completes the breakglass correction across all 5 adapters)
…sted managed-settings.json as seeded to ~/.claude/settings.json, but workcell_render_claude_settings seeds that from adapters/claude/.claude/settings.json, while managed-settings.json is the separate enterprise policy installed at /etc/claude-code/managed-settings.json; distinguish the two so contributors edit the right file for session-home settings) (docs-only accuracy fix; prevents edits to the wrong control-plane file)
…touchpoints (P2 CI) (the adapter READMEs are control-plane-tracked; enriching them left control-plane-manifest.json stale (copilot README sha), which verify-control-plane-manifest.sh caught once the mirror fix let Validate run - regenerate it; and the 'add a provider' worked example omitted that scripts/workcell validates the agent set and backs help/why/auth-status, so a contributor could finish the registry edits yet still have workcell --agent <new> fail - document the launcher/help touchpoints) (manifest/markdownlint/links/hygiene green; docs + regenerated manifest)
…er promotion (P2/P3) (verified vs source: runtime/container/entrypoint.sh routes only codex|claude|copilot|gemini (cases at 251/256) alongside the Rust core-launcher and Dockerfile symlinks, and print_injection_status in scripts/workcell prints a hard-coded supported_credential_keys list (5614); extend the new-adapter step to name the entrypoint/core-launcher/symlink touchpoints and the credential-key step to update the auth-status list, so a contributor can't complete the registry work yet still have workcell --agent <new> fail to dispatch or omit the key from --auth-status) (markdownlint/links/hygiene green, claims grep-verified; docs-only completeness)
…r/credential promotion (P2/P3) (the runbook checklists implied the registry edits were sufficient, but provider/credential registration is intentionally spread across layers - all grep-verified: resolver-backed keys need resolveCredential/ProbeResolverReadiness cases (not just allowedResolvers), a new key touches authpolicy staging (canonicalCredentialDestinations) and status ordering/summaries (statusOrder, bootstrapSummaryForCredential), and a supported-tier promotion needs the provider-e2e live certification; add a grep-the-tree consolidating note per checklist plus the resolution-dispatch and live-cert gates so contributors find every site rather than shipping a key/provider that validates but cannot resolve, stage, or be certified) (markdownlint/links/hygiene green, every symbol grep-verified; docs-only completeness, consolidating over enumerating)
…st touchpoint (P2/P3) (grep-verified: canonicalCredentialDestinations is declared in internal/authpolicy/manage.go (staging.go only consumes it) - fix the note's file reference; and a new adapter's baseline files under adapters/<name>/ are checked at seed time by workcell_verify_control_plane_prefix, generated from internal/metadatautil/core.go into control-plane-manifest.json - add regenerate-and-verify to the seed step so home seeding does not abort on a missing manifest entry) (markdownlint/links green, references grep-verified; docs-only accuracy + completeness)
…) (grep-verified: a promoted provider that installs a direct runtime binary must be added to PROTECTED_RUNTIME_PATHS in runtime/container/rust/src/lib.rs and the development-mode blocklist development_wrapper_protected_runtime_match + provider command-name case in development-wrapper.sh, or /usr/local/libexec/workcell/real/<new> can be executed directly and bypass the managed wrapper's unsafe-argument rejection; add this security touchpoint to the dispatch step) (markdownlint/links green, symbols grep-verified; docs-only security-relevant completeness)
|
Added the exec-guard touchpoint (security): a promoted provider with a direct runtime binary must be added to PROTECTED_RUNTIME_PATHS (rust/src/lib.rs) and the development-mode blocklist in development-wrapper.sh, else the real binary can be executed directly and bypass the managed wrapper. Grep-verified. Also rebased onto current main (now includes E5) so the diff is clean. markdownlint/links green. |
c839495 to
aa2a15c
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa2a15c490
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa2a15c490
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…l seeding (P2) (grep-verified: workcell_verify_control_plane_prefix only checks entries already listed in the manifest and does not scan adapters/<name>/, so verify-control-plane-manifest.sh (regenerate + diff) is the actual required gate - reframe the note; and home-control-plane.sh runs under set -e where workcell_copy_manifest_credential_file returns non-zero for an absent key, so an optional/mode-filtered credential needs the existing || true / if ! seed pattern or it aborts launches - document it) (markdownlint/links green, behavior grep-verified; docs-only accuracy)
|
Fixed both accuracy points: (1) reframed the manifest note — workcell_verify_control_plane_prefix only checks already-listed entries, so scripts/verify-control-plane-manifest.sh (regenerate + diff) is the real required gate, not a seed-time abort; (2) documented that optional/mode-filtered credentials need the existing || true / if ! seed guard since home-control-plane.sh runs under set -e. Grep-verified both. markdownlint/links green. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f0e05b180
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Copilot ships no repo baseline files under `adapters/copilot/`; its control | ||
| plane is entirely session-local. The adapter owns a session-local `COPILOT_HOME` |
There was a problem hiding this comment.
Stop saying Copilot has no baseline files
Fresh evidence after the prior manifest-hash finding is that the current tree still declares adapters/copilot/README.md as an adapter-baseline in internal/metadatautil/core.go, and seed_copilot_home verifies the /opt/workcell/adapters/copilot/ prefix before launch. Calling this directory baseline-free makes future Copilot README/control-plane edits look exempt from manifest regeneration, but a mismatched tracked README still aborts Copilot home seeding.
Useful? React with 👍 / 👎.
Implements E7: Contributor Runbook Depth.
What
internal/adapters/data.go,internal/authresolve,runtime/container/provider-policy.sh,provider-wrapper.sh,home-control-plane.sh). No stub README remains.docs/extending-adapters.mdwith two worked contributor examples — "add a credential type" and "extend/add an adapter" — each step referencing the invariants/threat-model items it touches (host-secret containment, workspace-write ownership, no silent widening, network posture, defense-in-depth, labeled lower-assurance).Verification
go build ./...+go test ./internal/adapters/... ./internal/injection/...passed; a negative control (omitting the container-path step) trippedTestScopedCredentialKeysHaveContainerPathswith the exact message the doc cites. All edits reverted.sharedCredentialsEnabled, unsafe-arg rejections, antigravity fail-closed).Validation
markdownlint, check-doc-links, codespell all green. 8 files, ~603 lines, docs-only.
🤖 Generated with Claude Code