Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CLAUDE.md`:
- Line 285: Add the missing "omp" provider entry to all provider lists in
CLAUDE.md to make them consistent with the new community/omp line; search for
other provider-set arrays or bullet lists that enumerate providers (e.g., the
pre-OMP provider lists and any occurrences around the existing community/omp
mention) and add the provider id `omp` (and the same descriptor style
"OmpProvider (builtIn: false) — `@oh-my-pi/pi-coding-agent`, provider id `omp`"
where appropriate) so every list matches and no sections omit `omp`.

In `@packages/docs-web/src/content/docs/guides/mcp-servers.md`:
- Line 170: Replace the internal camelCase usage `allowedTools` with the
user-facing snake_case `allowed_tools` in the sentence "When a Claude node loads
MCP servers, tool wildcards are automatically added to `allowedTools`." in the
MCP servers guide (packages/docs-web/src/content/docs/guides/mcp-servers.md);
update any nearby examples or YAML snippets in the same guide that use
`allowedTools` to `allowed_tools` so the docs and schema consistently show the
user-facing field name.

In `@packages/docs-web/src/content/docs/guides/skills.md`:
- Around line 68-69: The doc uses the camelCase key `allowedTools` but the
workflow YAML expects snake_case `allowed_tools`; update the text string in the
sentence to `allowed_tools` (wherever `allowedTools` appears in this guide) so
it matches the documented schema and examples.

In `@packages/providers/src/community/omp/config.ts`:
- Around line 9-13: The helper stringArray currently collapses empty arrays to
undefined which causes allowlist fields like toolNames to be omitted; update
stringArray so it preserves an explicit empty array (return filtered even when
filtered.length === 0) while still returning undefined when the input is not an
array or contains no string items due to type mismatch; specifically modify the
function stringArray(value: unknown) to return the filtered string[] (possibly
empty) for Array inputs so toolNames: [] remains [] rather than undefined.

In `@packages/providers/src/community/omp/event-bridge.ts`:
- Around line 337-349: The current logic pushes a {kind: 'done'} into queue when
session.prompt() resolves, which can cause early return in the for-await loop
and drop the final result; remove or change the success handler on promptPromise
so it does not enqueue 'done' on resolution, and instead enqueue the terminal
'done' only when the SDK emits its terminal event (e.g., the agent_end/result
finalization handler) that you already subscribe to; specifically update the
session.prompt promise handling around promptPromise and the queue usage so only
rejection pushes {kind: 'error'} from the promptPromise, and ensure the 'done'
item is enqueued by the agent_end or final-result handler that signals
completion (affecting promptPromise, session.prompt, queue, and the for-await
loop).

In `@packages/providers/src/community/omp/mcp.ts`:
- Around line 42-45: The connect step using sdk.MCPManager is not cleaned up on
failure: wrap the call to manager.connectServers(servers,
buildSources(serverNames, resolvedPath)) in a try/catch, and in the catch call
manager.disconnectAll() (or the appropriate cleanup method on sdk.MCPManager)
before rethrowing the error; ensure this occurs after creating manager (new
sdk.MCPManager(cwd, null)) so any partial connections are torn down and the
original error is propagated back to the caller that returns resolvedMcp.

In `@packages/providers/src/community/omp/options-translator.ts`:
- Around line 119-123: The current initialization of base treats an explicit
empty defaults.toolNames as "unset" and falls back to DEFAULT_OMP_TOOL_NAMES,
broadening permissions; change the logic in options-translator.ts so that if
defaults has an explicit toolNames property (use
Object.prototype.hasOwnProperty.call(defaults, 'toolNames') or check
defaults.toolNames !== undefined) you use that array (even if empty), otherwise
fall back to [...DEFAULT_OMP_TOOL_NAMES]; update the declaration that sets base
(the const base = ... line) to follow this presence-check semantics instead of
truthiness/length.

In `@packages/providers/src/community/omp/provider.ts`:
- Around line 60-71: The prompt string in augmentPromptForJsonSchema currently
forces "ONLY a JSON object", which breaks cases where outputFormat.schema may be
an array, string, number, or null; update the prompt to request "valid JSON
matching the schema below" (or equivalent wording) instead of specifying "JSON
object", remove any object-specific language like "Just the raw JSON object",
and ensure the instruction aligns with the parser contract so structuredOutput
can accept arrays, strings, numbers, or null as described by
outputFormat.schema.
- Around line 262-266: getRuntimeAuthOverride is only checking
requestOptions?.env and misses config-scoped env (e.g. assistants.omp.env) so
ensureProviderCredentials fails; update the call site that sets runtimeOverride
(where parsed.provider is used) to merge or pass the config env (parsed.env or
similar) together with requestOptions?.env into getRuntimeAuthOverride, and use
that merged env when calling authStorage.setRuntimeApiKey; apply the same fix
for the other occurrence around resolveSessionModel/ensureProviderCredentials
(lines near 313-315) so provider auth resolution considers config-scoped
environment variables as well.

In `@packages/providers/src/community/omp/session-resolver.ts`:
- Around line 32-38: The code currently lets errors from
sdk.SessionManager.list(...) or sdk.SessionManager.open(...) bubble up and fail
the request; modify the logic in session-resolver.ts to catch exceptions from
the resume path (both the list call and the open call) and fall back to
returning { sessionManager: sdk.SessionManager.create(cwd, dir), resumeFailed:
true } when any error occurs; specifically wrap the SessionManager.list(cwd,
dir) and the subsequent open(match.path, dir) in a try/catch around the
resumeSessionId match branch and on catch return the fresh session from
SessionManager.create with resumeFailed true.

In `@packages/providers/src/community/omp/ui-context-stub.ts`:
- Line 103: The stubbed async function custom<T>() currently returns undefined
as T; replace this with an explicit throw so unsupported calls fail fast —
update the custom<T>() implementation to immediately throw a clear
UnsupportedOperation/Error (e.g., "custom() not supported in UI context stub")
instead of returning undefined; ensure the thrown error is created inside the
async function so callers receive a rejected Promise and adjust any tests that
expected undefined accordingly.

In `@packages/providers/src/mcp-config.ts`:
- Around line 50-58: When `server.env` or `server.headers` is present, validate
they are plain non-null objects (reject arrays/other types) before calling
expandEnvVarsInRecord; replace the existing guards with checks like `server.env
!= null && typeof server.env === 'object' && !Array.isArray(server.env)` (and
similarly for `server.headers`), and if a value is present but fails the check
throw a clear error (e.g. mentioning "invalid shape for server.env" or "invalid
shape for server.headers") instead of silently skipping or passing it into
expandEnvVarsInRecord; keep using expandEnvVarsInRecord and missingVars for the
happy path.

---

Nitpick comments:
In `@packages/providers/src/mcp-config.ts`:
- Line 27: Summary: The structured log event name used in the getLog().warn call
doesn’t follow the required {domain}.{action}_{state} format. Fix: update the
event name string in the getLog().warn call in mcp-config.ts (the line calling
getLog().warn({ key, valueType: typeof val },
'mcp_env_value_coerced_to_string')) to follow the convention, e.g.
'mcp.env_value_coerced_to_string' (domain.action_state), leaving the metadata
object ({ key, valueType: typeof val }) unchanged; ensure any tests or log
consumers expecting the old event name are updated accordingly.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/core/src/config/config-loader.test.ts`:
- Around line 536-538: The test "toSafeConfig exposes only safe OMP defaults"
references a now-undefined mockReadConfigFile; update the test to use the new
file I/O mocks (e.g., replace mockReadConfigFile with mockFsReadFile and call
mockFsReadFile.mockResolvedValue(...) or otherwise wire
mockFsWriteFile/mockFsReadFile consistently) so the test uses the current mocks;
modify the test body where mockReadConfigFile is used to call mockFsReadFile (or
add a small alias const mockReadConfigFile = mockFsReadFile) and ensure the rest
of the test references the same mock symbol.

In `@packages/docs-web/src/content/docs/book/quick-reference.md`:
- Line 143: Update the MCP provider description in the quick-reference table row
for the `mcp` option: change the provider list string that currently reads
"Claude and Oh My Pi" to include "Codex" (e.g., "Claude, Codex, and Oh My Pi")
so the `mcp` table entry matches the rest of the docs.

In `@packages/providers/src/claude/provider.ts`:
- Around line 222-228: The replacement currently done in result[key] via
val.replace(...) only matches $VAR and ignores ${VAR} syntax; update the regex
used in the val.replace call to recognize both $VAR and ${VAR} forms (capture
the variable name into varName), then look up process.env[varName], push into
missingVars when undefined, and return envVal ?? '' so both placeholder styles
are expanded; keep the existing symbols result[key], val.replace, missingVars
and process.env in the fix so the logic and error collection remain unchanged.

In `@packages/providers/src/community/omp/event-bridge.ts`:
- Around line 137-139: The parseToolInput function currently accepts arrays as
objects which can break the expected Record<string, unknown> shape for
toolInput; update parseToolInput to only return args when it is a non-null,
non-array plain object (e.g., typeof args === 'object' && args !== null &&
!Array.isArray(args')), otherwise return an empty object so callers using
toolInput get a safe Record<string, unknown>; refer to parseToolInput and any
places that consume toolInput to verify they now receive a properly-guarded
object.

In `@packages/providers/src/community/omp/options-translator.ts`:
- Around line 341-359: envFlagEnabled currently reads only process.env so
request-scoped CLAUDE_CODE_USE_FOUNDRY in the supplied env is ignored; update
the logic so the Foundry flag is read from the same request env passed into
getRuntimeAuthOverride. Either change envFlagEnabled to accept an optional env:
Record<string,string>|undefined and prefer env[envName] (falling back to
process.env[envName]) or, inside getRuntimeAuthOverride, replace
envFlagEnabled('CLAUDE_CODE_USE_FOUNDRY') with a call that checks the flag via
findEnvValue(['CLAUDE_CODE_USE_FOUNDRY'], env) (normalizing the returned value
the same way) before returning findEnvValue(['ANTHROPIC_FOUNDRY_API_KEY',
...OMP_PROVIDER_ENV_VARS.anthropic], env); ensure you reference envFlagEnabled,
getRuntimeAuthOverride, findEnvValue, CLAUDE_CODE_USE_FOUNDRY and
ANTHROPIC_FOUNDRY_API_KEY when making the change.

In `@packages/providers/src/community/omp/provider.test.ts`:
- Around line 842-845: The test uses a setTimeout-based 10ms race to detect the
second prompt entering (secondPromptEntered / resolveSecondPromptEntered), which
makes the assertion flaky; replace that timing-based approach with explicit
synchronization by wiring the test to a deterministic barrier: create a Promise
that you resolve from the exact code path where the second prompt is
enqueued/handled (e.g., the callback or mock that represents "second prompt
entered" used by the provider under test) and remove the setTimeout; update the
spots around secondPromptEntered/resolveSecondPromptEntered (and the similar
block at 898-900) so the test awaits that Promise instead of racing with
setTimeout, ensuring the promise is resolved when the provider's second prompt
handler is invoked.

In `@packages/providers/src/community/omp/provider.ts`:
- Around line 98-113: acquireConfigEnvLease currently queues waiters in
configEnvWaiters without any abort handling, so pass the caller's abort signal
(requestOptions.abortSignal) into the function and store it on the waiter
object, attach an abort event listener that removes the waiter from
configEnvWaiters and rejects the Promise with an AbortError if triggered, and
ensure the listener is removed when the waiter resolves (both in the resolve
callback pushed into configEnvWaiters and inside
createReaderRelease/createWriterRelease cleanup) so cancelled callers are
removed and don't later acquire the lease; also check signal.aborted before
pushing a waiter to short-circuit immediate aborts and ensure
flushConfigEnvWaiters still works with the augmented waiter shape.

In `@packages/providers/src/community/omp/sdk-loader.ts`:
- Around line 3-117: The file declares local duplicates of the OMP SDK types
(e.g., OmpAuthStorage, OmpModelRegistry, OmpMcpManager,
OmpCreateAgentSessionOptions/Result, OmpCodingAgentSdk) which risks drifting
from the real SDK surface; remove these local interface/type declarations and
instead use "import type" to pull the official types from
`@oh-my-pi/pi-coding-agent` (e.g., SessionManager, MCPManager,
CreateAgentSessionOptions, CreateAgentSessionResult, and exported extension
APIs), then update any structural casts and variable signatures in sdk-loader.ts
to use the imported SDK types and keep only Archon-specific adapter types/glue
in this file.

In `@packages/providers/src/community/omp/session-resolver.ts`:
- Around line 43-59: The code wraps non-missing errors from
sdk.SessionManager.open twice (both in the inner catch and again in the outer
catch), producing duplicated "Oh My Pi session resume failed..." text; to fix,
change the inner catch (around sdk.SessionManager.open) to rethrow the original
error (throw error) instead of throwing a new Error, or alternatively remove the
inner wrapping so only the outer catch constructs the new Error; target the
catch blocks that reference sdk.SessionManager.open, isMissingSessionError and
resumeSessionId and ensure only one place creates the `new Error('Oh My Pi
session resume failed...')`.

---

Nitpick comments:
In `@packages/providers/package.json`:
- Line 24: The test script in package.json contains a duplicated entry for
src/community/omp/mcp-config.test.ts which causes the same test to run twice;
edit the "test" script string to remove the redundant
src/community/omp/mcp-config.test.ts occurrence so each test path appears only
once (inspect the "test" field in packages/providers/package.json and remove the
duplicate entry for src/community/omp/mcp-config.test.ts).

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/providers/src/claude/provider.test.ts`:
- Around line 101-120: The test that manipulates
process.env.TEST_CLAUDE_MCP_TOKEN should preserve and restore any pre-existing
value instead of always deleting it; update the test 'expands ${VAR}
placeholders in env and headers' to capture the original value (e.g. const prev
= process.env.TEST_CLAUDE_MCP_TOKEN) before setting it, and in the finally block
restore it by setting process.env.TEST_CLAUDE_MCP_TOKEN = prev when prev is
defined or deleting the variable only when prev is undefined (delete
process.env.TEST_CLAUDE_MCP_TOKEN); this ensures deterministic tests and avoids
leaking environment state.