Skip to content

Fix bundled Claude and Codex usage with isolated provider state - #2623

Merged
integry merged 2 commits into
2547/claude-opus-5-bundle-agent-tank-into-th-20260926-1849-t18from
fix/pr2555-live-usage
Sep 29, 2026
Merged

integry merged 2 commits into
2547/claude-opus-5-bundle-agent-tank-into-th-20260926-1849-t18from
fix/pr2555-live-usage

Conversation

@integry

@integry integry commented Sep 29, 2026

Copy link
Copy Markdown
Owner

Bundled Agent Tank returned null Claude/Codex usage despite passing the mocked suites. Codex's app-server failed to initialize its SQLite state under the read-only credential mount; Claude's interactive path expected configuration absent from that mount.

Prepare private writable provider homes inside the disposable container, copying only Claude's .credentials.json and Codex's auth.json. Enable Agent Tank's existing direct Claude usage API. Keep original host mounts read-only, exclude host sessions/databases/plugins/MCP configuration, and leave AGY's working path unchanged. Credential copies have mode 0600 in private mode-0700 directories and are removed with the container. The preparation script is included in both image-content hash lists and used by production and the live verifier.

This PR targets #2555's branch so it can be incorporated before that PR merges. Agent Tank remains pinned to 0.9.11; no Agent Tank source or release change is required.

Validation:

  • 92 focused checks passed across runtime isolation, bundled runner, image verifier, image supply chain, version management, and build scripts. Updated verifier assertions were rerun after wording changes.
  • Core build/typecheck and targeted ESLint passed; full agent image rebuilt successfully.
  • Strict live verification of the rebuilt image passed for Claude, Codex, and AGY together, requiring numeric usage for every provider, execution as node, read-only host mounts, and an unchanged credential manifest.
  • An earlier run also returned usage for all three, but the verifier detected concurrent changes in the actively used host Codex directory. The definitive run used private credential snapshots, mounted read-only and deleted afterward, to isolate the test from other local processes.
  • Tests verify that simulated credential refreshes/SQLite writes cannot change source credentials, unrelated host configuration is excluded, AGY retains its path, missing credentials stay missing, and failed setup cleans private copies without logging credentials.

No staging deployment was performed. The API and matching agent image must be rolled out together because the updated runner invokes the new image helper.

@integry integry added the AI label Sep 29, 2026
@integry

integry commented Sep 29, 2026

Copy link
Copy Markdown
Owner Author

/review

@propr-dev

propr-dev Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

✅ AI Code Review Complete requested by @integry

Posted 1 review:

View Task Details

@propr-dev

propr-dev Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

🔍 AI Code Review — codex:gpt-6-astra

Overall Evaluation

The PR addresses the provider-home failures with a small, shared preparation helper. It needs a localized packaging correction before merge.

✅ Private provider state — The helper copies only the two authentication files, sets directory/file permissions to 0700/0600, and preserves AGY’s configuration path.

✅ Consistent execution — Production and both verifier modes invoke the preparation helper before Agent Tank.

✅ Safe failure reporting — Preparation failures remove the runtime directory and emit a generic message without configuration contents.

The supplied current-head status reports 20 passed checks and no failed or pending checks. This review used static analysis only, as requested.

Merge blockers

Every finding below was introduced by this PR and must be resolved before merging.

F1: 🔴 Include the helper in app packaging

  • Required behavior: The new image-content hash input must be available to both the build script and the production backend, so they compute the same agent bundle identity.

  • Evidence: packages/core/src/agents/version/types.ts:57 — addition to AGENT_BUNDLE_CONTENT_FILES.

    1. The PR creates scripts/agent-tank-runtime.mjs and adds it to both hash lists.
    2. The source-side build script includes its contents when generating the agent bundle tag.
    3. The production app packages those inputs through docker/Dockerfile.app.prod; this PR does not add a copy instruction for the newly created helper.
    4. In the packaged backend, computeContentHash skips the missing file, producing a different content hash from the source-side build and therefore a different bundle identity.

    The existing missing-file handling only logs and skips; it does not preserve hash parity. The supplied test/agentBundleContentFiles.test.ts:57–65 explicitly requires a COPY instruction for every listed input, so the new entry also violates that packaging assertion. The green check summary does not establish that this particular assertion ran.

    static trace: the added hash entry and complete changed-file list, together with the supplied hash consumer and packaging-test contract. No commands were executed.

  • Minimum fix: Add the corresponding COPY scripts/agent-tank-runtime.mjs ... instruction to docker/Dockerfile.app.prod, preserving the path used by the backend’s hash computation, and run the existing bundle-content tests.

Suggestions

These are optional follow-ups and are not sent to /fix.

S1: 🟢 Exercise cleanup after a successful copy

The failure test supplies an invalid first entry, so it verifies directory cleanup without first creating a credential copy. An additional case with a valid first provider and an invalid second provider would directly verify that already-copied credentials are removed. This is optional because the existing catch block statically covers that cleanup.

Score

The runtime change is focused and implements the requested isolation well, but the missing production packaging update leaves the new hash contract inconsistent. The supplied checks are green; the packaging correction remains necessary.

Score: 6/10


🤖 Review Details

  • Model: GPT-6 Astra
  • Time: 35s
  • Tokens: 48,830 (47,868 in / 962 out)
  • Cost: $0.42

View Task


💡 Next step: Comment /fix to address every F# merge blocker, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them, and they never relax a merge blocker.

🤖 Review by ProPR

@integry

integry commented Sep 29, 2026

Copy link
Copy Markdown
Owner Author

/ultrafix

@propr-dev propr-dev Bot added the ultrafix label Sep 29, 2026
@propr-dev

propr-dev Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

🔄 Ultrafix loop started (goal: 8/10, max cycles: 10)

First action: /fix

💡 Tip: Remove the ultrafix label from this PR to stop further ultrafix cycles.

@propr-dev

propr-dev Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

✅ Applied the requested follow-up changes in commit 84aa66d

Addressed 1 AI review comment (IDs: 5889233305)

Addressed finding F1

Summary of Changes

Addressed F1 only: added the helper COPY to the production app’s runtime stage at the expected hash path. Strengthened the packaging regression to verify runtime-stage placement and destinations.

All 14 bundle-content and supply-chain tests passed; git diff --check passed. No visual preview was needed.


🤖 Implementation Details

  • Model: GPT-6 Astra (high)
  • Requested By: @integry
  • Time: 2m 25s
  • Tokens: 153,615 (151,690 in / 1,925 out)
  • Cost: $0.50

Undo Changes • View Task Execution


💡 ProPR Slash Commands
Command Description Example
/merge Merge target branch into this PR and resolve conflicts /merge
/review Request an AI code review /review or /review claude-sonnet
/fix Implement fixes for issues found by /review /fix or /fix address the null check issue
/switch Change the AI model for this PR /switch claude-opus
/use Override the model for a single follow-up run /use claude-sonnet
/ultrafix Loop review→fix cycles until score goal is met /ultrafix or /ultrafix goal=8 max=10

🤖 Generated by ProPR
Processing comment ID: 5889350886✓

…app’s runtime stage at the expected hash path. Strengthened the packaging regression to verify runtime-stage placement and destinations.

Addressed F1 only: added the helper COPY to the production app’s runtime stage at the expected hash path. Strengthened the packaging regression to verify runtime-stage placement and destinations.

All 14 bundle-content and supply-chain tests passed; `git diff --check` passed. No visual preview was needed.

PR: #2623
Comment by: @integry (ID: 5889350886)
Model: gpt-6-astra
@propr-dev

propr-dev Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

✅ AI Code Review Complete requested by @propr-ultrafix

Posted 1 review:

View Task Details

@propr-dev

propr-dev Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

🔍 AI Code Review — codex:gpt-6-astra

Overall Evaluation

Ready to merge within scope. The change addresses the reported provider initialization failures with a small, shared runtime preparation helper.

✅ Private provider state — Claude and Codex receive only their authentication files in mode-0700 directories, with copies restricted to mode 0600. AGY retains its existing configuration path.

✅ Consistent image integration — Production and verifier bootstraps invoke the helper; both image-content hash lists and the production build context include it.

✅ Controlled failure behavior — Preparation errors remove the private runtime directory and produce a fixed error message without exposing configuration.

This assessment uses static review of the supplied code; no commands were run. Authoritative current checks show 20 passed, no failures, and no pending checks.

Merge blockers

No merge blockers.

Suggestions

These are optional follow-ups and are not sent to /fix.

S2: 🟢 Exercise cleanup after a copy

In test/agentTankRuntime.test.mjs, the failure test rejects the first provider before copying credentials. Optionally add a valid first provider followed by an invalid second provider, then assert the runtime directory is removed and source credentials remain unchanged. This would directly exercise cleanup of already-created credential copies; the implementation’s existing catch block handles that path correctly by static inspection.

Score

The implementation satisfies the scoped objective, preserves read-only host mounts, and integrates the helper consistently across runtime and image packaging. Supplied validation and current checks support merge readiness, with no verified blocker.

Score: 9/10


🤖 Review Details

  • Model: GPT-6 Astra
  • Time: 22s
  • Tokens: 45,390 (44,936 in / 454 out)
  • Cost: $0.37

View Task


💡 Next step: Comment /fix to address every F# merge blocker, or name records explicitly, as in /fix F3 S5.
F# and S# IDs increment across review comments and remain permanent, so selectors such as /fix F3 F5 stay unambiguous across cycles. S# suggestions stay optional: they are implemented only when you name them, and they never relax a merge blocker.

🤖 Review by ProPR

@propr-dev propr-dev Bot removed the ultrafix label Sep 29, 2026
@integry
integry marked this pull request as ready for review September 29, 2026 11:45
@integry
integry merged commit 8b04403 into 2547/claude-opus-5-bundle-agent-tank-into-th-20260926-1849-t18 Sep 29, 2026
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants