Route Claude jobs back through ccrotate - #533
Merged
Merged
Conversation
Temporarily routes Anthropic traffic for Paperclip-spawned jobs through in-cluster ccrotate while keeping OpenAI/Codex on Penstock, and keeps /paperclip/bin in PATH for Claude hooks. Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
Hey @kkroo! Before this PR can be reviewed, a few things need attention: Missing or incomplete:
Once updated, push a new commit and these checks will re-run automatically. — commitperclip |
There was a problem hiding this comment.
Ally — Consolidated PR Review
Lenses: code review + test assertions + silent failure analysis.
Critical Issues (1)
- [native-codex]
values.blockcast.yaml:254-255—optional: trueremoved frompaperclip-ccrotate-serve-secretssecret references- Impact: Deployment will crash hard (
CreateContainerConfigError) if the secret doesn't exist, rather than gracefully degrading as before. The PR description does not confirm this secret is pre-created in the Blockcast cluster. - Recommendation: Either restore
optional: true(fail-open), document the prerequisite clearly in the comment block (it's missing today), or ensure the secret creation is a blocking prerequisite in the deployment runbook.
- Impact: Deployment will crash hard (
Important Issues (1)
- [native-codex]
runtime-cache.test.mjs:136-145+ values.blockcast.yaml — Test and values don't verify the secret actually exists- Impact: The test
assertCcrotateServeSecretEnvchecks that Helm renders a reference topaperclip-ccrotate-serve-secrets, but does not verify the secret is present in the cluster or that the keyserveTokenhas a value. A malformed or missing secret will only fail at pod startup, not at Helm render time. - Recommendation: Add a comment to the test noting this limitation, or add a prerequisite assertion in the values comment block confirming the secret must exist before deploy (see Critical Issues prerequisite). The PR body mentions "Live hotfix applied" — confirm the secret is now persistent in the deployment.
- Impact: The test
Suggestions (2)
-
[native-codex]
values.blockcast.yaml:230— RemovedROLLBACKcomment- The old comment explained exactly how to revert if Penstock is restored. The new comment says "temporarily" but doesn't include rollback steps. Consider adding a one-liner rollback guide:
# ROLLBACK: restore Penstock Anthropic by reverting ANTHROPIC_BASE_URL, secret name, and key.
- The old comment explained exactly how to revert if Penstock is restored. The new comment says "temporarily" but doesn't include rollback steps. Consider adding a one-liner rollback guide:
-
[native-codex]
values.blockcast.yaml:251-253— PREREQUISITES section missing ccrotate secret- The PREREQUISITES list items 1 and 2 still reference Penstock setup. Add:
# 0. Secret `paperclip-ccrotate-serve-secrets` (key `serveToken`) exists in ns paperclip # -- the ccrotate serve auth token. - This clarifies that the ccrotate route is the blocking dependency (not Penstock).
- The PREREQUISITES list items 1 and 2 still reference Penstock setup. Add:
Strengths
- Clear intent: The test name and comments clearly explain why Anthropic is temporarily back on ccrotate (Penstock malformed-200 issue).
- Split routing preserved: OpenAI/Codex remains on Penstock, reducing the scope of the rollback — good operational hygiene.
- PATH fix included: Adding
/paperclip/binto PATH solves the Claude hooks lookup issue in a single PR. - Test updates are thorough: All env assertions updated and function renamed to reflect ccrotate routing; no stale assertions left.
Recommended Action
- Before merge: Address Critical Issue #1 — either restore
optional: trueOR confirm the secret prerequisite in the comment block AND the deployment runbook. Unclear secret setup is a common deployment-time surprise. - This cycle: Add ccrotate secret to PREREQUISITES list (Suggestion #2).
- Consider: Add rollback instructions comment (Suggestion #1) so operators know exactly how to revert if Penstock is restored.
reviewed head: de98f1a
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thinking Path
Linked Issues or Issue Description
Refs #502, #451, #449, #450.
What happened
BLO/PEN Claude K8s agent runs repeatedly failed with
claude_transient_upstreamcaused by an empty or malformed HTTP 200 response on the Anthropic gateway path. Some Claude hooks also could not find the sharedccrotatewrapper because/paperclip/binwas not in the inherited PATH.Expected behavior
Paperclip-spawned Claude jobs should be able to call Anthropic through a healthy in-cluster route, and Claude hooks should be able to resolve the shared
ccrotatewrapper.Steps to reproduce
Run a BLO/PEN
claude_k8sheartbeat while the deployment-level Anthropic env points at the Penstock Anthropic gateway, then inspect failed heartbeat runs for malformed HTTP 200 errors.Paperclip version or commit
Blockcast deployment at
3e52b21ac34d1e82b7e537141ec19aafe981aed3.Deployment mode
Blockcast Kubernetes Helm deployment.
What Changed
ANTHROPIC_BASE_URLback to in-clusterhttp://ccrotate-serve.paperclip.svc:4001for Blockcast Paperclip pods.ANTHROPIC_AUTH_TOKENandANTHROPIC_API_KEYatpaperclip-ccrotate-serve-secrets/serveToken./paperclip/bintoPATHso Claude hooks can find the sharedccrotatewrapper.Verification
node deploy/helm/paperclip/tests/runtime-cache.test.mjsgit diff --checkdeploy/paperclip-apiandsts/paperclip; new env confirms Anthropic on ccrotate, OpenAI on Penstock, and/paperclip/binin PATH.Risks
Model Used
OpenAI GPT-5 Codex coding agent with shell, Kubernetes, GitHub CLI, and repository editing tools.
Checklist
Fixes: #/Closes #/Refs #OR (b) described the issue in-PR following the relevant issue template