fix(engine): paper-cut cluster — BigQuery JWT cache, sql transpile, wasm UTF-8, explain TOML, partial-success exit code#292
Merged
hugocorreia90 merged 1 commit intomainfrom Apr 29, 2026
Conversation
…asm UTF-8, explain TOML, partial-success exit code Five small, isolated bug fixes bundled into one PR. - rocky-bigquery (auth): cache the exchanged Google access token on ServiceAccount auth (mirrors the Databricks RwLock + 60s slack pattern), so each API call no longer mints + exchanges a fresh JWT. Adds invalidate_cache() for the post-401 case. - rocky-sql (transpile): replace the naive String::replace pipeline with a string/comment-aware walk plus ASCII word boundaries on type tokens. Fixes corruption inside string literals, line/block comments, and identifier substrings (e.g. INT no longer matches inside BIGINT). - rocky-wasm (byte_offset_to_line_col): clamp the offset to text.len() and walk back to the nearest char boundary so a multi-byte UTF-8 character at end of buffer (or a deliberately mid-char offset from adversarial LSP/WASM input) cannot panic. - rocky-ai (explain.save_intent_to_config): on TOML parse failure, return Err rather than silently overwriting the file with a fresh intent-only document. Preserves whatever the user was editing. - rocky-cli (run) + rocky (main): wire a PartialFailure sentinel so partial-success runs (status = partial_failure) exit with code 2 while total failures keep exit code 1. Mirrors the existing Interrupted -> 130 downcast. Restores the documented dagster allow_partial=True contract. Tests: - rocky-bigquery: cache-hit, expired-token, invalidate clears cache. - rocky-sql: word-boundary INT vs BIGINT, NVL/LEN/INT64 inside string literals, line comments, block comments, doubled-quote escapes. - rocky-wasm: offset past end, multi-byte char at EOF, mid-char offset. - rocky-ai: refuses to clobber unparseable TOML; merges into valid TOML; creates fresh sidecar when missing. cargo test -p rocky-bigquery -p rocky-sql -p rocky-wasm -p rocky-ai -p rocky-cli passes; cargo clippy --all-targets -- -D warnings clean; cargo fmt --check clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Five small, isolated bug fixes from the maintenance sweep — each fixes a subtle bug in already-shipped code. Bundled because they all share that shape and don't interact.
P1.1 BigQuery JWT not cached (
rocky-bigquery/src/auth.rs)Service Account auth was minting + exchanging a fresh JWT on every API call (~100ms/call). Added an
Arc<RwLock<Option<CachedToken>>>keyed by theBigQueryAuthinstance, mirroring the Databricks pattern (60sREFRESH_SLACK). The cache uses Google'sexpires_infrom the token-exchange response. Newinvalidate_cache()for post-401 retries. TheServiceAccountenum variant is now struct-form{ key, cached_token }.P1.2
rocky-sqltranspile corruption (rocky-sql/src/transpile.rs)String::replacewalked the whole SQL body, corrupting string literals, line/block comments, and identifier substrings (e.g. mappingINT64 → BIGINTincorrectly hit insideINT-prefixed words). Added a single-pass walker that tracksLineComment/BlockComment/'…'/"…"/`…`state (with doubled-quote escape handling), and requires ASCII word boundaries on type tokens. Function tokens (e.g.NVL() are still literal but skip strings/comments.P1.3 WASM
byte_offset_to_line_colpanic (rocky-wasm/src/lib.rs)An offset past
source.len()or mid-char into a multi-byte UTF-8 character would panic the slice. Clamp totext.len()and walk back to the nearest char boundary viais_char_boundary.P1.4
explain.save_intent_to_configclobber (rocky-ai/src/explain.rs)When the existing TOML sidecar failed to parse, the prior
unwrap_or(empty)silently overwrote the file with a freshintent = "..."document, deleting the user's in-flight edits. Now returnsErr(io::Error::other("refusing to overwrite unparseable TOML at <path>: …")).P1.5 Partial-success exit code 2 (
rocky-cli/src/commands/run.rs,rocky/src/main.rs)Documented contract said partial failures exit 2, but the post-JSON
bail!always returned exit 1, so dagsterallow_partial=Truecouldn't distinguish partial from total failure. Defined aPartialFailuresentinel next toInterrupted, branched thebail!site onderive_run_status(), and added the downcast inmain.rs(PartialFailure → exit 2, mirroringInterrupted → exit 130). Total failure (no tables copied) still exits 1.Test plan
cd engine && cargo test -p rocky-bigquery -p rocky-sql -p rocky-wasm -p rocky-ai -p rocky-cli— all green (incl. new tests for each fix).cd engine && cargo clippy --all-targets -- -D warningsclean.cd engine && cargo fmt --checkclean.cargo testworkspace-wide — no regressions.New regression tests:
read_fresh_token,invalidate_cacheclears the entry,invalidate_cacheis a no-op onBearer.INTvsBIGINT), function/type replacement skips string literals + line + block comments, doubled-quote escape inside literals round-trips.é), mid-char offset (日本語).No
*Outputstructs touched, so nojust codegencascade. No DSL change.