Add Codex OAuth model auth#715
Open
plgonzalezrx8 wants to merge 4 commits into
Open
Conversation
Contributor
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (3)
⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughImplements Codex OAuth support and a Responses-based Codex backend: credential loading/refresh, client/registry/runtime wiring, a new CodexResponsesBackend, extensive tests, documentation/config updates, agent integration examples, and a zsh oh-my-posh init script. ChangesCodex OAuth Feature Implementation
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![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: 1
🧹 Nitpick comments (3)
src/llm/backends/codex.py (1)
149-149: 💤 Low valueAdd comment explaining intentionally unused parameters.
The assignment
_ = (max_tokens, temperature, thinking_budget_tokens)silences unused-variable warnings but doesn't explain why these are unused. A brief comment would help future maintainers.📝 Suggested documentation
- _ = (max_tokens, temperature, thinking_budget_tokens) + # These params are intentionally unused: Codex Responses API derives + # these from the model's native configuration. + _ = (max_tokens, temperature, thinking_budget_tokens)🤖 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/llm/backends/codex.py` at line 149, Add a brief inline comment explaining that max_tokens, temperature, and thinking_budget_tokens are intentionally unused (e.g., because this backend ignores those options or they are handled elsewhere) next to the assignment `_ = (max_tokens, temperature, thinking_budget_tokens)` in src/llm/backends/codex.py so future maintainers understand why the variables are being intentionally discarded.src/llm/codex_oauth.py (2)
96-122: ⚖️ Poor tradeoffFile lock held during external HTTP call.
The file lock acquired at line 96 is held throughout the token refresh HTTP call (lines 114-117). While the timeout bounds the worst case, this could cause lock contention if multiple processes attempt refresh simultaneously and the OAuth endpoint is slow. Consider releasing the lock before the HTTP call and re-acquiring it for the 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 `@src/llm/codex_oauth.py` around lines 96 - 122, The code currently holds _auth_file_lock while making the external HTTP call refresh_codex_access_token; instead, after reading payload via _read_auth_payload and determining a refresh is needed (using _extract_tokens and _access_token_is_expiring), capture refresh_token, release the lock, perform refresh_codex_access_token(timeout_seconds=refresh_timeout_seconds), then re-acquire _auth_file_lock and re-read the payload/tokens to detect any race (another process may have refreshed already); if still needs updating merge the refreshed token values into payload["tokens"], set payload["last_refresh"] = int(time.time()), and call _write_auth_payload; keep checks for missing refresh_token and raise ValidationException as before.
139-148: 💤 Low valueHTTP response accessed after client context exit.
The
responseobject is accessed (lines 150-167) after thewith httpx.Client(...) as client:block exits. While this typically works because the response body is already buffered, it's safer to move the response processing inside the context manager.♻️ Move response processing inside context manager
timeout = httpx.Timeout(max(5.0, float(timeout_seconds))) with httpx.Client(timeout=timeout, headers={"Accept": "application/json"}) as client: response = client.post( CODEX_OAUTH_TOKEN_URL, headers={"Content-Type": "application/x-www-form-urlencoded"}, data={ "grant_type": "refresh_token", "refresh_token": refresh_token, "client_id": CODEX_OAUTH_CLIENT_ID, }, ) + status_code = response.status_code + try: + response_body = response.json() + except ValueError: + response_body = None - if response.status_code != 200: - message = f"Codex OAuth token refresh failed with status {response.status_code}." + if status_code != 200: + message = f"Codex OAuth token refresh failed with status {status_code}." try: - raw_body: Any = response.json() + raw_body: Any = response_body except ValueError: body = None🤖 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/llm/codex_oauth.py` around lines 139 - 148, The code creates an httpx.Client in a with block and then uses the response after the context exits; to fix this, move all response processing (inspecting response.status_code, calling response.json(), raising errors or extracting tokens) inside the with httpx.Client(...) as client: block where response is created. Specifically, keep the POST to CODEX_OAUTH_TOKEN_URL (using refresh_token and CODEX_OAUTH_CLIENT_ID) as-is but perform response.json() parsing and any status checks / exception handling before the with block ends so the response I/O is handled while the client is still active.
🤖 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 `@src/llm/backends/codex.py`:
- Around line 125-126: Replace the bare "except Exception" that logs "Codex
Responses stream did not expose final usage" with explicit exception handlers
(for example catch httpx.RequestError / httpx.HTTPError and the SDK-specific
error your Codex client raises, e.g., CodexSDKError or OpenAIError) in the same
block (the try that calls get_final_response() or finalization on the Codex
responses stream); for each specific except, log the exception details (use
logger.debug with exc_info=True or include str(e)) so the actual failure is
recorded, and only keep a very narrow fallback except if you re-raise or log the
full traceback.
---
Nitpick comments:
In `@src/llm/backends/codex.py`:
- Line 149: Add a brief inline comment explaining that max_tokens, temperature,
and thinking_budget_tokens are intentionally unused (e.g., because this backend
ignores those options or they are handled elsewhere) next to the assignment `_ =
(max_tokens, temperature, thinking_budget_tokens)` in src/llm/backends/codex.py
so future maintainers understand why the variables are being intentionally
discarded.
In `@src/llm/codex_oauth.py`:
- Around line 96-122: The code currently holds _auth_file_lock while making the
external HTTP call refresh_codex_access_token; instead, after reading payload
via _read_auth_payload and determining a refresh is needed (using
_extract_tokens and _access_token_is_expiring), capture refresh_token, release
the lock, perform
refresh_codex_access_token(timeout_seconds=refresh_timeout_seconds), then
re-acquire _auth_file_lock and re-read the payload/tokens to detect any race
(another process may have refreshed already); if still needs updating merge the
refreshed token values into payload["tokens"], set payload["last_refresh"] =
int(time.time()), and call _write_auth_payload; keep checks for missing
refresh_token and raise ValidationException as before.
- Around line 139-148: The code creates an httpx.Client in a with block and then
uses the response after the context exits; to fix this, move all response
processing (inspecting response.status_code, calling response.json(), raising
errors or extracting tokens) inside the with httpx.Client(...) as client: block
where response is created. Specifically, keep the POST to CODEX_OAUTH_TOKEN_URL
(using refresh_token and CODEX_OAUTH_CLIENT_ID) as-is but perform
response.json() parsing and any status checks / exception handling before the
with block ends so the response I/O is handled while the client is still active.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 116cb25c-3625-4f63-8a41-a70abb142898
📒 Files selected for processing (19)
.env.templateREADME.mdconfig.toml.exampledocs/v3/contributing/configuration.mdxdocs/v3/contributing/self-hosting.mdxdocs/v3/contributing/troubleshooting.mdxsrc/config.pysrc/llm/backends/__init__.pysrc/llm/backends/codex.pysrc/llm/codex_oauth.pysrc/llm/credentials.pysrc/llm/executor.pysrc/llm/registry.pysrc/llm/runtime.pytests/llm/test_backends/test_codex.pytests/llm/test_codex_oauth.pytests/llm/test_credentials.pytests/llm/test_model_config.pytests/llm/test_registry.py
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 (3)
src/config.py (1)
101-120:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject conflicting
transport+ prefixed model shorthands here.If someone sets
transport="anthropic"withmodel="codex/gpt-5"(or any other prefixed shorthand), this validator now leaves the mismatch untouched instead of failing fast. That pushes an invalid provider/model pair downstream with the wrong auth settings and a much less obvious error.💡 Suggested fix
if isinstance(model_value, str) and "/" in model_value: prefix, bare_model = model_value.split("/", 1) if prefix in {"anthropic", "openai", "gemini"}: - if transport_value in {None, prefix}: - update["transport"] = prefix - update["model"] = bare_model - elif prefix in {"openai-codex", "codex"} and transport_value in { - None, - "openai", - }: + if transport_value not in {None, prefix}: + raise ValueError( + f"model prefix '{prefix}/' requires transport='{prefix}'" + ) + update["transport"] = prefix + update["model"] = bare_model + elif prefix in {"openai-codex", "codex"}: + if transport_value not in {None, "openai"}: + raise ValueError( + f"model prefix '{prefix}/' requires transport='openai'" + ) update["transport"] = "openai" update["model"] = bare_model update["auth_mode"] = "codex_oauth"As per coding guidelines, "Use explicit error handling with appropriate exception types in Python code."
🤖 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/config.py` around lines 101 - 120, In src/config.py adjust the validator logic that parses model_value with a "/" prefix (the block handling prefix, bare_model, transport_value and the codex branch) to raise a clear exception when an explicit transport_value conflicts with the inferred provider (e.g., transport_value not in {None, expected_prefix}), instead of silently leaving the mismatch; for the codex branch apply the same conflict check for expected "openai" transport, and after computing overrides (the overrides variable built from update.get("overrides")) assign it back into update["overrides"] and set update["auth_mode"] as before so auth settings are applied atomically; use an explicit ValueError with a descriptive message naming model_value, transport_value, and the expected provider.src/llm/codex_oauth.py (1)
138-148:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCatch transport failures before
responseexists.In
refresh_codex_access_token(src/llm/codex_oauth.py),client.post(...)isn’t wrapped in error handling—network/timeout issues causehttpxto raise (e.g.,httpx.RequestError/ timeout subclasses) before anyresponseexists, so rawhttpxexceptions can leak instead of the module’sValidationExceptioncontract.💡 Suggested fix
timeout = httpx.Timeout(max(5.0, float(timeout_seconds))) - with httpx.Client(timeout=timeout, headers={"Accept": "application/json"}) as client: - response = client.post( - CODEX_OAUTH_TOKEN_URL, - headers={"Content-Type": "application/x-www-form-urlencoded"}, - data={ - "grant_type": "refresh_token", - "refresh_token": refresh_token, - "client_id": CODEX_OAUTH_CLIENT_ID, - }, - ) + try: + with httpx.Client( + timeout=timeout, headers={"Accept": "application/json"} + ) as client: + response = client.post( + CODEX_OAUTH_TOKEN_URL, + headers={"Content-Type": "application/x-www-form-urlencoded"}, + data={ + "grant_type": "refresh_token", + "refresh_token": refresh_token, + "client_id": CODEX_OAUTH_CLIENT_ID, + }, + ) + except httpx.HTTPError as exc: + raise ValidationException( + "Codex OAuth token refresh failed due to a network error" + ) from exc🤖 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/llm/codex_oauth.py` around lines 138 - 148, The call to client.post in refresh_codex_access_token can raise httpx transport exceptions before a response is returned; wrap the POST (the client.post to CODEX_OAUTH_TOKEN_URL) in a try/except that catches httpx.RequestError (and related timeout subclasses) and re-raises the module's ValidationException with an informative message and the original exception attached; ensure the timeout/httpx.Client creation remains but the network call is guarded so no raw httpx exceptions escape the refresh_codex_access_token function.src/llm/tool_loop.py (1)
351-357:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReuse the same attempt plan within an iteration.
Line 356 already resolves a plan, but Lines 426 and 465 resolve it again in the same iteration. With async planning, this can repeat Codex OAuth credential I/O and may yield a different client than the one that produced
response.Suggested fix
@@ - async def _call_with_messages( + last_plan: AttemptPlan | None = None + + async def _call_with_messages( effective_tool_choice: str | dict[str, Any] | None = effective_tool_choice, conversation_messages: list[dict[str, Any]] = conversation_messages, iteration_for_call: int = iteration + 1, ) -> HonchoLLMCallResponse[Any]: + nonlocal last_plan plan = await get_attempt_plan() + last_plan = plan return await honcho_llm_call_inner( @@ - winning_plan = await get_attempt_plan() + winning_plan = last_plan if last_plan is not None else await get_attempt_plan() @@ - current_provider = (await get_attempt_plan()).provider + current_provider = ( + last_plan.provider if last_plan is not None else (await get_attempt_plan()).provider + )Also applies to: 426-427, 465-465
🤖 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/llm/tool_loop.py` around lines 351 - 357, get_attempt_plan() is being awaited multiple times in the same iteration (once in _call_with_messages and again later), which can produce different clients; resolve the plan once and reuse it: call plan = await get_attempt_plan() once at the start of the iteration scope, then pass that plan into honcho_llm_call_inner (and any other places that currently call get_attempt_plan()), or change those call sites to accept a plan parameter instead of re-awaiting get_attempt_plan(); remove the duplicate awaits so the single resolved plan (variable name plan) is reused for the iteration.
🧹 Nitpick comments (2)
src/llm/backends/codex.py (1)
453-476: ⚡ Quick winDon’t widen the object
typecheck for reasoning items (SDK usestype: "reasoning").The OpenAI Responses SDK typed model for a reasoning output item uses
type: Literal["reasoning"](“Alwaysreasoning”), so the current object branch (item.type == "reasoning") shouldn’t drop valid reasoning details. If you want consistency, consider switching the dict branch fromstartswith("reasoning")to an exact"reasoning"match instead.🤖 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/llm/backends/codex.py` around lines 453 - 476, In _response_reasoning_details, narrow the dict-path type check to only accept exact "reasoning" (replace the item_type.startswith("reasoning") logic with item_type == "reasoning") so it matches the SDK's Literal["reasoning"] behavior; keep the object branch logic that checks getattr(item, "type", None) == "reasoning" as-is and ensure you still cast and append the dict when matched; use CodexResponsesBackend._response_output_items to locate the loop to update.tests/llm/test_registry.py (1)
97-97: ⚡ Quick winMake resolver-call assertion failure clearer before indexing
calls.
calls[0]can raiseIndexErrorand hide intent. Assert non-empty first, then checkauth_path.♻️ Suggested change
- assert calls[0]["auth_path"] == "/tmp/auth.json" + assert calls, "Expected resolve_codex_oauth_credentials to be called" + assert calls[0]["auth_path"] == "/tmp/auth.json"🤖 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/llm/test_registry.py` at line 97, The test currently indexes calls[0] directly which can raise IndexError and obscure the real failure; first assert that the list or iterable calls is non-empty (e.g., assert calls or assert len(calls) > 0) before asserting the specific field, then perform the existing assertion on calls[0]["auth_path"] == "/tmp/auth.json" so failures clearly indicate an empty resolver call list versus an incorrect auth_path.
🤖 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
@.agents/skills/honcho-integration/references/bot-frameworks/nanobot/session.py:
- Around line 155-166: The cache hydration path sorts and normalizes timestamps
using _created_at_utc but get_or_create still calls msg.created_at.isoformat(),
which will raise AttributeError when created_at is a string; update the
hydration logic that builds local_messages (and the branches around
existing_messages sorting for session_id) to reuse the normalized timestamp
produced by _created_at_utc (or a shared helper) instead of calling isoformat()
directly on msg.created_at, ensuring both the sorting block and the
get_or_create flow populate local_messages with the same safe, normalized
timestamp value.
- Around line 181-185: The change appends a hash suffix (suffix) to sanitized
IDs (normalized) which will break lookup of existing persisted Honcho records;
update the code that resolves peer/session IDs to be backward-compatible by
first attempting the new ID format (f"{normalized}_{suffix}") and falling back
to the legacy sanitized-only ID (normalized) when a lookup fails, or implement a
one-time migration that renames existing stored keys from the legacy sanitized
id to the new hashed id; locate the ID generation/lookup code that uses id_str,
normalized and suffix and add the fallback lookup/migration path so existing
conversation history and user memory remain accessible.
---
Outside diff comments:
In `@src/config.py`:
- Around line 101-120: In src/config.py adjust the validator logic that parses
model_value with a "/" prefix (the block handling prefix, bare_model,
transport_value and the codex branch) to raise a clear exception when an
explicit transport_value conflicts with the inferred provider (e.g.,
transport_value not in {None, expected_prefix}), instead of silently leaving the
mismatch; for the codex branch apply the same conflict check for expected
"openai" transport, and after computing overrides (the overrides variable built
from update.get("overrides")) assign it back into update["overrides"] and set
update["auth_mode"] as before so auth settings are applied atomically; use an
explicit ValueError with a descriptive message naming model_value,
transport_value, and the expected provider.
In `@src/llm/codex_oauth.py`:
- Around line 138-148: The call to client.post in refresh_codex_access_token can
raise httpx transport exceptions before a response is returned; wrap the POST
(the client.post to CODEX_OAUTH_TOKEN_URL) in a try/except that catches
httpx.RequestError (and related timeout subclasses) and re-raises the module's
ValidationException with an informative message and the original exception
attached; ensure the timeout/httpx.Client creation remains but the network call
is guarded so no raw httpx exceptions escape the refresh_codex_access_token
function.
In `@src/llm/tool_loop.py`:
- Around line 351-357: get_attempt_plan() is being awaited multiple times in the
same iteration (once in _call_with_messages and again later), which can produce
different clients; resolve the plan once and reuse it: call plan = await
get_attempt_plan() once at the start of the iteration scope, then pass that plan
into honcho_llm_call_inner (and any other places that currently call
get_attempt_plan()), or change those call sites to accept a plan parameter
instead of re-awaiting get_attempt_plan(); remove the duplicate awaits so the
single resolved plan (variable name plan) is reused for the iteration.
---
Nitpick comments:
In `@src/llm/backends/codex.py`:
- Around line 453-476: In _response_reasoning_details, narrow the dict-path type
check to only accept exact "reasoning" (replace the
item_type.startswith("reasoning") logic with item_type == "reasoning") so it
matches the SDK's Literal["reasoning"] behavior; keep the object branch logic
that checks getattr(item, "type", None) == "reasoning" as-is and ensure you
still cast and append the dict when matched; use
CodexResponsesBackend._response_output_items to locate the loop to update.
In `@tests/llm/test_registry.py`:
- Line 97: The test currently indexes calls[0] directly which can raise
IndexError and obscure the real failure; first assert that the list or iterable
calls is non-empty (e.g., assert calls or assert len(calls) > 0) before
asserting the specific field, then perform the existing assertion on
calls[0]["auth_path"] == "/tmp/auth.json" so failures clearly indicate an empty
resolver call list versus an incorrect auth_path.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cf1e9cf1-0c84-4011-b0b9-6ba1eec8e917
📒 Files selected for processing (18)
.agents/skills/honcho-integration/SKILL.md.agents/skills/honcho-integration/references/bot-frameworks/nanobot/client.py.agents/skills/honcho-integration/references/bot-frameworks/nanobot/honcho_tool.py.agents/skills/honcho-integration/references/bot-frameworks/nanobot/session.pyinit.2340073612492328463.zshsrc/config.pysrc/llm/api.pysrc/llm/backends/codex.pysrc/llm/codex_oauth.pysrc/llm/executor.pysrc/llm/registry.pysrc/llm/runtime.pysrc/llm/tool_loop.pytests/llm/test_backends/test_codex.pytests/llm/test_codex_oauth.pytests/llm/test_executor_codex.pytests/llm/test_model_config.pytests/llm/test_registry.py
✅ Files skipped from review due to trivial changes (1)
- .agents/skills/honcho-integration/SKILL.md
Comment on lines
+155
to
+166
| timestamps = [ | ||
| self._created_at_utc(m) | ||
| for m in existing_messages | ||
| if getattr(m, "created_at", None) | ||
| ] | ||
| if timestamps and timestamps != sorted(timestamps): | ||
| logger.warning( | ||
| f"Honcho messages not chronologically ordered for session '{session_id}', sorting" | ||
| ) | ||
| existing_messages = sorted( | ||
| existing_messages, | ||
| key=self._created_at_utc, |
Contributor
There was a problem hiding this comment.
Reuse the normalized timestamp when hydrating cached messages.
These new branches explicitly handle created_at arriving as a string, but get_or_create() still calls msg.created_at.isoformat() when it builds local_messages. The same payloads that need _created_at_utc() will still raise AttributeError during cache hydration.
💡 Suggested follow-up outside this hunk
for msg in existing_messages:
role = "assistant" if msg.peer_id == assistant_peer_id else "user"
+ timestamp = ""
+ if getattr(msg, "created_at", None):
+ timestamp = self._created_at_utc(msg).isoformat()
local_messages.append({
"role": role,
"content": msg.content,
- "timestamp": msg.created_at.isoformat() if msg.created_at else "",
+ "timestamp": timestamp,
"_synced": True, # Already in Honcho
})Also applies to: 188-199
🤖 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
@.agents/skills/honcho-integration/references/bot-frameworks/nanobot/session.py
around lines 155 - 166, The cache hydration path sorts and normalizes timestamps
using _created_at_utc but get_or_create still calls msg.created_at.isoformat(),
which will raise AttributeError when created_at is a string; update the
hydration logic that builds local_messages (and the branches around
existing_messages sorting for session_id) to reuse the normalized timestamp
produced by _created_at_utc (or a shared helper) instead of calling isoformat()
directly on msg.created_at, ensuring both the sorting block and the
get_or_create flow populate local_messages with the same safe, normalized
timestamp value.
Comment on lines
+181
to
+185
| normalized = re.sub(r"[^a-zA-Z0-9_-]+", "-", id_str).strip("-") | ||
| if not normalized: | ||
| normalized = "id" | ||
| suffix = hashlib.sha256(id_str.encode("utf-8")).hexdigest()[:8] | ||
| return f"{normalized}_{suffix}" |
Contributor
There was a problem hiding this comment.
This changes every persisted Honcho ID.
Adding a hash suffix to every sanitized peer/session ID means Honcho records created with the previous mapping will no longer be found after upgrade. That severs existing conversation history and user memory unless you keep a backward-compatible lookup path or migrate legacy IDs.
🤖 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
@.agents/skills/honcho-integration/references/bot-frameworks/nanobot/session.py
around lines 181 - 185, The change appends a hash suffix (suffix) to sanitized
IDs (normalized) which will break lookup of existing persisted Honcho records;
update the code that resolves peer/session IDs to be backward-compatible by
first attempting the new ID format (f"{normalized}_{suffix}") and falling back
to the legacy sanitized-only ID (normalized) when a lookup fails, or implement a
one-time migration that renames existing stored keys from the legacy sanitized
id to the new hashed id; locate the ID generation/lookup code that uses id_str,
normalized and suffix and add the fallback lookup/migration path so existing
conversation history and user memory remain accessible.
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
Validation
Summary by CodeRabbit
New Features
Documentation
Tests