feat(cli): add Bitbucket Cloud auth and read commands (1/2)#912
Open
Nih1tGupta wants to merge 5 commits into
Open
feat(cli): add Bitbucket Cloud auth and read commands (1/2)#912Nih1tGupta wants to merge 5 commits into
Nih1tGupta wants to merge 5 commits into
Conversation
Contributor
📝 WalkthroughWalkthroughAdds Bitbucket Cloud as a first-class CLI integration. New outbound clients handle API token verification and workspace/repository/pull-request fetching. Credential storage is refactored from keychain to local file-backed secrets across all providers. New CLI commands ( ChangesBitbucket Cloud Integration
Sequence Diagram(s)sequenceDiagram
actor User
participant auth_commands
participant bitbucket_auth
participant bitbucket_read
participant credentials_store
participant bitbucket_client
participant bitbucket_read_client
participant BitbucketAPI
rect rgba(100, 149, 237, 0.5)
Note over User,BitbucketAPI: potpie bitbucket login
User->>auth_commands: bitbucket_login(--email, --api-token)
auth_commands->>bitbucket_auth: run_bitbucket_login()
bitbucket_auth->>credentials_store: get_integration_status("bitbucket")
bitbucket_auth->>bitbucket_client: verify_bitbucket_api_token(email, token)
bitbucket_client->>BitbucketAPI: GET /user
bitbucket_client->>BitbucketAPI: GET /user/workspaces
bitbucket_client->>BitbucketAPI: GET /repositories/{workspace}
bitbucket_client-->>bitbucket_auth: BitbucketVerifyResult(ok=True, display_name)
bitbucket_auth->>credentials_store: save_bitbucket_credentials(credentials)
end
rect rgba(144, 238, 144, 0.5)
Note over User,BitbucketAPI: potpie bitbucket select
User->>auth_commands: bitbucket_select(--workspace, --repo, --limit)
auth_commands->>bitbucket_read: run_bitbucket_use_flow(workspace_key, repo_slug)
bitbucket_read->>bitbucket_read_client: fetch_bitbucket_workspaces()
bitbucket_read_client->>BitbucketAPI: GET /user/workspaces
bitbucket_read->>bitbucket_read_client: fetch_bitbucket_repositories(workspace_key)
bitbucket_read_client->>BitbucketAPI: GET /repositories/{workspace}
bitbucket_read->>bitbucket_read_client: fetch_bitbucket_pull_requests(workspace_key, repo_slug)
bitbucket_read_client->>BitbucketAPI: GET /repositories/{workspace}/{repo}/pullrequests
bitbucket_read->>credentials_store: save_bitbucket_workspace_prefs(workspace_key, repo_slug)
bitbucket_read-->>auth_commands: result dict with PR items
auth_commands->>auth_commands: _run_product_use_result("Bitbucket")
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@potpie/context-engine/adapters/inbound/cli/auth/auth_commands.py`:
- Around line 25-29: The import statement for BitbucketReadError and
fetch_bitbucket_workspaces should be changed to import directly from their
source module bitbucket_read_client instead of importing them indirectly through
bitbucket_read. Update the import statement to separate the imports: import
BitbucketReadError and fetch_bitbucket_workspaces from
adapters.inbound.cli.auth.bitbucket_read_client, while keeping
run_bitbucket_use_flow imported from adapters.inbound.cli.auth.bitbucket_read
where it is actually defined. This makes the dependency chain explicit and
clear.
In `@potpie/context-engine/adapters/inbound/cli/auth/bitbucket_auth.py`:
- Around line 81-96: The code uses select.select on stdin which is not supported
on Windows since stdin is not a socket, and the broad except Exception clause
silently masks this platform-specific failure. To address this, add a
platform-specific check before the select.select call in the countdown loop:
check if the system is Windows using sys.platform, and either skip the
select.select call entirely on Windows or implement an alternative approach for
Windows. This will make the behavior explicit and prevent silent failures while
maintaining cross-platform compatibility.
In `@potpie/context-engine/adapters/outbound/cli_auth/bitbucket_read_client.py`:
- Around line 7-9: The function `_basic_auth_header` is a private implementation
detail (indicated by the leading underscore) being imported from
`bitbucket_client` into this module, creating unnecessary coupling. Either
rename `_basic_auth_header` to `basic_auth_header` in the `bitbucket_client`
module to make it a public function and update the import statement in this file
to reflect the new public name, or extract the `_basic_auth_header` function to
a shared utility module that both files can import from independently.
🪄 Autofix (Beta)
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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3b03fcec-af89-4ba6-b35c-23f269ac7662
📒 Files selected for processing (17)
potpie/context-engine/adapters/inbound/cli/auth/auth_commands.pypotpie/context-engine/adapters/inbound/cli/auth/bitbucket_auth.pypotpie/context-engine/adapters/inbound/cli/auth/bitbucket_read.pypotpie/context-engine/adapters/outbound/cli_auth/bitbucket_client.pypotpie/context-engine/adapters/outbound/cli_auth/bitbucket_read_client.pypotpie/context-engine/adapters/outbound/cli_auth/credentials.pypotpie/context-engine/adapters/outbound/cli_auth/credentials_store.pypotpie/context-engine/adapters/outbound/cli_auth/integration_profile.pypotpie/context-engine/adapters/outbound/cli_auth/integration_verify.pypotpie/context-engine/adapters/outbound/cli_auth/provider_config.pypotpie/context-engine/domain/ports/cli_auth/credentials.pypotpie/context-engine/tests/_auth_fakes.pypotpie/context-engine/tests/unit/test_bitbucket_client.pypotpie/context-engine/tests/unit/test_bitbucket_read.pypotpie/context-engine/tests/unit/test_bitbucket_read_client.pypotpie/context-engine/tests/unit/test_cli_atlassian_shared.pypotpie/context-engine/tests/unit/test_credentials_store.py
Comment on lines
+25
to
+29
| from adapters.inbound.cli.auth.bitbucket_read import ( | ||
| BitbucketReadError, | ||
| fetch_bitbucket_workspaces, | ||
| run_bitbucket_use_flow, | ||
| ) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Import BitbucketReadError and fetch_bitbucket_workspaces from their source module.
These symbols are defined in bitbucket_read_client.py but imported here via bitbucket_read.py, which only imports them for internal use without explicitly re-exporting. While this works, it creates an implicit coupling. Import directly from the source for clarity.
♻️ Suggested import adjustment
-from adapters.inbound.cli.auth.bitbucket_read import (
- BitbucketReadError,
- fetch_bitbucket_workspaces,
- run_bitbucket_use_flow,
-)
+from adapters.inbound.cli.auth.bitbucket_read import run_bitbucket_use_flow
+from adapters.outbound.cli_auth.bitbucket_read_client import (
+ BitbucketReadError,
+ fetch_bitbucket_workspaces,
+)📝 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.
Suggested change
| from adapters.inbound.cli.auth.bitbucket_read import ( | |
| BitbucketReadError, | |
| fetch_bitbucket_workspaces, | |
| run_bitbucket_use_flow, | |
| ) | |
| from adapters.inbound.cli.auth.bitbucket_read import run_bitbucket_use_flow | |
| from adapters.outbound.cli_auth.bitbucket_read_client import ( | |
| BitbucketReadError, | |
| fetch_bitbucket_workspaces, | |
| ) |
🤖 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 `@potpie/context-engine/adapters/inbound/cli/auth/auth_commands.py` around
lines 25 - 29, The import statement for BitbucketReadError and
fetch_bitbucket_workspaces should be changed to import directly from their
source module bitbucket_read_client instead of importing them indirectly through
bitbucket_read. Update the import statement to separate the imports: import
BitbucketReadError and fetch_bitbucket_workspaces from
adapters.inbound.cli.auth.bitbucket_read_client, while keeping
run_bitbucket_use_flow imported from adapters.inbound.cli.auth.bitbucket_read
where it is actually defined. This makes the dependency chain explicit and
clear.
Comment on lines
+81
to
+96
| try: | ||
| if sys.stdin.isatty(): | ||
| for remaining in range(timeout_seconds, 0, -1): | ||
| sys.stdout.write(f"\r opening in {remaining}s ...") | ||
| sys.stdout.flush() | ||
| ready, _, _ = select.select([sys.stdin], [], [], 1.0) | ||
| if ready: | ||
| try: | ||
| sys.stdin.readline() | ||
| except Exception: | ||
| pass | ||
| break | ||
| sys.stdout.write("\n") | ||
| sys.stdout.flush() | ||
| except Exception: | ||
| pass |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Platform note: select.select on stdin doesn't work on Windows.
The select.select([sys.stdin], [], [], 1.0) call won't work on Windows (stdin isn't a socket). The broad except Exception catches this gracefully, so the countdown simply won't be interruptible on Windows—it will fall through after the full timeout. This is acceptable but worth noting if Windows CLI users report issues.
🤖 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 `@potpie/context-engine/adapters/inbound/cli/auth/bitbucket_auth.py` around
lines 81 - 96, The code uses select.select on stdin which is not supported on
Windows since stdin is not a socket, and the broad except Exception clause
silently masks this platform-specific failure. To address this, add a
platform-specific check before the select.select call in the countdown loop:
check if the system is Windows using sys.platform, and either skip the
select.select call entirely on Windows or implement an alternative approach for
Windows. This will make the behavior explicit and prevent silent failures while
maintaining cross-platform compatibility.
Comment on lines
+7
to
+9
| from adapters.outbound.cli_auth.bitbucket_client import ( | ||
| _basic_auth_header, | ||
| workspace_slugs_from_list_payload, |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider making _basic_auth_header a public function.
Importing _basic_auth_header (underscore-prefixed) from bitbucket_client couples this module to an implementation detail. Consider either removing the underscore prefix to make it public, or extracting it to a shared utility module.
🤖 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 `@potpie/context-engine/adapters/outbound/cli_auth/bitbucket_read_client.py`
around lines 7 - 9, The function `_basic_auth_header` is a private
implementation detail (indicated by the leading underscore) being imported from
`bitbucket_client` into this module, creating unnecessary coupling. Either
rename `_basic_auth_header` to `basic_auth_header` in the `bitbucket_client`
module to make it a public function and update the import statement in this file
to reflect the new public name, or extract the `_basic_auth_header` function to
a shared utility module that both files can import from independently.
4 tasks
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@potpie/context-engine/adapters/inbound/cli/auth/auth_commands.py`:
- Line 94: The IntegrationAuthProvider literal type is missing "bitbucket" from
its allowed values, which causes a type safety violation when the
bitbucket_login command calls _run_tracked_integration_login with "bitbucket" as
the provider string. Add "bitbucket" to the IntegrationAuthProvider literal
definition on line 94 alongside the existing values "linear", "atlassian",
"jira", and "confluence". Additionally, update the _integration_auth_provider
validation function (around line 439) to include "bitbucket" in its allowed set
of provider strings to ensure consistency and prevent runtime ValueError
exceptions if the bitbucket provider is validated through this function.
🪄 Autofix (Beta)
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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: f0ee3843-69ba-405b-a5ac-82824d46554c
📒 Files selected for processing (1)
potpie/context-engine/adapters/inbound/cli/auth/auth_commands.py
| "confluence", | ||
| "bitbucket", | ||
| ) | ||
| IntegrationAuthProvider = Literal["linear", "atlassian", "jira", "confluence"] |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the bitbucket login command + its telemetry path
fd -t f 'auth_commands.py' --exec sed -n '1180,1305p' {}
echo "=== _run_tracked_integration_login / run_integration_login usages ==="
rg -nP '\b(run_integration_login|_run_tracked_integration_login)\s*\(' --type=py -C3
echo "=== bitbucket telemetry wrapping in auth_commands ==="
rg -nP 'bitbucket' --type=py -C3 -g '*auth_commands.py' | rg -nP 'tracked|capture_integration_auth_event|run_integration_login|provider='Repository: potpie-ai/potpie
Length of output: 3812
🏁 Script executed:
# Check the _integration_auth_provider definition and usage around line 439
fd -t f 'auth_commands.py' --exec sed -n '430,450p' {}Repository: potpie-ai/potpie
Length of output: 862
🏁 Script executed:
# Find the _run_tracked_integration_login function definition
rg -n '_run_tracked_integration_login' --type=py -A 20 | head -50Repository: potpie-ai/potpie
Length of output: 154
🏁 Script executed:
# Check the full IntegrationAuthProvider type and any validation
fd -t f 'auth_commands.py' --exec sed -n '90,100p' {}Repository: potpie-ai/potpie
Length of output: 491
🏁 Script executed:
# Search for explicit validation against integration providers
rg -n 'Unknown integration provider|_integration_auth_provider' --type=py -B 2 -A 2Repository: potpie-ai/potpie
Length of output: 154
🏁 Script executed:
fd -t f 'auth_commands.py' --exec sed -n '440,480p' {}Repository: potpie-ai/potpie
Length of output: 1414
🏁 Script executed:
# Get more context for _run_tracked_integration_login function
rg -n '_run_tracked_integration_login' --type=py -A 30 | head -80Repository: potpie-ai/potpie
Length of output: 154
🏁 Script executed:
# Search for calls to run_integration_login
rg -n 'run_integration_login\s*\(' --type=py -B 2 -A 2Repository: potpie-ai/potpie
Length of output: 154
🏁 Script executed:
# Check what capture_integration_auth_event does with the provider parameter
rg -n 'capture_integration_auth_event|def capture_integration_auth_event' --type=py -A 10 | head -50Repository: potpie-ai/potpie
Length of output: 154
🏁 Script executed:
# Find all files and search for run_integration_login usage
rg 'run_integration_login' --type=pyRepository: potpie-ai/potpie
Length of output: 154
🏁 Script executed:
# Find where capture_integration_auth_event is defined
fd -t f --type=py . | xargs grep -l 'def capture_integration_auth_event' 2>/dev/nullRepository: potpie-ai/potpie
Length of output: 328
🏁 Script executed:
# Check if there are any type: ignore or cast statements related to bitbucket/provider
rg 'bitbucket.*IntegrationAuthProvider|IntegrationAuthProvider.*bitbucket|type: ignore.*bitbucket|type: ignore.*provider' --type=pyRepository: potpie-ai/potpie
Length of output: 154
🏁 Script executed:
# Search for run_integration_login without the type flag
rg 'run_integration_login' | head -20Repository: potpie-ai/potpie
Length of output: 154
🏁 Script executed:
# Find capture_integration_auth_event definition
rg -l 'def capture_integration_auth_event'Repository: potpie-ai/potpie
Length of output: 154
🏁 Script executed:
# Check the bitbucket_login function one more time to confirm the flow
sed -n '1192,1220p' potpie/context-engine/adapters/inbound/cli/auth/auth_commands.pyRepository: potpie-ai/potpie
Length of output: 937
Add "bitbucket" to IntegrationAuthProvider literal and _integration_auth_provider validation set.
The bitbucket_login command calls _run_tracked_integration_login("bitbucket", ...) directly, passing a string not in the IntegrationAuthProvider literal type (line 94: Literal["linear", "atlassian", "jira", "confluence"]). While this doesn't cause a runtime error—_run_tracked_integration_login doesn't validate the provider string—it violates type safety: the parameter is typed as IntegrationAuthProvider, but "bitbucket" is not included.
Additionally, the _integration_auth_provider validation function (line ~439) also excludes "bitbucket" from its allowed set, creating an inconsistency. If any code path inadvertently calls run_integration_login("bitbucket"), it will raise ValueError at runtime.
For consistency and type correctness, add "bitbucket" to both the IntegrationAuthProvider literal and the validation set in _integration_auth_provider.
🤖 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 `@potpie/context-engine/adapters/inbound/cli/auth/auth_commands.py` at line 94,
The IntegrationAuthProvider literal type is missing "bitbucket" from its allowed
values, which causes a type safety violation when the bitbucket_login command
calls _run_tracked_integration_login with "bitbucket" as the provider string.
Add "bitbucket" to the IntegrationAuthProvider literal definition on line 94
alongside the existing values "linear", "atlassian", "jira", and "confluence".
Additionally, update the _integration_auth_provider validation function (around
line 439) to include "bitbucket" in its allowed set of provider strings to
ensure consistency and prevent runtime ValueError exceptions if the bitbucket
provider is validated through this function.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
potpie/context-engine/adapters/outbound/cli_auth/credentials_store.py (1)
748-799: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRollback token writes when metadata persistence fails.
These paths replace the file secret before
_write_metadata_entry(). If metadata writing fails, the new token remains paired with stale or missing metadata; GitHub already handles this with a restore/delete rollback. Apply the same pattern here for Linear, Bitbucket, and Atlassian credential saves.Also applies to: 942-950, 982-990, 1038-1050
🤖 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 `@potpie/context-engine/adapters/outbound/cli_auth/credentials_store.py` around lines 748 - 799, The credential save flow updates the secret file before calling _write_metadata_entry(), so a metadata write failure can leave the new token stored without matching metadata. Update the Linear save path shown here, and the corresponding Bitbucket and Atlassian save paths, to use the same rollback pattern as GitHub: capture the prior secret state before _store_file_secret(), and if _write_metadata_entry() fails, restore or delete the token secret so the secret and metadata stay consistent. Focus on the credential write helpers that build the record and call _write_metadata_entry() for each provider.
🤖 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 `@potpie/context-engine/adapters/outbound/cli_auth/__init__.py`:
- Around line 3-4: The outbound adapter package docstring in
cli_auth/__init__.py still omits Bitbucket from the listed provider auth/flow
clients. Update the module-level description to include Bitbucket alongside the
existing integrations, and keep the wording consistent with the symbols in this
package so the new first-class CLI integration is accurately documented.
In `@potpie/context-engine/adapters/outbound/cli_auth/credentials_store.py`:
- Around line 213-217: `store_secure_secret()` and the shared file mutators are
treating a malformed or transiently unreadable secrets file as empty, which can
overwrite existing integration tokens. Update `_store_integration_file_secret()`
and `_delete_integration_file_secret()` to use a strict read path instead of the
fallback-empty behavior, while keeping the permissive empty fallback only for
read/status flows. Use the existing
`_read_integration_secrets_file()`/secret-store helpers to locate the mutating
logic and route writes/deletes through strict error handling.
---
Outside diff comments:
In `@potpie/context-engine/adapters/outbound/cli_auth/credentials_store.py`:
- Around line 748-799: The credential save flow updates the secret file before
calling _write_metadata_entry(), so a metadata write failure can leave the new
token stored without matching metadata. Update the Linear save path shown here,
and the corresponding Bitbucket and Atlassian save paths, to use the same
rollback pattern as GitHub: capture the prior secret state before
_store_file_secret(), and if _write_metadata_entry() fails, restore or delete
the token secret so the secret and metadata stay consistent. Focus on the
credential write helpers that build the record and call _write_metadata_entry()
for each provider.
🪄 Autofix (Beta)
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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 67a638ca-1bfe-4dcb-bee9-87e237215e30
📒 Files selected for processing (10)
potpie/context-engine/adapters/outbound/cli_auth/__init__.pypotpie/context-engine/adapters/outbound/cli_auth/credentials.pypotpie/context-engine/adapters/outbound/cli_auth/credentials_store.pypotpie/context-engine/bootstrap/cli_auth_wiring.pypotpie/context-engine/domain/ports/cli_auth/credentials.pypotpie/context-engine/tests/unit/test_cli_atlassian.pypotpie/context-engine/tests/unit/test_cli_atlassian_shared.pypotpie/context-engine/tests/unit/test_cli_linear_shared.pypotpie/context-engine/tests/unit/test_credentials_store.pypotpie/context-engine/tests/unit/test_github_cli_auth.py
Comment on lines
+3
to
4
| Persistence (file credential store), HTTP transport, and the | ||
| provider auth/flow clients (GitHub, Firebase, Potpie, Linear, Atlassian) that |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Include Bitbucket in the outbound adapter list.
Line 4 still omits Bitbucket even though this PR adds it as a first-class CLI integration.
-provider auth/flow clients (GitHub, Firebase, Potpie, Linear, Atlassian) that
+provider auth/flow clients (GitHub, Firebase, Potpie, Linear, Atlassian, Bitbucket) that📝 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.
Suggested change
| Persistence (file credential store), HTTP transport, and the | |
| provider auth/flow clients (GitHub, Firebase, Potpie, Linear, Atlassian) that | |
| Persistence (file credential store), HTTP transport, and the | |
| provider auth/flow clients (GitHub, Firebase, Potpie, Linear, Atlassian, Bitbucket) that |
🤖 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 `@potpie/context-engine/adapters/outbound/cli_auth/__init__.py` around lines 3
- 4, The outbound adapter package docstring in cli_auth/__init__.py still omits
Bitbucket from the listed provider auth/flow clients. Update the module-level
description to include Bitbucket alongside the existing integrations, and keep
the wording consistent with the symbols in this package so the new first-class
CLI integration is accurately documented.
Comment on lines
213
to
+217
| def store_secure_secret(name: str, secret: str, *, label: str | None = None) -> None: | ||
| """Store a secret in the system keychain under the Potpie CLI service.""" | ||
| """Store a secret in the local file-backed secret store.""" | ||
| key = _norm_secret_name(name) | ||
| try: | ||
| keyring.set_password(_KEYRING_SERVICE, key, secret) | ||
| except KeyringError as exc: | ||
| raise CredentialStoreError( | ||
| f"Failed to store {label or key} in system keychain: {exc}" | ||
| ) from exc | ||
| except Exception as exc: | ||
| raise CredentialStoreError( | ||
| f"Failed to store {label or key} in system keychain: {exc}" | ||
| ) from exc | ||
| _store_integration_file_secret(key, secret, label=label) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not treat a corrupt secrets file as empty on writes.
store_secure_secret() now writes to the shared local secrets file. If _read_integration_secrets_file() hits malformed JSON or a transient read error, it returns {} and the next store/delete can overwrite or mis-handle all existing integration tokens. Use a strict read path for mutating operations and keep the empty fallback only for read/status flows.
Suggested direction
-def _read_integration_secrets_file() -> dict[str, str]:
+def _read_integration_secrets_file(*, strict: bool = False) -> dict[str, str]:
path = integration_secrets_path()
if not path.is_file():
return {}
try:
raw = path.read_text(encoding="utf-8")
data = json.loads(raw)
- except (OSError, json.JSONDecodeError):
+ except (OSError, json.JSONDecodeError) as exc:
+ if strict:
+ raise CredentialStoreError(
+ f"Failed to read local credentials file: {exc}"
+ ) from exc
return {}Then call the strict mode from _store_integration_file_secret() and _delete_integration_file_secret().
Also applies to: 228-233
🤖 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 `@potpie/context-engine/adapters/outbound/cli_auth/credentials_store.py` around
lines 213 - 217, `store_secure_secret()` and the shared file mutators are
treating a malformed or transiently unreadable secrets file as empty, which can
overwrite existing integration tokens. Update `_store_integration_file_secret()`
and `_delete_integration_file_secret()` to use a strict read path instead of the
fallback-empty behavior, while keeping the permissive empty fallback only for
read/status flows. Use the existing
`_read_integration_secrets_file()`/secret-store helpers to locate the mutating
logic and route writes/deletes through strict error handling.
Nih1tGupta
changed the title
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
1 participant
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.
Summary
bitbucket_auth.pyfor standalone Bitbucket loginpotpie bitbucket login|logout|ls|selectRelated issue
Closes #911
Stack
Part 1 of 2. PR 2 will add
potpie atlassian login|logout|statuson top of this branch.Test plan
pytest tests/unit/test_bitbucket_*.py tests/unit/test_credentials_store.py tests/unit/test_cli_atlassian_shared.py -qpotpie bitbucket --helppotpie jira --help(unchanged)