Share one AI backend chain and add optional account sync - #145
Share one AI backend chain and add optional account sync#145nmbrthirteen wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughThis change centralizes AI provider selection, adds podcli Pro authentication and workspace commands, synchronizes clips, assets, and knowledge with cloud workspaces, adds a native sync launcher, and exposes account, insight, and AI setup UI. ChangesCentralized AI provider execution
podcli Pro cloud synchronization
Pro account and workspace UI
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant ai_provider
participant podcli_cloud
participant ai_cli
CLI->>ai_provider: request AI generation
ai_provider->>podcli_cloud: try cloud provider
ai_provider->>ai_cli: try local CLI provider
ai_provider-->>CLI: return normalized result
sequenceDiagram
participant User
participant sync
participant ClipsHistory
participant AssetSync
participant KnowledgeSync
User->>sync: run podcli sync
sync->>ClipsHistory: backfill cloud clips
sync->>AssetSync: synchronize workspace assets
sync->>KnowledgeSync: synchronize knowledge files
sync-->>User: report results and exit status
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (8)
cli/internal/engine/engine.go (1)
173-178: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
errors.Asfor the exit-code check.
exec.Cmd.Runreturns*exec.ExitErrorunwrapped today, so the type assertion works.errors.Asis the idiomatic form and keeps working if the error is ever wrapped.♻️ Proposed refactor
if err := cmd.Run(); err != nil { - if ee, ok := err.(*exec.ExitError); ok { + var ee *exec.ExitError + if errors.As(err, &ee) { return ee.ExitCode(), nil } return 1, err }Add
"errors"to the import block.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/internal/engine/engine.go` around lines 173 - 178, Update the error handling after cmd.Run in the engine flow to use errors.As when checking for *exec.ExitError, adding the errors import as needed. Preserve the existing exit-code return for matched errors and the fallback return for other errors.src/services/clips-history-cloud.test.ts (1)
24-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the fixed 10 ms wait with a condition poll.
settle()assumes the background sync started byrecord()finishes within 10 ms. Under CI load it may still be in flight, and the followingbackfillCloudthen returns early on thesyncingguard, soregisterClipis never called and Line 68 fails. Poll for the observable state instead.The overlap test also asserts only call counts. Add an assertion on the returned
{ synced, failed }pair; it documents what an overlapping run should report.♻️ Proposed refactor
-/** record() fires its cloud sync in the background; let it settle before asserting. */ -const settle = () => new Promise((resolve) => setTimeout(resolve, 10)); +/** record() fires its cloud sync in the background; wait for it to finish. */ +const settle = async () => { + for (let i = 0; i < 200; i++) { + if (vi.mocked(cloud.signedIn).mock.calls.length > 0) return; + await new Promise((r) => setTimeout(r, 5)); + } +};Also applies to: 75-84
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/clips-history-cloud.test.ts` around lines 24 - 25, Replace the fixed-delay settle helper in the record/backfill tests with polling that waits until the observable sync state indicates the background operation has completed, avoiding the syncing guard race under CI load. In the overlap test around backfillCloud, retain the call-count assertions and also assert the returned synced and failed values for the overlapping run.src/services/asset-sync.test.ts (1)
14-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
pull.The suite exercises
pushonly.pullhandles server-supplied names, writes files, and registers them locally, so it carries the higher risk of the two. Add cases for a remote-only asset that downloads and registers, a name such as../../pwned.pngthat must stay inside the assets directory, and an existing local file that must not be overwritten.I can draft these tests if you want.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/asset-sync.test.ts` around lines 14 - 24, Extend the test suite around the imported asset-sync symbols to cover pull: verify a remote-only asset is downloaded, written, and registered; verify a name such as ../../pwned.png is safely kept within the assets directory; and verify an existing local file is not overwritten. Reuse the existing cloud mocks and assert the relevant download, filesystem, and registration behavior.backend/services/content_generator.py (1)
228-233: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider passing
project_dirfor consistency.
generate_clip_contentpassesproject_dirtoai_provider.generate; this call omits it. If a CLI backend usesproject_diras its working directory, the two paths run with different context. Confirm the omission is intentional.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/content_generator.py` around lines 228 - 233, Update the ai_provider.generate call in the surrounding content-generation function to pass the same project_dir context used by generate_clip_content. Preserve the existing prompt, timeout, on_attempt, and adapt arguments, and ensure both generation paths use the intended working directory.tests/test_entitlement_chain.py (1)
15-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the temporary directory after each test.
tempfile.mkdtemp()has no matching cleanup, so each test leaves a directory containingauth.jsonbehind. Eight tests in this class leak eight directories per run.💚 Proposed fix
+import shutil + def setUp(self): self.tmp = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) patcher = mock.patch.dict(podcli_cloud.paths, {"home": self.tmp})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_entitlement_chain.py` around lines 15 - 19, Update setUp to register cleanup for self.tmp after each test, using the existing unittest cleanup mechanism so the temporary directory and its contents are removed while preserving the patcher cleanup.backend/cli.py (2)
1010-1012: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the sentinel and drop the duplicate import.
ai_provideris already imported at Line 923 in the same function. The name_ai_cli_pathnow holds the string"cloud"instead of a path, which reads as a filesystem path to later readers. A boolean flag states the intent directly.♻️ Proposed refactor
- # Per-clip content generation needs any provider, not specifically a binary. - from services import ai_provider - _ai_cli_path = "cloud" if ai_provider.available() else None + # Per-clip content generation needs any provider, not specifically a binary. + _ai_available = ai_provider.available()Then update the two later uses (Line 1032, Line 1105, Line 1304) to
_ai_available.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/cli.py` around lines 1010 - 1012, In the current function, remove the duplicate local import of ai_provider and rename the _ai_cli_path sentinel to the boolean flag _ai_available, assigning it from ai_provider.available(). Update all later references to _ai_cli_path, including the uses near the noted generation and execution logic, to use _ai_available instead.
3667-3674: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReport why the backfill failed.
The bare
except Exceptionsetssynced, failed = 0, 0, so a failed backfill prints nothing at all. The user cannot tell the difference between "nothing to sync" and "sync crashed". Print the reason to stderr and keep sign-in successful.♻️ Proposed refactor
try: synced, failed = podcli_cloud.backfill_clips() - except Exception: + except Exception as exc: # noqa: BLE001 - sign-in must not fail on backfill synced, failed = 0, 0 + print(f"Could not sync existing clips: {exc}", file=sys.stderr)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/cli.py` around lines 3667 - 3674, Update the exception handler around podcli_cloud.backfill_clips to capture the exception, print its reason to stderr, and retain the zeroed synced/failed values so sign-in remains successful. Keep the existing success and partial-failure messages unchanged.Source: Linters/SAST tools
backend/services/claude_suggest.py (1)
25-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
ai_clihelper imports.These helpers are not called in
backend/services/claude_suggest.py; keep onlyclassify_cli_errorif it is still needed, otherwise drop the dead re-export imports.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/claude_suggest.py` around lines 25 - 33, Remove the unused helper imports from the services.ai_cli import block in claude_suggest.py, including _engine_label, _find_ai_cli, _find_ai_cli_candidates, _format_timeout_label, _run_ai_command, and get_ai_cli_status. Retain classify_cli_error only if it is referenced elsewhere in the module; otherwise remove it as well.
🤖 Prompt for all review comments with AI agents
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 `@backend/cli.py`:
- Around line 3677-3684: Update cmd_logout to detect an active PODCLI_TOKEN
session before calling podcli_cloud.clear_token(), warn the user that logout
cannot clear the environment-provided token, and avoid printing the successful
“Signed out” message in that case; preserve the existing file-token logout
behavior.
- Around line 3721-3746: Update cmd_workspace to avoid direct indexing of
optional workspace display fields: use safe defaults for created and switched
names and plans, and use .get() with fallback values when matching and listing
workspaces instead of w["name"].lower() or other raw display-field access.
Preserve identifier-based matching and the existing command output behavior.
In `@backend/main.py`:
- Around line 693-704: Update the failure handling in backend/main.py lines
693-704 within the suggestion-generation handler to return the selected
provider’s error or provider-neutral guidance instead of naming Claude, Codex,
or a specific CLI. Apply the same change at backend/main.py lines 796-807 in the
content-generation handler, removing the claim that an AI CLI was found and
preserving the existing error result flow.
In `@backend/services/ai_cli.py`:
- Around line 347-356: The _discovery_key function does not change when the
env-file values used by _configured_cli_path change. Include a fingerprint or
equivalent change marker for the env-settings file in _discovery_key, or
invalidate both _discover and _lookup_dirs immediately after env-settings
writes, while preserving existing environment-based invalidation.
- Around line 421-427: Update the Codex invocation in the subprocess.run call to
use a read-only, no-write sandbox and disable external tool access instead of
--full-auto. Ensure prompts containing user or transcript input cannot enable
workspace edits, while preserving the existing generation flow and output
handling.
In `@backend/services/claude_suggest.py`:
- Around line 517-528: Update the accepted-response handling following usable()
so it revalidates the extracted JSON as a dict with a truthy clips value before
subscripting clips. Reuse the validated parsed result when possible, and
preserve rejection of malformed or empty responses instead of allowing TypeError
or KeyError to escape.
In `@backend/services/content_generator.py`:
- Around line 362-383: Update the Claude streaming branch in
generate_clip_content around _stream_claude_content so exceptions from timeout
or subprocess failures are caught and treated as Claude failure, allowing
execution to continue to the provider fallback chain. Preserve prompt_file
cleanup in all cases and retain the existing successful engine_used assignment
and unusable-output fallback behavior.
In `@backend/services/podcli_cloud.py`:
- Around line 145-169: Update `_describe` to accept only dictionary-shaped JSON
responses before calling `payload.get` or reading `used`/`cap`; normalize any
list, string, number, or other decoded value to an empty dict so malformed error
bodies still return the appropriate `CloudError` classification. Also simplify
the server-error condition by removing the redundant `exc.code == 503` check
after `exc.code >= 500`.
- Around line 53-61: Update _write_auth to create auth.json with mode 0o600 at
file creation time, using an exclusive low-level file-descriptor/open flow
before writing the JSON, rather than relying on os.chmod after open. Preserve
the existing path creation and JSON content behavior, and ensure the descriptor
is properly closed.
In `@cli/main.go`:
- Around line 781-783: Update the CLI help text near the command list in main so
login, logout, and whoami are grouped with the engine commands rather than
launcher commands, matching their default dispatch through runEngine(args); keep
sync listed as the launcher-handled command.
In `@src/services/asset-sync.ts`:
- Around line 113-128: Update the pull logic around cloud.downloadAsset and
manager.register to preserve entry.kind when it is a valid AssetType, falling
back to inferType only when necessary. Replace basename-only target construction
with a deterministic asset-unique filename so distinct remote names cannot
overwrite each other or share registry paths.
In `@src/services/clips-history.ts`:
- Around line 347-356: Update syncToCloud and the backfillCloud loop so an
already-in-flight entry is observable as a skipped sync rather than a failure.
Have syncToCloud return a distinct skip result when this.syncing already
contains the entry id, and make backfillCloud exclude skipped entries from both
synced and failed counters while preserving existing counting for completed and
genuinely failed syncs.
In `@src/services/knowledge-sync.ts`:
- Around line 108-112: Update the conflict-writing branch in the knowledge sync
flow to reuse the resolved target from insideKnowledge instead of rebuilding the
filename from the raw path. Ensure the target’s parent directory is created
before writeFile, matching the pull branch, while preserving the existing
conflict report.
- Around line 120-125: Move the readFile call inside the try block in sync’s
local-path processing loop, keeping the existing cloud.putKnowledge and
per-entry error handling intact. Ensure unreadable files or directories are
handled by that catch so sync continues and saveState still executes for
successfully processed entries.
In `@src/services/podcli-cloud.ts`:
- Around line 198-273: Add AbortSignal.timeout(...) to the fetch calls in
putKnowledge, uploadAsset, and downloadAsset, using a shorter timeout for the
knowledge write and a longer timeout for the asset upload and download
transfers. Preserve the existing request behavior and error handling while
ensuring each stalled connection aborts instead of waiting indefinitely.
In `@src/ui/client/AiSetup.tsx`:
- Around line 46-50: Update the provider-status fetch in AiSetup’s useEffect to
check response.ok before parsing or storing the response, and normalize
providers and candidates to empty arrays for successful responses when absent.
Ensure failed responses also produce a safe status shape before setStatus so
rendering status.candidates.length remains valid.
In `@src/ui/client/ConfigPage.tsx`:
- Around line 173-174: Update the parent component containing AiSetup so it
tracks a provider-status refresh revision or state, increments or updates it
after each successful saveSetting and clearSetting operation, and passes it to
AiSetup as a prop. Modify AiSetup to reload provider status whenever that prop
changes, while preserving its existing mount-time loading behavior.
In `@src/ui/web-server.ts`:
- Around line 1457-1463: Update the response construction in the web-server
route to populate cap from the authenticated account payload rather than
deriving it from me.plan. Remove the hard-coded team/non-team conditional and
use the account’s existing quota-limit field, preserving the other response
properties.
In `@tests/test_ai_fallback.py`:
- Around line 424-430: Update test_get_ai_cli_status_reports_candidates to patch
ai._find_ai_cli_candidates instead of cs._find_ai_cli_candidates, matching the
dependency called by ai.get_ai_cli_status(). Keep the mocked candidate value and
_configured_cli_path patch unchanged.
In `@tests/test_entitlement_chain.py`:
- Around line 20-23: Update the test setup in tests/test_entitlement_chain.py to
clear ANTHROPIC_API_KEY alongside the existing PODCLI_TOKEN and
PODCLI_AI_PROVIDER values. In test_forced_cloud_mode_ignores_a_free_verdict,
patch ai_cli._find_ai_cli_candidates consistently with the other chain tests
before calling _chain().
---
Nitpick comments:
In `@backend/cli.py`:
- Around line 1010-1012: In the current function, remove the duplicate local
import of ai_provider and rename the _ai_cli_path sentinel to the boolean flag
_ai_available, assigning it from ai_provider.available(). Update all later
references to _ai_cli_path, including the uses near the noted generation and
execution logic, to use _ai_available instead.
- Around line 3667-3674: Update the exception handler around
podcli_cloud.backfill_clips to capture the exception, print its reason to
stderr, and retain the zeroed synced/failed values so sign-in remains
successful. Keep the existing success and partial-failure messages unchanged.
In `@backend/services/claude_suggest.py`:
- Around line 25-33: Remove the unused helper imports from the services.ai_cli
import block in claude_suggest.py, including _engine_label, _find_ai_cli,
_find_ai_cli_candidates, _format_timeout_label, _run_ai_command, and
get_ai_cli_status. Retain classify_cli_error only if it is referenced elsewhere
in the module; otherwise remove it as well.
In `@backend/services/content_generator.py`:
- Around line 228-233: Update the ai_provider.generate call in the surrounding
content-generation function to pass the same project_dir context used by
generate_clip_content. Preserve the existing prompt, timeout, on_attempt, and
adapt arguments, and ensure both generation paths use the intended working
directory.
In `@cli/internal/engine/engine.go`:
- Around line 173-178: Update the error handling after cmd.Run in the engine
flow to use errors.As when checking for *exec.ExitError, adding the errors
import as needed. Preserve the existing exit-code return for matched errors and
the fallback return for other errors.
In `@src/services/asset-sync.test.ts`:
- Around line 14-24: Extend the test suite around the imported asset-sync
symbols to cover pull: verify a remote-only asset is downloaded, written, and
registered; verify a name such as ../../pwned.png is safely kept within the
assets directory; and verify an existing local file is not overwritten. Reuse
the existing cloud mocks and assert the relevant download, filesystem, and
registration behavior.
In `@src/services/clips-history-cloud.test.ts`:
- Around line 24-25: Replace the fixed-delay settle helper in the
record/backfill tests with polling that waits until the observable sync state
indicates the background operation has completed, avoiding the syncing guard
race under CI load. In the overlap test around backfillCloud, retain the
call-count assertions and also assert the returned synced and failed values for
the overlapping run.
In `@tests/test_entitlement_chain.py`:
- Around line 15-19: Update setUp to register cleanup for self.tmp after each
test, using the existing unittest cleanup mechanism so the temporary directory
and its contents are removed while preserving the patcher cleanup.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c34f0015-3306-4676-8e63-17988c3a7904
📒 Files selected for processing (34)
.gitignorebackend/cli.pybackend/main.pybackend/services/ai_cli.pybackend/services/ai_provider.pybackend/services/claude_suggest.pybackend/services/content_generator.pybackend/services/env_settings.pybackend/services/integrations/youtube/learnings.pybackend/services/podcli_cloud.pybackend/services/thumbnail_ai.pycli/internal/engine/engine.gocli/main.goscripts/build-studio.shsrc/models/index.tssrc/services/asset-sync.test.tssrc/services/asset-sync.tssrc/services/clips-history-cloud.test.tssrc/services/clips-history.tssrc/services/knowledge-sync.test.tssrc/services/knowledge-sync.tssrc/services/podcli-cloud.tssrc/sync.tssrc/ui/client/AccountChip.tsxsrc/ui/client/AiSetup.tsxsrc/ui/client/AnalyticsPage.tsxsrc/ui/client/ConfigPage.tsxsrc/ui/client/Layout.tsxsrc/ui/client/WorkspaceInsights.tsxsrc/ui/public/css/styles.csssrc/ui/web-server.tstests/test_ai_fallback.pytests/test_entitlement_chain.pytests/test_find_moments.py
| def cmd_logout(args): | ||
| from services import podcli_cloud | ||
|
|
||
| if not podcli_cloud.signed_in(): | ||
| print("Not signed in.") | ||
| return | ||
| podcli_cloud.clear_token() | ||
| print("Signed out. podcli will use your local AI CLI from now on.") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
logout reports success while a PODCLI_TOKEN session stays active.
podcli_cloud.read_token() returns PODCLI_TOKEN before it reads the file. clear_token() only unlinks the file. If the user set PODCLI_TOKEN, signed_in() stays True after logout and the printed message is wrong. Warn in that case.
🐛 Proposed fix
if not podcli_cloud.signed_in():
print("Not signed in.")
return
podcli_cloud.clear_token()
+ if os.environ.get("PODCLI_TOKEN", "").strip():
+ print("PODCLI_TOKEN is still set in this environment — unset it to fully sign out.")
+ return
print("Signed out. podcli will use your local AI CLI from now on.")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/cli.py` around lines 3677 - 3684, Update cmd_logout to detect an
active PODCLI_TOKEN session before calling podcli_cloud.clear_token(), warn the
user that logout cannot clear the environment-provided token, and avoid printing
the successful “Signed out” message in that case; preserve the existing
file-token logout behavior.
| if action == "new": | ||
| created = podcli_cloud.create_workspace(args.name) | ||
| print(f"Created {created['name']} and switched to it (free plan).") | ||
| print("Each show carries its own subscription, so this one needs its own.") | ||
| _warn_local_data() | ||
| return | ||
|
|
||
| workspaces = podcli_cloud.list_workspaces() | ||
|
|
||
| if action == "use": | ||
| target = next( | ||
| (w for w in workspaces | ||
| if args.name.lower() in (w["name"].lower(), w["id"].lower())), | ||
| None, | ||
| ) | ||
| if not target: | ||
| print(f"No workspace matching {args.name!r}.") | ||
| sys.exit(1) | ||
| switched = podcli_cloud.switch_workspace(target["id"]) | ||
| print(f"Switched to {switched['name']} ({switched['plan']} plan).") | ||
| _warn_local_data() | ||
| return | ||
|
|
||
| for w in workspaces: | ||
| marker = "*" if w.get("current") else " " | ||
| print(f" {marker} {w['name']} ({w['plan']}, {w['role']})") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect the workspace payload contract on both clients.
fd -t f 'podcli.cloud' -x rg -n -C4 'workspaces|workspaceId|switch_workspace|switchWorkspace|create_workspace|createWorkspace' {}Repository: nmbrthirteen/podcli
Length of output: 1652
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)cli\.py$|(^|/)podcli\.cloud$' || true
echo "== backend/cli.py relevant sections =="
if [ -f backend/cli.py ]; then
wc -l backend/cli.py
sed -n '3680,3770p' backend/cli.py | cat -n
echo "-- Workspace command area --"
rg -n -C 8 'def cmd_workspace|workspace_action|_warn_local_data|create_workspace|switch_workspace|list_workspaces' backend/cli.py
fi
echo "== podcli.cloud workspace related section =="
if [ -f backend/cli/cloud/podcli.cloud ] || [ -f backend/cli/podcli.cloud ]; then
file=$(git ls-files | rg '/podcli\.cloud$' | head -n1)
sed -n '1,400p' "$file" | cat -n | sed -n '240,330p'
fi
echo "== workspace TypeScript related section =="
for f in $(git ls-files | rg 'podcli.cloud$'); do
echo "--- $f ---"
rg -n -C 6 'workspaceId|plan|workspaces|createWorkspace|switchWorkspace' "$f"
doneRepository: nmbrthirteen/podcli
Length of output: 7829
Guard optional workspace display fields against KeyError.
cmd_workspace() uses raw payload fields in print() and w["name"].lower(), but the workspace payload contract only guarantees token/workspace identifiers. A missing display field raises KeyError, and except podcli_cloud.CloudError does not catch it. Use .get() with safe defaults for display fields, such as created.get("name", args.name), switched.get("name", target.get("name", "workspace")), and switched.get("plan", "unknown").
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/cli.py` around lines 3721 - 3746, Update cmd_workspace to avoid
direct indexing of optional workspace display fields: use safe defaults for
created and switched names and plans, and use .get() with fallback values when
matching and listing workspaces instead of w["name"].lower() or other raw
display-field access. Preserve identifier-based matching and the existing
command output behavior.
| # Gate on the provider chain, not on a local binary: a signed-in Pro user | ||
| # has AI available without installing anything. | ||
| if not ai_provider.available(): | ||
| emit_result( | ||
| task_id, | ||
| "error", | ||
| error=( | ||
| "No AI CLI available (install Claude Code or Codex). " | ||
| "If already installed, set the path in Config → AI CLI or PODCLI_CLAUDE_PATH." | ||
| "No AI available. Sign in with `podcli login`, install Claude Code " | ||
| "or Codex, or set ANTHROPIC_API_KEY." | ||
| ), | ||
| ) | ||
| return |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use provider-neutral failure guidance after unified provider selection.
Both handlers can now use cloud, CLI, or direct API providers. Do not report only Claude or Codex recovery steps.
backend/main.py#L693-L704: when suggestion generation fails, return the provider error or provider-neutral guidance.backend/main.py#L796-L807: when content generation fails, do not state that an AI CLI was found.
📍 Affects 1 file
backend/main.py#L693-L704(this comment)backend/main.py#L796-L807
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/main.py` around lines 693 - 704, Update the failure handling in
backend/main.py lines 693-704 within the suggestion-generation handler to return
the selected provider’s error or provider-neutral guidance instead of naming
Claude, Codex, or a specific CLI. Apply the same change at backend/main.py lines
796-807 in the content-generation handler, removing the claim that an AI CLI was
found and preserving the existing error result flow.
| def _discovery_key() -> tuple: | ||
| """Everything discovery reads. Changing any of it must re-probe.""" | ||
| return tuple( | ||
| os.environ.get(name, "") | ||
| for name in ( | ||
| "PATH", "HOME", "NVM_DIR", "APPDATA", "ProgramData", | ||
| "NPM_CONFIG_PREFIX", "npm_config_prefix", | ||
| "PODCLI_CLAUDE_PATH", "PODCLI_CODEX_PATH", | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Invalidate discovery after configured-path changes.
Line 347 keys the cache only from environment values. _configured_cli_path() also reads PODCLI_CLAUDE_PATH and PODCLI_CODEX_PATH from the env file. If a user updates that file after discovery ran, _discover() keeps the old candidates until process restart.
Include the env-file fingerprint in the key, or clear _discover and _lookup_dirs after an env-settings write.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/ai_cli.py` around lines 347 - 356, The _discovery_key
function does not change when the env-file values used by _configured_cli_path
change. Include a fingerprint or equivalent change marker for the env-settings
file in _discovery_key, or invalidate both _discover and _lookup_dirs
immediately after env-settings writes, while preserving existing
environment-based invalidation.
| result = subprocess.run( | ||
| [ | ||
| cli_path, "exec", | ||
| "--full-auto", | ||
| "-o", output_file, | ||
| prompt, | ||
| ], |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg '(^|/)backend/services/ai_cli\.py$|ai_cli\.py$' || true
echo "== relevant lines =="
if [ -f backend/services/ai_cli.py ]; then
nl -ba backend/services/ai_cli.py | sed -n '360,470p'
fi
echo "== search full-auto and exec usage =="
rg -n -- '--full-auto|codex|project_dir|output_file|stdin\s*=' backend/services/ai_cli.py . --glob '!**/.git/**' --glob '!**/node_modules/**' | head -200Repository: nmbrthirteen/podcli
Length of output: 263
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file info =="
wc -l backend/services/ai_cli.py
echo "== relevant lines with cat -n =="
cat -n backend/services/ai_cli.py | sed -n '360,470p'
echo "== search full-auto, codex, project_dir in backend/services/ai_cli.py =="
rg -n -- '--full-auto|--no-auto|--sandbox|codex|project_dir|output_file|stdin\s*=|call_kwargs|subprocess.run|subprocess.Popen' backend/services/ai_cli.py || true
echo "== repository-wide prompt/project_dir usage =="
rg -n -- '--full-auto|--no-auto|project_dir|stdin\s*=|output_file' backend/services/ai_cli.py . --glob '!**/.git/**' --glob '!**/node_modules/**' | head -250 || trueRepository: nmbrthirteen/podcli
Length of output: 9964
🌐 Web query:
OpenAI Codex CLI --full-auto deprecated workspace write mode issue 6522
💡 Result:
The --full-auto flag in the OpenAI Codex CLI has been deprecated as of version 0.128.0 [1][2]. It was previously used to bypass approval prompts and grant the agent unrestricted access to the filesystem and network, but it has been replaced by explicit permission and sandbox configurations to improve security and predictability [1]. When using the --full-auto flag in newer versions of the CLI, the system will print a deprecation warning and may cause processes to exit with a non-zero status (code 1) if the CLI environment or dependent tools are not configured to handle the deprecation [1][2]. To resolve issues related to this deprecation: 1. Update your command to use --sandbox workspace-write [3][4][2]. For non-interactive scripts, the recommended replacement is: codex exec --sandbox workspace-write [3][4][5] 2. If you require finer control over approvals, use the --ask-for-approval flag alongside the sandbox mode [4][5]: codex exec --sandbox workspace-write --ask-for-approval on-request [4][5] The issue number 6522, as referenced in your query, does not appear to be the primary tracking issue for the --full-auto deprecation, which was handled via pull request #20133 [6][7]. Users experiencing hangs or "code 1" exits are generally advised to update their automation scripts, daemon configurations, or tool-calling logic to replace the deprecated flag with the explicit --sandbox profiles [2][5]. If you encounter persistent hangs or "timeout waiting for child process" errors, ensure your environment is updated to the latest CLI version, as several unrelated hang issues have been addressed in recent updates [8][9][10].
Citations:
- 1: https://codex.danielvaughan.com/2026/05/02/codex-cli-full-auto-deprecation-permission-profiles-trust-flows/
- 2: [Bug] Codex CLI agent exits with code 1 due to deprecated --full-auto flag (v0.128.0) nexu-io/open-design#438
- 3: https://developers.openai.com/codex/noninteractive
- 4: https://developers.openai.com/codex/llms-full.txt
- 5: https://ofox.ai/blog/codex-command-failed-retry-without-sandbox-fix-2026/
- 6: chore(cli) deprecate --full-auto openai/codex#20133
- 7: openai/codex@3d10ba9
- 8: Codex CLI hangs indefinitely when the workspace is out of credits openai/codex#6512
- 9: Codex CLI hangs indefinitely on any prompt ("Working…" forever) and stops responding mid-session openai/codex#7187
- 10:
codex exechangs indefinitely —codex_models_managerlogs "timeout waiting for child process to exit", no error surfaced, no recovery openai/codex#34397
🌐 Web query:
OpenAI Codex exec --sandbox no-write --full-auto deprecated workspace-write noninteractive
💡 Result:
In the OpenAI Codex CLI, the --full-auto flag is deprecated [1][2]. Users are advised to transition to explicit --sandbox configurations to maintain secure and predictable automation behavior [1][2]. The command structure you inquired about, codex exec --sandbox no-write --full-auto, reflects an outdated approach. Official documentation now mandates the use of explicit sandbox modes for non-interactive and automated workflows [1][2]. Specifically: 1. Deprecation of --full-auto: The --full-auto flag is a legacy compatibility option that triggers a warning [1][2]. It was previously a convenience alias that bundled sandbox workspace-write with an auto-approval policy [3][4]. 2. Replacement Strategy: For non-interactive or automated tasks, you should use explicit flags. For example, to run in a read-only state, use --sandbox read-only [1][2]. To allow workspace editing while maintaining control, use --sandbox workspace-write [1][2]. 3. Sandbox Configuration: The --sandbox flag defines the boundary for filesystem and network access [5][4]. Common modes include read-only (no file edits), workspace-write (edits within the workspace), and danger-full-access (unrestricted) [5][2][4]. 4. Workspace-write non-interactive: If your goal is to allow automated edits without interactive prompts, use the combination of --sandbox workspace-write and an appropriate approval policy (e.g., --ask-for-approval never for fully automated CI/CD environments) [1][2][3]. You should remove --full-auto from your scripts and replace it with the specific --sandbox and approval policy flags that align with your required security posture [1][2]. Ensure that your configuration does not mix older sandbox_mode settings with newer permission profile structures if your environment has been updated [6].
Citations:
- 1: https://developers.openai.com/codex/noninteractive
- 2: https://developers.openai.com/codex/agent-approvals-security
- 3: https://github.com/openai/codex/blob/13c42a077c88a0d04ae7680a9891d2daf4558577/docs/sandbox.md
- 4: https://www.vincentschmalbach.com/how-codex-cli-flags-actually-work-full-auto-sandbox-and-bypass/
- 5: https://developers.openai.com/codex/concepts/sandboxing
- 6: https://developers.openai.com/codex/permissions
Do not let transcript-driven generation run Codex with open write access.
backend/services/ai_cli.py:421-432 invokes Codex --full-auto with the workspace as cwd. This flag is deprecated and enables auto-approval for workspace edits. If this path only needs generation, replace it with a no-write/read-only Codex sandbox and remove external tool access. Keep --full-auto out of requests that can contain user or transcript input.
🧰 Tools
🪛 Ruff (0.16.1)
[error] 421-421: subprocess call: check for execution of untrusted input
(S603)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/ai_cli.py` around lines 421 - 427, Update the Codex
invocation in the subprocess.run call to use a read-only, no-write sandbox and
disable external tool access instead of --full-auto. Ensure prompts containing
user or transcript input cannot enable workspace edits, while preserving the
existing generation flow and output handling.
| useEffect(() => { | ||
| fetch("/api/ai-provider-status") | ||
| .then((r) => r.json()) | ||
| .then(setStatus) | ||
| .catch(() => setStatus(null)); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle non-success provider-status responses before rendering.
The server returns { error: ... } with HTTP 500 when provider-status execution fails. This code stores that object as Status. Line 125 then reads status.candidates.length, which causes a render exception because candidates is undefined.
Check response.ok and normalize providers and candidates to empty arrays before calling setStatus.
Proposed fix
+const UNAVAILABLE_STATUS: Status = {
+ available: false,
+ providers: [],
+ mode: "",
+ api_key_set: false,
+ candidates: [],
+};
+
useEffect(() => {
fetch("/api/ai-provider-status")
- .then((r) => r.json())
- .then(setStatus)
- .catch(() => setStatus(null));
+ .then(async (r) => {
+ if (!r.ok) throw new Error("Could not load AI provider status");
+ return r.json() as Promise<Partial<Status>>;
+ })
+ .then((data) => setStatus({
+ ...UNAVAILABLE_STATUS,
+ ...data,
+ providers: data.providers ?? [],
+ candidates: data.candidates ?? [],
+ }))
+ .catch(() => setStatus(UNAVAILABLE_STATUS));
}, []);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| fetch("/api/ai-provider-status") | |
| .then((r) => r.json()) | |
| .then(setStatus) | |
| .catch(() => setStatus(null)); | |
| const UNAVAILABLE_STATUS: Status = { | |
| available: false, | |
| providers: [], | |
| mode: "", | |
| api_key_set: false, | |
| candidates: [], | |
| }; | |
| useEffect(() => { | |
| fetch("/api/ai-provider-status") | |
| .then(async (r) => { | |
| if (!r.ok) throw new Error("Could not load AI provider status"); | |
| return r.json() as Promise<Partial<Status>>; | |
| }) | |
| .then((data) => setStatus({ | |
| ...UNAVAILABLE_STATUS, | |
| ...data, | |
| providers: data.providers ?? [], | |
| candidates: data.candidates ?? [], | |
| })) | |
| .catch(() => setStatus(UNAVAILABLE_STATUS)); | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ui/client/AiSetup.tsx` around lines 46 - 50, Update the provider-status
fetch in AiSetup’s useEffect to check response.ok before parsing or storing the
response, and normalize providers and candidates to empty arrays for successful
responses when absent. Ensure failed responses also produce a safe status shape
before setStatus so rendering status.candidates.length remains valid.
| <AiSetup /> | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Refresh AiSetup after AI settings change.
AiSetup loads provider status only when it mounts. saveSetting and clearSetting refresh only aiCli. After a user configures a CLI path or API key, this panel can continue to show “AI is not set up” until a page reload.
Pass a refresh revision or provider-status state into AiSetup, then update it after each successful save or clear.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ui/client/ConfigPage.tsx` around lines 173 - 174, Update the parent
component containing AiSetup so it tracks a provider-status refresh revision or
state, increments or updates it after each successful saveSetting and
clearSetting operation, and passes it to AiSetup as a prop. Modify AiSetup to
reload provider status whenever that prop changes, while preserving its existing
mount-time loading behavior.
| res.json({ | ||
| signedIn: true, | ||
| workspace: me.workspace?.name, | ||
| plan: me.plan, | ||
| episodesUsed: me.workspace?.episodes_used, | ||
| cap: me.plan === "team" ? 40 : 10, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect the whoami/account payload fields on both cloud clients.
fd -t f 'podcli.cloud' -x rg -n -C6 'episodes_used|episode|cap|plan' {}Repository: nmbrthirteen/podcli
Length of output: 6182
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
fd -t f 'web-server\.ts|test_entitlement_chain\.py|podcli-cloud\.ts|entitlement|quota|cap|episode' . | sed 's#^\./##' | head -200
echo
echo "== src/ui/web-server.ts around reported route =="
sed -n '1420,1485p' src/ui/web-server.ts
echo
echo "== analytics / quota display references =="
rg -n -C4 'episodesUsed|sessionData|me/workspace|whoami|Analytics|quota|cap|plan' src tests -g '!*.pyc' | head -300Repository: nmbrthirteen/podcli
Length of output: 24336
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tests/test_entitlement_chain.py =="
sed -n '1,220p' tests/test_entitlement_chain.py
echo
echo "== podcli-cloud quota/limit enforcement =="
sed -n '1,260p' src/services/podcli-cloud.ts
echo
echo "== local quota enforcement references =="
rg -n -C5 'episodes_used| episodes_used|episode_limit|episode cap|cap|monthly limit|429|402|403|generated|count|limit' src backend backend/tests tests -g '!*.pyc' | head -400
echo
echo "== API endpoints mentioning used or cap =="
rg -n -C6 'episodes_used|episode_cap|used|cap|quota|limit' backend src tests -g '!*.pyc'Repository: nmbrthirteen/podcli
Length of output: 38838
Stop deriving the account quota limit in the web server route.
cap: me.plan === "team" ? 40 : 10 duplicates entitlement rules and ignores paid plans that are not team, such as pro or studio. Get the limit from the authenticated account payload instead of hard-coding plan tiers in this route.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/ui/web-server.ts` around lines 1457 - 1463, Update the response
construction in the web-server route to populate cap from the authenticated
account payload rather than deriving it from me.plan. Remove the hard-coded
team/non-team conditional and use the account’s existing quota-limit field,
preserving the other response properties.
| def test_get_ai_cli_status_reports_candidates(self): | ||
| with mock.patch.object( | ||
| cs, | ||
| "_find_ai_cli_candidates", | ||
| return_value=[("/tmp/claude", "claude")], | ||
| ), mock.patch.object(cs, "_configured_cli_path", return_value=None): | ||
| status = cs.get_ai_cli_status() | ||
| ), mock.patch.object(ai, "_configured_cli_path", return_value=None): | ||
| status = ai.get_ai_cli_status() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Patch the dependency that get_ai_cli_status() uses.
get_ai_cli_status() calls ai._find_ai_cli_candidates(), but this test patches cs._find_ai_cli_candidates. The test can run real discovery probes and does not verify the mocked candidate. Patch ai instead.
Proposed fix
- cs,
+ ai,
"_find_ai_cli_candidates",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_get_ai_cli_status_reports_candidates(self): | |
| with mock.patch.object( | |
| cs, | |
| "_find_ai_cli_candidates", | |
| return_value=[("/tmp/claude", "claude")], | |
| ), mock.patch.object(cs, "_configured_cli_path", return_value=None): | |
| status = cs.get_ai_cli_status() | |
| ), mock.patch.object(ai, "_configured_cli_path", return_value=None): | |
| status = ai.get_ai_cli_status() | |
| def test_get_ai_cli_status_reports_candidates(self): | |
| with mock.patch.object( | |
| ai, | |
| "_find_ai_cli_candidates", | |
| return_value=[("/tmp/claude", "claude")], | |
| ), mock.patch.object(ai, "_configured_cli_path", return_value=None): | |
| status = ai.get_ai_cli_status() |
🧰 Tools
🪛 ast-grep (0.45.0)
[info] 427-427: Do not hardcode temporary file or directory names
Context: "/tmp/claude"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
🪛 Ruff (0.16.1)
[error] 428-428: Probable insecure usage of temporary file or directory: "/tmp/claude"
(S108)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_ai_fallback.py` around lines 424 - 430, Update
test_get_ai_cli_status_reports_candidates to patch ai._find_ai_cli_candidates
instead of cs._find_ai_cli_candidates, matching the dependency called by
ai.get_ai_cli_status(). Keep the mocked candidate value and _configured_cli_path
patch unchanged.
| # PODCLI_TOKEN would shadow the file these tests are about. | ||
| env = mock.patch.dict(os.environ, {"PODCLI_TOKEN": "", "PODCLI_AI_PROVIDER": ""}) | ||
| env.start() | ||
| self.addCleanup(env.stop) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: List the environment variables that influence the provider chain.
fd -t f 'ai_provider.py' -x rg -n -C4 'environ' {}Repository: nmbrthirteen/podcli
Length of output: 780
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Locate files"
fd -t f 'ai_provider.py|test_entitlement_chain.py' .
echo
echo "## ai_provider outline"
file=$(fd -t f 'ai_provider.py' . | head -n1 || true)
if [ -n "${file:-}" ]; then
ast-grep outline "$file" --view compact || true
echo
echo "## ai_provider relevant sections"
cat -n "$file" | sed -n '1,180p'
fi
echo
echo "## test file relevant sections"
tfile=$(fd -t f 'test_entitlement_chain.py' . | head -n1 || true)
if [ -n "${tfile:-}" ]; then
cat -n "$tfile" | sed -n '1,120p'
fiRepository: nmbrthirteen/podcli
Length of output: 11466
Clear the remaining chain env vars and patch CLI candidates in the forced-mode test.
_chain() also reads ANTHROPIC_API_KEY for the API backend, so leaving it unset can add the api leg when the suite runs. The forced cloud test also calls _chain() without overriding ai_cli._find_ai_cli_candidates, unlike the other chain tests. Clear ANTHROPIC_API_KEY in setUp and patch the CLI candidates in test_forced_cloud_mode_ignores_a_free_verdict.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_entitlement_chain.py` around lines 20 - 23, Update the test setup
in tests/test_entitlement_chain.py to clear ANTHROPIC_API_KEY alongside the
existing PODCLI_TOKEN and PODCLI_AI_PROVIDER values. In
test_forced_cloud_mode_ignores_a_free_verdict, patch
ai_cli._find_ai_cli_candidates consistently with the other chain tests before
calling _chain().
|
Reopened from a cleaner branch. |
1 participant
Two changes.
One AI backend chain. Discovery, invocation and error classification lived across
claude_suggestand its callers, each finding the CLI again. They now shareai_clifor discovery andai_providerfor choosing a backend. Discovery is cached against the environment it was derived from — each probe shells out to npm, pnpm and yarn for about three seconds, and callers ask several times per render, so this ran for every user at every AI gate.Optional account sync. Signing in reconciles clips, assets and the knowledge base with an account, and uploads rendered clips. Only rendered clips are uploaded; the source video never leaves the machine. Signed out, behaviour is unchanged.
Paths supplied by the server are resolved and confined before anything is written, and knowledge reconciles before it pushes so shipped defaults cannot replace curated files.
Python 595 pass (3 pre-existing failures on main), TypeScript 243 pass, tsc clean, Go build and vet clean.
Summary by CodeRabbit
podcli syncto synchronize clips, media assets, and knowledge files, with conflict and failure reporting.