feat(rust-port): phases 0-1 foundations — Bun-free gjc host binary slice - #3788
feat(rust-port): phases 0-1 foundations — Bun-free gjc host binary slice#3788Yeachan-Heo wants to merge 10 commits into
Conversation
![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: 639953f3bf
ℹ️ 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".
| pub fn load(dirs: &Dirs) -> Self { | ||
| let global = parse_yaml_object(&dirs.settings_path()); | ||
|
|
There was a problem hiding this comment.
When a current global config.yml contains a supported scalar coercion such as images.autoResize: "false" or a numeric setting written as a string, the Bun loader runs reconcileSettingsSchema and returns the corresponding boolean or number, whereas this loader stores the raw YAML string. Consequently gjc config get/list is not byte-identical for these valid legacy-compatible inputs, and future Rust consumers receive a value of the wrong type. Apply the schema reconciliation/coercion step to the global layer before merging it.
Useful? React with 👍 / 👎.
| // TS counts UTF-16 code units; chars here counts Unicode scalars, which | ||
| // agrees for all BMP text (astral characters differ by design). | ||
| let chars = trimmed.chars().count(); |
There was a problem hiding this comment.
For any message containing astral characters such as emoji, chars().count() counts each Unicode scalar once while the TypeScript metric uses trimmed.length and counts its two UTF-16 code units. This silently changes persisted/aggregated user-message statistics during the Rust migration despite the crate's parity contract; for example, "a😀" reports 2 instead of 3. Count encode_utf16() units and add an astral-character parity vector.
Useful? React with 👍 / 👎.
639953f to
b79dd5e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8cc69e93d3
ℹ️ 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".
| for (index, line) in lines.enumerate() { | ||
| let entry: SessionEntry = serde_json::from_str(line) | ||
| .with_context(|| format!("invalid transcript entry at line {}", index + 2))?; | ||
| entries.push(entry); |
There was a problem hiding this comment.
For v4/v5 transcripts containing header_patch or entry_patch records, this loop exposes the patches as ordinary SessionEntry values without applying them. The Bun read path applies valid header patches to the current title/cwd and entry patches to the referenced message, so Rust consumers would instead see stale session metadata or message content plus pseudo-entries in the history. Replay these records into the logical header/entries view while retaining raw records separately if lossless serialization is required.
Useful? React with 👍 / 👎.
| let entry: SessionEntry = serde_json::from_str(line) | ||
| .with_context(|| format!("invalid transcript entry at line {}", index + 2))?; |
There was a problem hiding this comment.
When an interrupted append leaves a truncated record, or any non-header line is otherwise malformed, the ? aborts parsing of the entire transcript. The existing Bun session reader deliberately skips malformed JSONL records and continues with later valid lines, so this turns a recoverable persisted session into an unreadable one during the Rust migration. Match the lenient per-line behavior after validating the header.
Useful? React with 👍 / 👎.
| pub struct SessionHeader { | ||
| #[serde(rename = "type")] | ||
| pub kind: String, | ||
| pub version: u32, |
There was a problem hiding this comment.
For legacy v1 transcripts, version is intentionally absent, but requiring a u32 makes Serde reject the header before the session can be read. The current session loader treats a missing version as v1 and migrates it in memory, so users with older retained sessions would lose access after this reader takes over. Represent the field as optional and interpret absence as version 1.
Useful? React with 👍 / 👎.
Phase 0 of docs/roadmap/full-conversion-to-rust.md: clap+tokio host binary with stubbed run/acp/config commands. Subsystems are ported from TypeScript per the roadmap; browser, background jobs and TS plugins run as subprocesses.
…jc-config Faithful ports of packages/utils/src/env-file.ts and dirs.ts: - dotenv/shell-env parsing with the same identifier and safety rules - GJC_CONFIG_DIR/PI_CONFIG_DIR resolution with .env trust guard and '..'-segment sanitization - agent-dir override and XDG data/state/cache redirection semantics - explicit DirsInput (home/cwd/env) so parity tests can replay any environment deterministically
…onfig CLI - scripts/generate-json-schemas.ts gains a third drift-checked output, schemas/settings-flat.json: the SETTINGS_SCHEMA table flattened in definition order with type/default/enum values/ui description+tab - gjc-config embeds the flat table (settings_schema), ports the read path of settings.ts (deepMerge, project notification stripping, builtin .gjc/settings.json + config.yml project layering, schema default fallback) and the secret-redaction rules from config-cli.ts - crates/gjc wires 'gjc config list|get|path' (read-only; set/reset/ doctor stay Bun-side for now) Parity gate: 'gjc config list --json', plain list, get, secret-redacted get, and path are byte-identical between the Bun CLI and the Rust binary against a real ~/.gjc.
Read-only ports from packages/coding-agent/src/session: - scope: managed v2 directory naming (scopeDigest sha256+base32 with TS-reference parity vectors), binding-file parsing with unknown-field preservation, scope listing - transcript: JSONL header/entry parsing with loss-free round-trips (unknown fields kept via serde flatten; explicit parentId null preserved), transcript listing newest-first Verified against a live ~/.gjc store: 36 transcripts / 11902 entries round-trip with semantic equality. Golden fixture + smoke tests cover the synthetic path in CI.
…-stats gjc-util (ports of packages/utils leaf modules with unit tests): - format.ts: duration/number/bytes/age/percent formatting with the same truncate-below-boundary and clamp semantics, pluralization rules - spawn-env.ts: env name/value safety and malloc-logging filtering - snowflake.ts: 22-bit-sequence hex snowflakes (format/parse/bounds) - mime.ts: PNG/JPEG/GIF/WebP header metadata sniffing gjc-stats: - user-metrics.ts: full behavioral-metrics port (profanity/anguish/ negation/repetition/blame/yelling) with the JS lookbehind and backreference constructs rewritten for the regex crate; validated against 15 parity vectors generated from the TS implementation (checked into tests/fixtures) Skipped modules and rationale are documented in each crate's lib.rs (runtime-coupled layers, SQLite storage, ai-typed parser/aggregator).
…es-core Establishes the roadmap phase 1 pattern: algorithmic cores move to the new NAPI-free pi-natives-core crate while pi-natives keeps thin facades with unchanged NAPI export names and signatures (Bun runtime on dev is unaffected; cargo check -p pi-natives stays green). Migrated in this step: glob_util (string errors in core, NAPI error conversion in the facade), hashline (UTF-16 core with byte-identical TS parity vectors from hashline/hash.ts; JsString/Utf16String interop stays in the facade), linediff (jsdiff-parity algorithm in core, NAPI object re-wrap in the facade). crates/gjc now depends on pi-natives-core; cargo tree confirms napi is absent from both the gjc and pi-natives-core dependency trees. The remaining modules migrate module-by-module (tracked in the ultragoal ledger and roadmap).
The repo's rustfmt.toml relies on nightly-only options (hard tabs, alignment thresholds, format_strings); the new crates were initially formatted with a stable rustfmt. Reformat with the rust-toolchain pinned nightly so cargo fmt --check passes in CI. No behavior change; the gjc-session smoke test's binding literal is rebuilt with json! to survive format_strings.
8cc69e9 to
26e873c
Compare
…-scope allowlist The Dev CI affected-path validator fails closed when the Cargo workspace contains unclassified members; register gjc, gjc-config, gjc-session, gjc-stats, gjc-util, and pi-natives-core (none are NAPI addon sources) and add their rust-scope rationales.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26e873c190
ℹ️ 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".
| Ok(Self { | ||
| home, | ||
| cwd: std::env::current_dir()?, | ||
| env: std::env::vars().collect(), |
There was a problem hiding this comment.
On Unix, an unrelated environment variable may contain bytes that are not valid UTF-8, and std::env::vars() panics when it encounters one. Because DirsInput::from_process() runs before every implemented config action, such an entry prevents the Rust CLI from starting even though none of the relevant GJC/XDG variables are affected; iterate with vars_os() and skip or explicitly decode unusable entries instead.
Useful? React with 👍 / 👎.
| let rest = trimmed | ||
| .strip_prefix("export ") | ||
| .map_or(trimmed, str::trim_start); |
There was a problem hiding this comment.
For shell startup lines such as export OPENAI_API_KEY=... or export\tOPENAI_API_KEY=..., the TypeScript parser's export\s+ prefix accepts the assignment, while stripping only the literal "export " leaves leading whitespace or export\t in the Rust key and causes validation to discard it. This loses credentials and other environment settings for valid, commonly formatted shell assignments; parse the optional export token followed by one or more whitespace characters.
Useful? React with 👍 / 👎.
| format!("{}B", js_round(n as f64 / 1_000_000_000.0)) | ||
| } | ||
|
|
||
| /// Format with up to 1 decimal place, dropping a trailing `.0`. |
There was a problem hiding this comment.
Rust's fixed-precision formatter uses round-half-even, whereas the TypeScript implementation uses toFixed(1). At exact ties this changes observable output—for example, format_number(1_250) produces 1.2K here but formatNumber(1_250) produces 1.3K—breaking the stated formatting parity for token, request, and download counts; implement the JavaScript rounding behavior before formatting.
Useful? React with 👍 / 👎.
| if char_count <= max_len { | ||
| return s.to_owned(); | ||
| } | ||
| let slice_len = max_len.saturating_sub(ellipsis.chars().count()); |
There was a problem hiding this comment.
When the input contains astral characters, iterating Rust chars no longer matches JavaScript's UTF-16 length and slice contract. For example, truncating "😀abc" to 3 with "…" returns "😀a…" here but "😀…" in TypeScript, so the result can exceed the original length limit and change user-visible previews; perform the limit calculation and cut in UTF-16 code units.
Useful? React with 👍 / 👎.
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
REQUEST_CHANGES — signed owner-batch hostile review
Exact binding
- PR head:
2adb118587e19afec633f7feffdf9a8dc832e3c7(feat/full-conversion-to-rust) - PR-recorded base / merge base:
38e026c785968e722e5b3a1b8025cadfc54c8c84 - Current target
dev:732856b3ccb3fade6e9fbc17908a4fbca5a7682f - Divergence from the recorded base: current
dev+22 commits; PR +10 commits - Shape: draft, 45 files, +6,376 / -545,
mergeable=false,mergeable_state=dirty - Concrete merge conflict:
packages/coding-agent/src/internal-urls/docs-index.generated.ts; rebase must retain both live-dev docs and the Rust roadmap, then regenerate the full docs index.
Exact CI
- PR-head checks on
2adb1185: 32 success / 6 skipped / 0 failure (run30801035984). These are exact-head but not current-base evidence; the PR is now dirty against livedev. - Current
devchecks on732856b3: 23 success / 5 skipped / 3 failure (run30935832940). The primary failure istest:@gajae-code/coding-agent:shard-8-of-8atsdk-operation-inventory.test.ts:66(Expected: 0,Received: 1); the other two failures are aggregate/evidence jobs. This failure does not overlap the PR's changed paths, but there is no fresh exact-head/current-base green run.
Contributor ledger
- Author association:
OWNER; same-repository head. - All 10 current commits are authored as
Yeachan-Heo <yeachan-heo@gajae.dev>. - No co-author trailers and no external contributor credit to preserve.
- All 10 commits are currently reported unsigned; recorded as ledger provenance, not used as a standalone blocker.
- The PR body's commit table names pre-rewrite SHAs (
663d001f9,2545063ed, …), not the current 10-commit chain (01ba7052…2adb1185), so the published milestone ledger is stale.
Hostile review verdict
Six independent GPT-heavy read-only lanes all returned REQUEST_CHANGES. The nine existing automated review comments were independently confirmed; none has a reply or disposition.
Blocking findings:
-
Config trust boundary — HIGH
crates/gjc-config/src/dirs.rs:80-101: POSIX sanitization accepts..\\outside, then the custom join splits backslashes and escapes the home-relative config root.crates/gjc-config/src/env_file.rs:66-70: any unrelated invalid UTF-8 byte makes.envparse as empty, allowing a preloaded project override to bypass the equality-based trust guard.- Existing findings are confirmed: raw global settings skip schema reconciliation;
std::env::vars()can panic on unrelated non-UTF-8 entries; shell parsing rejects valid multiple-space/tabexportforms.
-
Session compatibility — HIGH
crates/gjc-session/src/transcript.rs:69-82: v4/v5header_patchandentry_patchrecords are exposed raw rather than replayed.crates/gjc-session/src/transcript.rs:70-80: one malformed append record aborts the whole transcript instead of preserving lenient recovery.crates/gjc-session/src/transcript.rs:21-34: valid versionless v1 headers are rejected while unsupported future versions are accepted without a version gate.- Additional parity drift: discovery trusts filename identity and creation timestamp ordering rather than validated headers and file mtime.
-
Formatting/stats parity — MEDIUM
- Existing findings are confirmed at
crates/gjc-util/src/format.rs:52-73,115-122andcrates/gjc-stats/src/user_metrics.rs:557-560: JS decimal rounding and UTF-16 length/count contracts are not preserved. - Additional classifier drift uses Rust Unicode word/whitespace semantics where the TypeScript baseline uses ECMAScript semantics.
- Existing findings are confirmed at
-
CLI/roadmap contract — HIGH
docs/roadmap/full-conversion-to-rust.md:53-54checks off byte-identical config parity, butcrates/gjc/src/cli.rs/config_cmd.rschange bare-config defaults, missing-key behavior, exit codes, and TTY scope.- The host claims subprocess ownership while
run/acpare stubs, and-p/--promptis accepted but never consumed.
-
Admission/generator gates — HIGH
scripts/check-rust-scope.tsonly discoverscrates/*/Cargo.tomlwhile accepting every Rust descendant of an allowlisted root; a nested standalone crate bypasses package/path admission.- The new general product/pure-logic allowlist entries contradict the file's stated native/OS/measured-hot-path policy without an explicit policy replacement.
schemas/settings-flat.jsonis Rust-consumed, but generator/artifact-only changes are not mapped to a Rust consumer test.
Required before re-sign
- Rebase onto live
dev, resolve the generated docs-index conflict by regeneration, and refresh the PR body/commit ledger. - Fix or explicitly rebut every existing review comment and each HIGH finding above; silent drops are not accepted.
- Make roadmap/CLI claims match observable behavior.
- Close the nested-crate admission hole and settle the Rust-scope policy change explicitly.
- Obtain fresh terminal CI bound to the new exact head against the then-current base, followed by another signed hostile review.
VERDICT: REQUEST_CHANGES
— Signed: GJC owner batch / GPT-heavy hostile review, 2026-08-05
|
Triage status: LEFT OPEN — not mergeable in current form. This is the healthiest of the open drafts (green exact-head CI, additive, phased roadmap), but it is dirty against Verified state (head
Remaining plan requested (please reply with the intended next steps): (1) rebase onto live |
1 participant
Note
Draft until the Rust binary is daily-usable (roadmap phase 6 TUI parity, or earlier if the hybrid Rust-core + Bun-TUI-client path is taken after phases 2-3). Phases 0-1 here are foundation only: nothing in the user-facing runtime is routed through Rust yet.
Summary
First slice of the full conversion to a single Rust binary (see
docs/roadmap/full-conversion-to-rust.md, added in this PR). The Rustgjchost binary boots, resolves config with byte-identical parity to the Bun CLI, and reads the existing session store losslessly. All changes are additive: the Bun runtime ondevis unaffected (NAPI export names/signatures unchanged;pi-nativesis touched in exactly 4 files).Commits → roadmap milestones
663d001f9crates/gjchost binary skeleton (tokio + clap) + roadmap doc2545063edgjc-config—env-file.ts/dirs.tsports (config-dir precedence,.envtrust guards, XDG)d346ddf12gjc config list|get|path— byte-identical to the Bun CLI on a real~/.gjc(schemas/settings-flat.jsonis drift-checked bygenerate-json-schemas.ts --check)b0fe360ddgjc-session— managed v2 scope naming (TS digest parity vectors), binding files, loss-free JSONL transcript round-trips (validated on 36 real transcripts / 11,902 entries)8067f7643gjc-util(format/spawn-env/snowflake/mime) +gjc-stats(user-metrics with 15 TS parity vectors)dcb160ed7pi-natives-coreestablished — NAPI-free core crate with glob_util/hashline/linediff migrated behind unchanged NAPI facades;cargo tree -p gjccontains zero napi; roadmap checkboxes updatedb79dd5e9dVerification
cargo testfor all new crates: 58 tests passing (incl. 16 inpi-natives-core)cargo clippy --all-targetson every new/touched crate: zero warnings under the strict workspace lintscargo check -p pi-natives: green (NAPI facade intact)rustfmt --checkon all new/touched files: cleanbun scripts/generate-json-schemas.ts --check: generated schemas in syncdiff <(gjc config list --json) <(target/debug/gjc config list --json)plus plain/get/secret-redacted/path variants — all byte-identicalDeliberate deferrals (documented in crate lib.rs docs + roadmap)