Skip to content

fix(providers): update package imports from @mariozechner to @earendil-works#1732

Open
jkbjhs-potluck wants to merge 1 commit into
coleam00:devfrom
jkbjhs-potluck:fix/pi-provider-github-repo
Open

fix(providers): update package imports from @mariozechner to @earendil-works#1732
jkbjhs-potluck wants to merge 1 commit into
coleam00:devfrom
jkbjhs-potluck:fix/pi-provider-github-repo

Conversation

@jkbjhs-potluck
Copy link
Copy Markdown

@jkbjhs-potluck jkbjhs-potluck commented May 20, 2026

Summary

Describe this PR in 2-5 bullets:

  • Problem: Archon v0.3.12 bundles the old @mariozechner/* Pi packages, whose embedded model catalog does not include openai-codex/gpt-5.5.
  • Why it matters: provider: pi workflows using model: openai-codex/gpt-5.5 fail before execution with Pi model not found, even though current upstream Pi supports the model.
  • What changed: Updated Archon's Pi integration to @earendil-works/pi-ai / @earendil-works/pi-coding-agent ^0.75.3, refreshed bun.lock, updated Pi imports/docs/tests, and added a regression test that resolves openai-codex/gpt-5.5 through Pi's ModelRegistry.
  • What did not change (scope boundary): No provider-selection behavior, workflow YAML schema, database schema, platform adapter behavior, or non-Pi providers were intentionally changed.

UX Journey

Before

User                   Archon PiProvider                Embedded Pi catalog
────                   ─────────────────                ───────────────────
runs workflow ───────▶ parses provider/model
provider: pi           creates ModelRegistry ─────────▶ only knows older
model: gpt-5.5                                      openai-codex entries
                       lookup fails ◀─────────────── missing gpt-5.5
sees failure ◀──────── throws "Pi model not found"

After

User                   Archon PiProvider                Embedded Pi catalog
────                   ─────────────────                ───────────────────
runs workflow ───────▶ parses provider/model
provider: pi           creates ModelRegistry ─────────▶ [current @earendil]
model: gpt-5.5                                      [catalog includes gpt-5.5]
                       lookup succeeds ◀──────────── resolves openai-codex/gpt-5.5
workflow continues ◀── creates Pi session with model

Architecture Diagram

Before

@archon/workflows
      │ sends SendQueryOptions(model="openai-codex/gpt-5.5")
      ▼
@archon/providers/community/pi
      ├── provider.ts ──dynamic import──▶ @mariozechner/pi-coding-agent@0.67.5
      ├── event-bridge.ts ──types──────▶ @mariozechner/pi-ai@0.67.5
      ├── options-translator.ts ───────▶ @mariozechner/pi-coding-agent APIs
      ├── resource-loader.ts ──────────▶ DefaultResourceLoader(old API)
      └── ui-context-stub.ts ──────────▶ ExtensionUIContext(old shape)
                                      │
                                      ▼
                              stale ModelRegistry catalog
                              missing openai-codex/gpt-5.5

After

@archon/workflows
      │ sends SendQueryOptions(model="openai-codex/gpt-5.5")
      ▼
@archon/providers/community/pi [~]
      ├── provider.ts [~] ──dynamic import===▶ @earendil-works/pi-coding-agent@0.75.3 [+]
      ├── event-bridge.ts [~] ──types========▶ @earendil-works/pi-ai@0.75.3 [+]
      ├── options-translator.ts [~] ─────────▶ SDK tool-name/customTools API [~]
      ├── resource-loader.ts [~] ────────────▶ DefaultResourceLoader(agentDir required) [~]
      ├── ui-context-stub.ts [~] ────────────▶ ExtensionUIContext(new methods) [~]
      └── model-catalog.test.ts [+] ─────────▶ ModelRegistry regression for gpt-5.5 [+]
                                      │
                                      ▼
                              current ModelRegistry catalog
                              resolves openai-codex/gpt-5.5

Connection inventory (list every module-to-module edge, mark changes):

From To Status Notes
packages/providers/package.json @earendil-works/pi-ai new Replaces direct @mariozechner/pi-ai dependency.
packages/providers/package.json @earendil-works/pi-coding-agent new Replaces direct @mariozechner/pi-coding-agent dependency.
packages/providers/package.json @mariozechner/pi-ai removed Old direct dependency removed.
packages/providers/package.json @mariozechner/pi-coding-agent removed Old direct dependency removed.
PiProvider.provider.ts @earendil-works/pi-coding-agent modified Dynamic import now loads renamed/current Pi SDK.
event-bridge.ts @earendil-works/pi-coding-agent / @earendil-works/pi-ai modified Type imports updated to renamed packages.
options-translator.ts @earendil-works/pi-coding-agent modified Uses createBashToolDefinition plus customTools for env-aware bash under the new SDK API.
resource-loader.ts DefaultResourceLoader modified Supplies required agentDir: getAgentDir() for the new SDK API.
ui-context-stub.ts ExtensionUIContext modified Adds no-op implementations for new UI context methods required by current Pi.
model-catalog.test.ts ModelRegistry new Regression test asserts openai-codex/gpt-5.5 resolves.
Docs/examples Pi package names modified User-facing install/package references now point to @earendil-works/*.

Label Snapshot

  • Risk: risk: medium
  • Size: size: M
  • Scope: dependencies, tests, docs
  • Module: providers:pi

Change Metadata

  • Change type: bug
  • Primary scope: multi

Linked Issue

Validation Evidence (required)

Commands and result summary:

bun install
bun --filter @archon/providers type-check
bun --filter @archon/providers test
bun run validate
TARGET=bun-darwin-arm64 OUTFILE=dist/binaries/archon-darwin-arm64 bash scripts/build-binaries.sh
install -m 755 dist/binaries/archon-darwin-arm64 /opt/homebrew/bin/archon
archon version
  • Evidence provided (test/log/trace/screenshot):
    • bun install completed and refreshed bun.lock to @earendil-works/pi-ai@0.75.3 and @earendil-works/pi-coding-agent@0.75.3.
    • bun --filter @archon/providers type-check exited with code 0.
    • bun --filter @archon/providers test exited with code 0.
    • bun run validate exited with code 0 after bundled checks, type-check, lint, format check, and package-isolated tests.
    • TARGET=bun-darwin-arm64 OUTFILE=dist/binaries/archon-darwin-arm64 bash scripts/build-binaries.sh produced dist/binaries/archon-darwin-arm64 (~68 MB).
    • Installed the built binary to /opt/homebrew/bin/archon and verified archon version reports Archon CLI v0.3.12, Build: binary, Git commit: 5283ea93.
  • If any command is intentionally skipped, explain why: None.

Security Impact (required)

  • New permissions/capabilities? (Yes/No): No
  • New external network calls? (Yes/No): No
  • Secrets/tokens handling changed? (Yes/No): No
  • File system access scope changed? (Yes/No): No
  • If any Yes, describe risk and mitigation: N/A. This updates the bundled Pi SDK package namespace/version and adapts to its API; it does not add new Archon runtime permissions, network calls, secret stores, or filesystem scopes.

Compatibility / Migration

  • Backward compatible? (Yes/No): Yes
  • Config/env changes? (Yes/No): No
  • Database migration needed? (Yes/No): No
  • If yes, exact upgrade steps: No database or config migration is required. Users only need the updated Archon build/binary to receive the refreshed embedded Pi catalog.

Human Verification (required)

What was personally validated beyond CI:

  • Verified scenarios:
    • Confirmed current @earendil-works/pi-coding-agent ModelRegistry resolves openai-codex/gpt-5.5.
    • Added and ran packages/providers/src/community/pi/model-catalog.test.ts covering that resolution.
    • Built a local Darwin arm64 Archon binary from this branch.
    • Installed that built binary to /opt/homebrew/bin/archon and verified archon version runs from PATH.
  • Edge cases checked:
    • Existing Pi lazy-load regression still passes; registering/instantiating the provider does not eagerly load Pi SDK packages.
    • Env-aware bash passthrough still has coverage after adapting from direct tool objects to Pi's current tools/customTools API.
    • Extension UI context remains headless-safe by adding no-op implementations for new current-SDK UI methods.
  • What was not verified:
    • A live remote LLM call to openai-codex/gpt-5.5 was not executed; validation is limited to catalog/model resolution and provider integration tests.

Side Effects / Blast Radius (required)

  • Affected subsystems/workflows:
    • Pi community provider (packages/providers/src/community/pi/*).
    • Provider package dependency graph and lockfile.
    • Pi-related docs/examples and test references.
    • Compiled Archon binaries that embed provider dependencies.
  • Potential unintended effects:
    • Current Pi SDK API changes could affect less common extension/UI/tool paths not covered by tests.
    • Lockfile dependency churn may affect binary bundle size or transitive package resolution.
    • Env-aware bash override now uses Pi's current customTools hook rather than passing constructed tool objects directly.
  • Guardrails/monitoring for early detection:
    • Full bun run validate passes.
    • New ModelRegistry regression catches stale catalogs that drop openai-codex/gpt-5.5.
    • Existing provider, lazy-load, event bridge, options translator, and session resolver tests cover the Pi integration seam.

Rollback Plan (required)

  • Fast rollback command/path: Revert this PR/commit to restore @mariozechner/* Pi dependencies and previous provider integration code, then rebuild/reinstall the Archon binary.
  • Feature flags or config toggles (if any): None.
  • Observable failure symptoms:
    • Pi workflows fail during provider startup/import.
    • provider: pi + model: openai-codex/gpt-5.5 again reports Pi model not found.
    • Pi extension workflows fail with UI context or resource loader method/type errors.

Risks and Mitigations

List real risks in this PR (or write None).

  • Risk: Updating Pi from the old @mariozechner/* packages to current @earendil-works/* packages changes SDK APIs used by the provider.
    • Mitigation: Updated affected integration points (DefaultResourceLoader, ExtensionUIContext, tool restriction/env passthrough handling) and ran provider tests plus full bun run validate.
  • Risk: The regression proves catalog resolution but not a live openai-codex/gpt-5.5 API call.
    • Mitigation: The original failure occurs before network execution at ModelRegistry.find; the new regression directly covers that failure point.

Summary by CodeRabbit

  • Chores

    • Migrated package dependencies from one provider to another.
  • Tests

    • Added model catalog test coverage.
    • Updated test assertions to reflect new dependency structure.
  • Documentation

    • Updated installation and package references in guides and documentation.
  • New Features

    • Enhanced tool handling mechanism with improved customization support.
    • Extended UI context stub with additional placeholder methods.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 106110cd-8dc2-43d4-840f-98750c421b49

📥 Commits

Reviewing files that changed from the base of the PR and between 5283ea9 and 97655b8.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (18)
  • CLAUDE.md
  • examples/workflows/README.md
  • packages/cli/src/commands/setup.test.ts
  • packages/docs-web/src/content/docs/getting-started/ai-assistants.md
  • packages/providers/package.json
  • packages/providers/src/community/pi/event-bridge.test.ts
  • packages/providers/src/community/pi/event-bridge.ts
  • packages/providers/src/community/pi/model-catalog.test.ts
  • packages/providers/src/community/pi/options-translator.ts
  • packages/providers/src/community/pi/provider-lazy-load.test.ts
  • packages/providers/src/community/pi/provider.test.ts
  • packages/providers/src/community/pi/provider.ts
  • packages/providers/src/community/pi/resource-loader.ts
  • packages/providers/src/community/pi/session-resolver.test.ts
  • packages/providers/src/community/pi/session-resolver.ts
  • packages/providers/src/community/pi/ui-context-stub.ts
  • packages/providers/src/types.ts
  • scripts/build-binaries.sh

📝 Walkthrough

Walkthrough

This PR migrates the Pi community provider from the @mariozechner npm scope to @earendil-works, updates dependencies to v0.75.3, refactors tool resolution to return tool names with optional environment-aware bash injection via customTools, and adapts tests and documentation accordingly.

Changes

Pi SDK Migration and Tool Wiring

Layer / File(s) Summary
Package dependencies and scope migration
packages/providers/package.json
Dependencies updated from @mariozechner/pi-ai and @mariozechner/pi-coding-agent to @earendil-works/* at ^0.75.3. Test script extended with binary-resolver dev tests.
Tool resolution interface and core implementation
packages/providers/src/community/pi/options-translator.ts
ResolvedTools.tools changed from PiTool[] to PiToolName[], added customTools?: PiCustomTools field. New buildEnvAwareBashTool helper creates bash tool definitions with optional spawn hooks. resolvePiTools returns tool names and conditionally supplies env-aware bash override.
Provider lazy-loading and tool wiring integration
packages/providers/src/community/pi/provider.ts, packages/providers/src/community/pi/resource-loader.ts, packages/providers/src/community/pi/session-resolver.ts, packages/providers/src/community/pi/event-bridge.ts, packages/providers/src/community/pi/ui-context-stub.ts
Provider imports switched to @earendil-works/pi-coding-agent. PiProvider.sendQuery() destructures and passes customTools to createAgentSession. Resource loader imports getAgentDir and passes it explicitly. UI context stub adds setWorkingVisible, setWorkingIndicator, addAutocompleteProvider and editor component methods.
Test suite adaptations for tool representation and package scope
packages/providers/src/community/pi/provider.test.ts, packages/providers/src/community/pi/provider-lazy-load.test.ts, packages/providers/src/community/pi/session-resolver.test.ts, packages/providers/src/community/pi/event-bridge.test.ts, packages/providers/src/community/pi/model-catalog.test.ts
Tests mock @earendil-works packages. provider.test.ts adds createBashToolDefinition mock, treats tools as string[], and asserts customTools presence for env-aware bash. Lazy-load test checks flags remain false to prevent eager module resolution. New model catalog test verifies ModelRegistry resolves openai-codex/gpt-5.5.
Documentation and configuration updates
CLAUDE.md, examples/workflows/README.md, packages/docs-web/src/content/docs/getting-started/ai-assistants.md, packages/providers/src/types.ts, scripts/build-binaries.sh
Package scope updated from @mariozechner to @earendil-works across all docs and examples. Author attribution updated in code comments.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

  • coleam00/Archon#965: Directly addresses the Pi provider session/event/tool integration implementation that this PR adapts to the new SDK scope and tool wiring.
  • coleam00/Archon#1731: Addressed by the stale Pi model catalog problem—PR updates Pi package to v0.75.3 and adds ModelRegistry test for openai-codex/gpt-5.5 resolution.

Possibly related PRs

  • coleam00/Archon#1284: Modifies PiProvider.sendQuery() with overlapping changes to Pi SDK imports and model/tool/auth handling.
  • coleam00/Archon#1296: Directly overlaps on resolvePiTools env-injection wiring and customTools return shape in options-translator.ts and provider.ts.
  • coleam00/Archon#1270: Builds on the Pi provider implementation by adapting existing tool-wiring and provider code to the new SDK scope.

Poem

🐰 From Mariozechner's nest to Earendil's domain,
The Pi SDK hops with customTools in tow—
Tool names dance freely, bash gets its special say,
Tests align their assertions with the new way,
Docs whisper the author's name anew today! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly and concisely summarizes the main change: updating package imports from @mariozechner to @earendil-works for the Pi provider.
Description check ✅ Passed The PR description comprehensively follows the template structure with all required sections: Summary, UX Journey, Architecture Diagram, Label Snapshot, Change Metadata, Linked Issue, Validation Evidence, Security Impact, Compatibility, Human Verification, Side Effects, and Rollback Plan.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 97655b8d68

ℹ️ 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".

Comment on lines +188 to +189
tools: [...PI_DEFAULT_TOOL_NAMES],
...(customBashTool ? { customTools: [customBashTool] } : {}),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid narrowing tool allowlist in env-only path

When requestOptions.env is set and no allowed_tools/denied_tools are provided, this branch now sends tools: ["read","bash","edit","write"] to Pi. In the new @earendil-works/pi-coding-agent API, tools is a global allowlist (including extension/custom tools), so this unintentionally disables extension-registered tools by default whenever env injection is used. That regresses extension workflows (which are enabled by default in PiProvider) even though the user did not request tool restriction.

Useful? React with 👍 / 👎.

@Wirasm
Copy link
Copy Markdown
Collaborator

Wirasm commented May 21, 2026

PR Review Summary — Multi-Agent

Six specialized reviewers (code, docs-impact, tests, types, comments, silent-failure) ran against this PR. Migration mechanics are sound; findings are concentrated around (a) one missed doc file, (b) test-assertion gaps for the new customTools channel, and (c) one stale JSDoc count claim.

Critical Issues (0)

None. The SDK migration is mechanically correct, the lazy-load guarantee is preserved, the as unknown as PiCustomTools[number] cast is justified, and the env-aware bash override flows through the meaningful code paths.

Important Issues (5)

# Agent Issue Location
I1 code-reviewer / docs-impact .env.example:41 still references @mariozechner/pi-coding-agent — the only stale package-name reference on the PR branch (verified via git grep on refs/pull/1732/head; CHANGELOG entries at lines 142/151 are historical and should stay) .env.example:41
I2 comment-analyzer ensurePiPackageDirShim() JSDoc says Pi reads "three optional fields" — @earendil-works/pi-coding-agent@0.75.3 dist/config.js now reads four (piConfig.name, piConfig.configDir, version, name). Stub already supplies all four so behavior is correct, but the count is wrong packages/providers/src/community/pi/provider.ts:80-82
I3 pr-test-analyzer options-translator.test.ts never asserts on result.customTools — the pure function driving the new dual-channel API has zero unit coverage of its new return field. Three branches uncovered: (no-restrict + env), (allow-list w/ bash + env), (allow-list w/o bash + env) packages/providers/src/community/pi/options-translator.test.ts:137-153
I4 pr-test-analyzer / code-reviewer provider.test.ts "allowed_tools includes bash + env" test verifies the spawnHook was constructed but doesn't assert callArgs.customTools reached createAgentSession. Asymmetric with the no-restriction-with-env test which does assert it packages/providers/src/community/pi/provider.test.ts:993-1015
I5 silent-failure-hunter denied_tools: ['bash'] + codebase env configured silently drops env injection: no log, no system chunk, no diagnostic. User has no way to know their managed env vars aren't reaching any subprocess. No test covers this combination either packages/providers/src/community/pi/options-translator.ts:225-235

Suggestions (4)

# Agent Suggestion Location
S1 pr-test-analyzer Add loaderArgs?.agentDir assertion to verify the new required DefaultResourceLoader field is threaded through — currently no test would catch its accidental removal packages/providers/src/community/pi/provider.test.ts (loader assertion test)
S2 pr-test-analyzer Add explicit test for denied_tools: ['bash'] + env present asserting callArgs.customTools is undefined and tools excludes bash — guards I5 above packages/providers/src/community/pi/provider.test.ts
S3 silent-failure-hunter Add a minimal runtime shape assertion (e.g. typeof tool.name === 'string') before the as unknown as PiCustomTools[number] cast — converts silent SDK drift into a loud, actionable error packages/providers/src/community/pi/options-translator.ts:125-129
S4 comment-analyzer / type-design-analyzer Add a one-line JSDoc on the PiCustomTools alias explaining the derivation-from-SDK rationale (the removed PiTool alias had equivalent docs) packages/providers/src/community/pi/options-translator.ts:118

Strengths

  • model-catalog.test.ts is a meaningful regression test for Pi provider binary embeds stale model catalog; openai-codex/gpt-5.5 fails #1731. It exercises the exact failure point (ModelRegistry.find) reported in the issue and asserts both provider and id fields — cannot produce a false positive from a stub/fallback.
  • Lazy-load regression test (provider-lazy-load.test.ts) still meaningfully protects the binary-startup bug; the package-name update preserves the detection contract.
  • The as unknown as PiCustomTools[number] cast in buildEnvAwareBashTool is honest: an explanatory comment names the SDK variance root cause, doesn't reach for as any, and is structurally accurate (verified against @earendil-works/pi-coding-agent@0.75.3 bash.d.ts and sdk.d.ts).
  • The pre-existing "Do NOT add static imports" header block in provider.ts and the provider-lazy-load.test.ts file-header comment remain accurate after the rename — both correctly cite @earendil-works/pi-coding-agent/dist/config.js line 346 (readFileSync(getPackageJsonPath(), 'utf-8') at module top level).
  • Build-script bytecode comment remains valid: Bun 1.3.11 is still in use, and the CJS/ESM interop shape in the graph (via jiti transitive dep) persists in the new package.
  • Type-design ratings: encapsulation 7/10, expression 4/10, usefulness 7/10, enforcement 4/10. Low scores explained by SDK-imposed limits (customTools[number] element type doesn't constrain name) — Archon can't fix these without upstream changes.

Documentation

  • .env.example:41 is the only missed reference (see I1). CHANGELOG entries at lines 142/151 are historical records and should not be rewritten.
  • Version string 0.67.5 does not appear anywhere outside the lockfile.

Verdict

READY TO MERGE WITH MINOR FIXES — no production bugs found. I1 (.env.example) is the only fix that is strictly user-facing. I2–I5 are quality improvements (one stale field count, three test-assertion gaps, one silent-degradation path under a specific config). The merge can proceed safely; addressing I5 in a follow-up would close the only user-observable diagnostic gap.

Recommended Actions

  1. Update .env.example:41 package reference (I1)
  2. Fix the "three fields" → "four fields" claim in provider.ts shim JSDoc and add name to the list (I2)
  3. Add customTools assertions to the three uncovered branches in options-translator.test.ts (I3) — single-file change, no production code touched
  4. Mirror the existing callArgs.customTools assertion into the explicit-allow-list test in provider.test.ts (I4)
  5. Consider surfacing a warning when env is configured but bash is denied/excluded (I5) — either log, add a warnings: string[] field to ResolvedTools, or emit a system chunk

@Wirasm
Copy link
Copy Markdown
Collaborator

Wirasm commented May 22, 2026

Review Summary

Verdict: minor-fixes-needed

This PR migrates the Pi community provider from @mariozechner/* to @earendil-works/* and upgrades to Pi v0.75.3, refactoring the tools API to pass PiToolName[] strings + a customTools override array instead of constructed Tool objects. The implementation is solid and well-tested. Four medium-priority items need attention before merge.

Blocking issues

None.

Suggested fixes

  • packages/providers/src/community/pi/options-translator.test.ts:~137 — Add assertions for result.customTools in the env-aware bash test. The test already verifies tools.length and unknownTools but doesn't check that customTools is present with the correct override:

    expect(result.customTools).toBeDefined();
    expect(result.customTools).toHaveLength(1);
    expect(result.customTools![0].name).toBe('bash');
  • options-translator.ts — buildEnvAwareBashTool() — Add unit-level coverage for this helper. Three cases to cover:

    • resolvePiTools(cwd, { allowed_tools: ['bash'] }, { FOO: 'bar' })customTools present, name 'bash'
    • resolvePiTools(cwd, { allowed_tools: ['read'] }, { FOO: 'bar' })customTools undefined (bash not in allowlist)
    • resolvePiTools(cwd, { denied_tools: ['bash'] }, { FOO: 'bar' })customTools undefined (bash explicitly denied)

Minor / nice-to-have

  • packages/providers/src/community/pi/provider.test.ts:965 — Stale comment: // Env present → we override Pi's built-in codingTools so bash sees the env. → Update to // Env present → pass explicit default tool names plus an env-aware bash override.

  • packages/providers/src/community/pi/provider.test.ts:1025 — Stale comment: // Every createBashTool call in this test path is either (cwd) or (cwd, undefined). → Update to // No env means no env-aware bash override is needed. (note: function was also renamed from createBashTool to createBashToolDefinition)

  • packages/providers/src/community/pi/ui-context-stub.ts:141–175 — Add a grouped doc comment above the four new stub methods (setWorkingVisible, setWorkingIndicator, addAutocompleteProvider, getEditorComponent) explaining that these are PI v0.75.3 API additions and that no-op stubs are intentional for Archon's headless execution context.

Compliments

  • The dynamic-import architecture in provider.ts is well-explained — the module header comment correctly captures the failure mode (compiled binary crash) and why type-only imports are safe. This kind of non-obvious "why" comment is exactly what the codebase needs.
  • The ensurePiPackageDirShim docstring clearly documents idempotency and why the shim is built per-sendQuery rather than at module load.
  • The buildBashSpawnHook docstring explains the env-merge precedence and the empty-env early return — useful for future maintainers.

Reviewed via maintainer-review-pr workflow (Pi/Minimax). Aspects run: code-review, error-handling, test-coverage, comment-quality, docs-impact.

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.

Pi provider binary embeds stale model catalog; openai-codex/gpt-5.5 fails

2 participants