fix(idl-v2): cycle/duplicate guards and FFI hardening

[ukint-vs]

Closes #1319 (with documentation, not a new API — see thread for the discussion).

Background

The original report asked for a computeInterfaceIds API to avoid the "submit IDL with @0x0000..., paste hex from the validation error, repeat" loop when authoring v2 IDL fixtures. Per @vobradovich's review, the existing parse_idl already returns canonical ids in the AST when callers omit explicit @0x... suffixes on non-@partial services. So this PR closes the issue by documenting that pattern rather than adding a parallel API.

What this PR ships

A handful of correctness fixes that surfaced while exploring the original API design — each one is a real bug or UB on the existing parse_idl_to_json path:

File	Fix
rs/idl-parser-v2/src/post_process.rs	Cycle detection on extends graphs (service A { extends { A } } and longer cycles A → B → A previously stack-overflowed in compute_service_id with no validation guard)
rs/idl-parser-v2/src/post_process.rs	Duplicate-service-name guard at index-build time (BTreeMap previously collapsed the second entry, then trapped later via expect(interface_id must be set))
rs/idl-parser-wasm/src/lib.rs	Replace String::from_utf8_unchecked with safe str::from_utf8 (was UB on any non-UTF-8 input from a raw FFI caller)
rs/idl-parser-wasm/src/lib.rs	Sanitize NUL bytes before CString::new in create_parse_result (validation errors can quote user input containing \0, e.g. @entry_id: 1\0, which previously panicked the WASM module)
rs/idl-parser-wasm/src/lib.rs	Accept zero-length input with a non-null pointer so empty/whitespace-only IDLs do not surface as NullPtr errors

Documentation

docs/idl-v2-spec.md now spells out the strip-and-parse pattern for tooling that needs ids deterministically up front:

1. Generate or load IDL.
2. Strip explicit @0x... suffixes from non-@partial service declarations.
3. Call sails_idl_parser_v2::parse_idl (Rust) or SailsIdlParser.parse(idl) (sails-js).
4. Read each service's interface_id from the returned AST.
5. Optionally re-render the IDL with the computed ids filled in.

@partial services must keep their explicit id — the partial signature is a subset, so the parser cannot recompute the original service's id from it.

Test plan

• cargo test -p sails-idl-parser-v2 --locked — 35 tests pass (3 new regression tests exercise cycle / self-cycle / duplicate-name detection via parse_idl)
• cargo clippy -p sails-idl-parser-v2 -p sails-idl-parser-wasm --all-targets --locked -- -D warnings — clean
• cargo fmt --all --check — clean
• pnpm --filter sails-js-parser-idl-v2 build — clean rebuild
• Jest in js/parser-idl-v2 — 8 tests pass (3 new regression tests for cycle / self-cycle / duplicate names via parse())
• Diff against master: 5 files, +194/-23

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces a new computeInterfaceIds API to the Sails IDL parser, enabling deterministic computation of service identifiers. Key changes include updates to the IDL specification to support an optional @interface_id suffix, implementation of the new logic in both Rust and TypeScript, and enhanced validation to detect duplicate service definitions and cyclic extension graphs. Review feedback highlights a potential fragility in manual JSON construction within the WASM FFI and suggests an optimization to reduce redundant string allocations in the Rust post-processing phase.

[gemini-code-assist]

Manual JSON construction without escaping assumes that service identifiers are strictly restricted to characters that do not require JSON escaping (like double quotes or backslashes). While the IDL grammar likely enforces this, it's a fragile assumption for an FFI boundary. If the grammar is ever expanded to allow more characters in identifiers, this will produce invalid JSON.

[ukint-vs]

Good catch — switched to serde_json::to_string over a BTreeMap<&str, String> in 54e7055. serde_json is already in the WASM blob via sails-idl-ast's serde feature (used by parse_idl_to_json), so the ~13KB cost is for the new monomorphization, not a new dep. Future-proof against any grammar evolution.

[gemini-code-assist]

The service name is being cloned twice here: once via to_string() and then again via clone() for the insert call. Since s.name.name is already a String, you can avoid one allocation by cloning it once and reusing it for both the index insertion and the potential error message.

Suggested change

	let name = s.name.name.to_string();
	if service_index.insert(name.clone(), idx).is_some() {
	let name = s.name.name.clone();
	if service_index.insert(name.clone(), idx).is_some() {

[ukint-vs]

Fixed in the same commit. Used the borrow form in format!() so we keep the single clone BTreeMap::insert actually needs and drop the redundant one.

[vobradovich]

Remove computeInterfaceIds

parse_idl_to_json already returns the post-processed AST JSON with computed interface_ids: parse_idl runs validate_and_post_process, and the JS parse() path normalizes those ids into InterfaceId values. So SailsIdlParser.computeInterfaceIds() mostly duplicates existing surface area. Its only semantic difference is that it ignores explicit non-@partial placeholder/mismatched ids, which also bypasses the normal parse_idl validation behavior. I’d remove the new exported WASM function and JS method, and have callers read parse(idl).services[*].interface_id instead.

IDL text that already contains placeholder or stale explicit ids

parse_idl already computes the canonical id when the suffix is absent. So tooling can do:

1. generate or load IDL,
2. remove explicit service ids from non-@partial service declarations,
3. call parse_idl / parse(),
4. read service.interface_id from the returned AST,
5. optionally render the IDL again with the computed ids filled in.

The only exception is @partial: those must keep an explicit id, because their subset cannot safely recompute the original service id from the partial signature.

Keep cycle detection

The extends cycle detection is still worth keeping. I checked the merge base: old ServiceInterfaceId::compute_service_id only looked in computed and then recursively computed bases, so service A { extends { A } } and A -> B -> A would recurse indefinitely. There was no previous cyclic / visiting validation in the parser path.

[ukint-vs]

@vobradovich Done in 704032f — you were right to push back on the duplicate surface.

Removed: compute_interface_ids (Rust), compute_interface_ids_to_json (WASM FFI), SailsIdlParser.computeInterfaceIds (JS), the dedicated test file, and the direct serde-json dep that the WASM export had pulled in.

Kept (per your request): cycle detection on extends graphs. Also kept the duplicate-name guard, str::from_utf8/NUL-sanitization in the FFI, and zero-length input acceptance — each fixes a real bug or UB on the existing parse_idl_to_json path. Three Rust + three JS regression tests now exercise these via parse_idl / parse().

Docs: rewrote the docs/idl-v2-spec.md paragraph to document the strip-and-parse pattern (omit @0x... from non-@partial services, call parse_idl, read service.interface_id from the AST) in place of the removed API.

Diff against master is now 5 files, +194/-23.