Conversation
|
@ddarkr is attempting to deploy a commit to the Inevitable Team on Vercel. A member of the Team first needs to authorize it. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 10 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/tokscale-cli/src/tui/data/mod.rs">
<violation number="1" location="crates/tokscale-cli/src/tui/data/mod.rs:1067">
P2: Pricing regression tests were weakened from exact expected costs to generic positive/finite checks, reducing ability to catch cost-calculation regressions.</violation>
</file>
<file name="crates/tokscale-cli/src/antigravity.rs">
<violation number="1" location="crates/tokscale-cli/src/antigravity.rs:373">
P1: Session artifact filenames are derived from a lossy sanitized session ID, allowing collisions between distinct session IDs and silent artifact overwrite.</violation>
<violation number="2" location="crates/tokscale-cli/src/antigravity.rs:710">
P2: Non-zero discovery command exits are silently treated as empty output, masking real detection failures.</violation>
<violation number="3" location="crates/tokscale-cli/src/antigravity.rs:727">
P1: Per-connection RPC failure in summary collection aborts the entire antigravity sync instead of skipping the bad connection.</violation>
<violation number="4" location="crates/tokscale-cli/src/antigravity.rs:1022">
P2: Chunked body parsing treats chunk-size parse failures as terminal zero-length chunks, which can prematurely truncate valid responses (e.g., with chunk extensions).</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d7c24c61f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let response = rpc_request( | ||
| connection, | ||
| "GetAllCascadeTrajectories", | ||
| &serde_json::json!({}), | ||
| )?; |
There was a problem hiding this comment.
list_trajectory_summaries currently propagates any single rpc_request error with ?, so tokscale antigravity sync aborts even if other detected language-server connections are healthy. This is user-visible when one editor instance exits or transiently fails between heartbeat probing and trajectory listing: one bad process prevents exporting sessions from all remaining processes. Treat per-connection RPC failures as best-effort (skip/log) to keep sync resilient.
Useful? React with 👍 / 👎.
| .get("createdAt") | ||
| .or_else(|| usage.get("timestamp")) | ||
| .and_then(Value::as_str) | ||
| .and_then(|text| chrono::DateTime::parse_from_rfc3339(text).ok()) |
There was a problem hiding this comment.
Timestamp extraction for retry usage only accepts RFC3339 strings via as_str, so numeric createdAt/timestamp fields become null in emitted JSONL rows. The antigravity parser later drops usage rows with non-positive timestamps, which silently undercounts tokens/cost whenever the RPC payload uses epoch numbers instead of strings. Add numeric timestamp handling here before falling back to created_at.
Useful? React with 👍 / 👎.
Skip broken trajectory RPC connections, make session artifact filenames collision-resistant, improve discovery diagnostics, and parse chunked responses without truncation. Restore the exact RooCode cost regression assertion to keep coverage strong.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/tokscale-cli/src/antigravity.rs">
<violation number="1" location="crates/tokscale-cli/src/antigravity.rs:373">
P2: Artifact filename migration introduces coexistence of legacy and new files, and downstream Antigravity ingestion scans `sessions/*.jsonl` directly, so both copies may be parsed.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12d2b21b96
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let mut ports = parse_ports(&run_command( | ||
| "lsof", | ||
| &["-Pan", "-p", &pid_str, "-iTCP", "-sTCP:LISTEN"], | ||
| )?); |
There was a problem hiding this comment.
Connection discovery hard-fails when lsof is unavailable because find_listening_ports propagates run_command("lsof", ...) errors with ?. On systems where lsof is not installed (common in minimal Linux setups), tokscale antigravity status/sync will abort as soon as an Antigravity process candidate is found, so users cannot refresh cache data despite having a running editor. This should degrade to a warning and skip that process instead of failing the whole command.
Useful? React with 👍 / 👎.
Remove stale legacy session artifacts after sync, tolerate missing lsof during port discovery, and preserve numeric retry timestamps in exported metadata.
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/tokscale-cli/src/antigravity.rs">
<violation number="1" location="crates/tokscale-cli/src/antigravity.rs:202">
P1: Stale artifacts are deleted before manifest persistence, so a save failure can leave the old manifest pointing to files that were already removed.</violation>
<violation number="2" location="crates/tokscale-cli/src/antigravity.rs:202">
P1: Sync cleanup deletes manifest-sourced artifact paths without constraining them to the cache root, enabling path traversal/absolute-path file deletion outside the antigravity cache.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20a5ac4f82
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let artifact_path = cache_dir.join(relative_path); | ||
| if artifact_path.exists() { | ||
| fs::remove_file(&artifact_path)?; |
There was a problem hiding this comment.
delete_artifact_relative_path trusts artifact_path from the manifest and directly does cache_dir.join(relative_path) before remove_file. Because join accepts absolute paths and .. segments, a corrupted/edited manifest can cause sync cleanup to delete files outside ~/.config/tokscale/antigravity-cache (for example an absolute path or ../../...). This makes artifact cleanup unsafe against malformed local state and can lead to unintended data loss.
Useful? React with 👍 / 👎.
| let ports = find_listening_ports(candidate.pid)?; | ||
| for port in ports { |
There was a problem hiding this comment.
Connection discovery only iterates ports returned by find_listening_ports, so candidates are dropped when lsof is unavailable/restricted or returns no rows. Fresh evidence in this commit: ProcessCandidate.declared_port is parsed from --extension_server_port but never used during probing, which means antigravity status/sync can still report zero connections even though the process advertises a usable RPC port.
Useful? React with 👍 / 👎.
Persist the manifest before deleting stale artifacts, restrict cleanup to cache-relative paths, and probe declared RPC ports when lsof discovery returns nothing.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/tokscale-cli/src/antigravity.rs">
<violation number="1" location="crates/tokscale-cli/src/antigravity.rs:413">
P2: `cleanup_stale_session_artifacts` deletes any relative path from the prior manifest as long as it’s under the cache root. The new `resolve_cache_relative_artifact_path` only blocks absolute/`..` components, so entries like `manifest.json` are considered valid and can delete arbitrary cache-root files (including the freshly saved manifest after the order change). Cleanup should restrict deletions to `sessions/*.jsonl` (or canonicalize and enforce the sessions dir) to avoid deleting non-artifacts.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1660f1be3f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
crates/tokscale-core/src/clients.rs
Outdated
| relative: ".config/tokscale/antigravity-cache/sessions", | ||
| pattern: "*.jsonl", | ||
| headless: false, | ||
| parse_local: false |
There was a problem hiding this comment.
ClientId::Antigravity is registered with parse_local: false, but default local parse paths (e.g., parse_local_unified_messages and run_clients_command) build their source list from ClientId::iter().filter(|c| c.parse_local()). That means synced Antigravity cache files are omitted unless callers explicitly pass "antigravity", so default local workflows can silently miss this new client’s data. Make Antigravity parse-local (or explicitly include it in those defaults) so the new integration is visible without manual source overrides.
Useful? React with 👍 / 👎.
Limit stale artifact deletion to session jsonl files under the cache root and include Antigravity in default local parsing so synced sessions are visible without explicit source filters.
1 participant
Summary
tokscale antigravity status|sync|purge-cacheso Antigravity usage can be collected from the local language server while the editor is runningtokscale-corefocused on parsing local data instead of live RPC collectionTest proof
cargo test— passedcargo test -p tokscale-cli antigravity— passedcargo test -p tokscale-core antigravity— passedcargo run -p tokscale-cli -- antigravity status— verified sync-first UXcargo run -p tokscale-cli -- antigravity sync— verified cache sync flowcargo run -p tokscale-cli -- models --antigravity --json— verified parsed reporting pathSummary by cubic
Adds Antigravity support:
tokscale antigravitysyncs usage from the local language server into a cached JSONL store, andtokscale-coreparses it with canonical model aliases for accurate pricing. Runtokscale antigravity syncwhile the editor is open; synced usage appears in local reports by default (optional--antigravityfilter, TUI hotkeya).New Features
tokscale antigravitysubcommands:status,sync,purge-cache.~/.config/tokscale/antigravity-cache/sessions/*.jsonl; manifest preserves prior confirmed artifacts.antigravityclient (scans cached JSONL), parser, and pricing alias normalization for placeholders (e.g.,model_placeholder_m26→claude-opus-4-6).--antigravityfilter and TUI entry (hotkeya).Bug Fixes
.jsonlpaths, and remove stale legacy artifacts after sync.lsof, probe declared RPC ports when port discovery returns nothing, and improvestatus/syncdiagnostics.Written for commit 135ac58. Summary will update on new commits.