From 62782afca6fb45b77132006e6835ca622d65fc1b Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Sun, 16 Aug 2026 18:09:41 +0300 Subject: [PATCH] feat(ci): guard against silently inlined vendor crates, re-vendor tinydocs Commit 3ee5a3cad de-vendored tinywallet and tinydocs and inlined their sources into the tree, leaving the manifest comments and AGENTS.md still describing the crate-based design. An inlined copy of a shared crate is a silent fork: tinywallet's copy accrued a SLIP-10 key-derivation bug that no other host ever saw. tinywallet was restored in #5533; tinydocs was still inlined, with four Cargo.toml comment blocks instructing readers to run `git submodule update --init vendor/tinydocs` against a submodule that did not exist. Add scripts/ci/check-vendored-crates.mjs (lane: Vendored Crates Gate). A crate is "claimed as vendored" by a manifest comment, a `path = vendor/` dependency, or an id in modules::registry::ALL; the guard asserts each claim is backed by a real gitlink, a .gitmodules entry, and a path dependency. Three details are load-bearing: - Gitlinks are read from `git ls-files --stage` (mode 160000), not the filesystem. On disk an uninitialised submodule and an inlined crate are indistinguishable, which is exactly the state this guard exists to catch. - Exit 2 means "the guard could not run" (no claims parsed, or no gitlinks under vendor/ at all), so a broken scanner fails loudly instead of passing vacuously. - Waivers require a reason string and are staleness-checked: a waiver for a crate that is properly vendored fails as STALE, so the allow-list cannot quietly accumulate. Re-vendor tinydocs at v0.1.13 (6a07dbe), the release the module registry already pins. The inlined copy was diffed against that tag before deletion: the difference is entirely mechanical path rewrites and rustfmt, src/error/ byte-identical, so no local fixes needed to go upstream first. Host policy stays host-side (artifact pipeline, spawn_blocking hop, GENERATION_TIMEOUT, DocumentError::GenerationTimeout) along with the From catch-all arm the #[non_exhaustive] enum requires. Closes #5559 Co-authored-by: Medulla --- .github/workflows/ci-lite.yml | 29 ++ .gitmodules | 3 + AGENTS.md | 53 ++- Cargo.lock | 9 + Cargo.toml | 7 +- app/src-tauri/Cargo.lock | 9 + scripts/__tests__/vendored-crates.test.mjs | 316 +++++++++++++++ scripts/ci/check-vendored-crates.mjs | 161 ++++++++ scripts/lib/vendored-crates.mjs | 332 ++++++++++++++++ src/openhuman/modules/documents.rs | 4 +- src/openhuman/modules/documents_tests.rs | 4 +- .../tools/impl/document/format/error/mod.rs | 120 ------ .../tools/impl/document/format/error/test.rs | 54 --- .../tools/impl/document/format/mod.rs | 29 -- .../impl/document/format/spec/document/mod.rs | 261 ------------- .../document/format/spec/document/test.rs | 272 ------------- .../impl/document/format/spec/image/mod.rs | 161 -------- .../impl/document/format/spec/image/test.rs | 151 ------- .../tools/impl/document/format/spec/mod.rs | 45 --- .../document/format/spec/presentation/mod.rs | 340 ---------------- .../document/format/spec/presentation/test.rs | 367 ------------------ .../document/format/spec/presentation/wire.rs | 73 ---- src/openhuman/tools/impl/document/mod.rs | 1 - src/openhuman/tools/impl/document/types.rs | 60 ++- .../tools/impl/presentation/engine.rs | 11 +- src/openhuman/tools/impl/presentation/mod.rs | 4 +- .../tools/impl/presentation/types.rs | 2 +- vendor/tinydocs | 1 + 28 files changed, 954 insertions(+), 1925 deletions(-) create mode 100644 scripts/__tests__/vendored-crates.test.mjs create mode 100644 scripts/ci/check-vendored-crates.mjs create mode 100644 scripts/lib/vendored-crates.mjs delete mode 100644 src/openhuman/tools/impl/document/format/error/mod.rs delete mode 100644 src/openhuman/tools/impl/document/format/error/test.rs delete mode 100644 src/openhuman/tools/impl/document/format/mod.rs delete mode 100644 src/openhuman/tools/impl/document/format/spec/document/mod.rs delete mode 100644 src/openhuman/tools/impl/document/format/spec/document/test.rs delete mode 100644 src/openhuman/tools/impl/document/format/spec/image/mod.rs delete mode 100644 src/openhuman/tools/impl/document/format/spec/image/test.rs delete mode 100644 src/openhuman/tools/impl/document/format/spec/mod.rs delete mode 100644 src/openhuman/tools/impl/document/format/spec/presentation/mod.rs delete mode 100644 src/openhuman/tools/impl/document/format/spec/presentation/test.rs delete mode 100644 src/openhuman/tools/impl/document/format/spec/presentation/wire.rs create mode 160000 vendor/tinydocs diff --git a/.github/workflows/ci-lite.yml b/.github/workflows/ci-lite.yml index 644e4d5823..d80029c8c7 100644 --- a/.github/workflows/ci-lite.yml +++ b/.github/workflows/ci-lite.yml @@ -974,6 +974,33 @@ jobs: - name: Verify the desktop shell forwards every default-ON core gate run: node scripts/ci/check-feature-forwarding.mjs + vendored-crates-gate: + name: Vendored Crates Gate (no silently inlined crates) + runs-on: ubuntu-22.04 + timeout-minutes: 5 + steps: + - name: Checkout code + uses: actions/checkout@v7 + with: + fetch-depth: 1 + persist-credentials: false + # Deliberately NOT `submodules: recursive`. The check reads the git + # INDEX (mode 160000), so an uninitialised submodule is indistinguishable + # from an initialised one — and an inlined crate is distinguishable from + # both. Cloning ~10 submodules to answer that would be pure cost. + + # Fail when a crate the build describes as vendored has been inlined into + # the tree instead. `3ee5a3cad` did exactly that to tinywallet, leaving the + # manifest comments intact; the resulting silent fork hid a SLIP-10 + # key-derivation bug from every other host until #5533 (#5559). + # + # Deliberately NOT filtered on `changes`: the drift can be introduced by + # editing a manifest, .gitmodules, the module registry, or by deleting a + # gitlink, and a skipped job counts as a pass in the gate below. The check + # is a second of pure Node with no dependencies and no Rust toolchain. + - name: Verify every crate documented as vendored really is + run: node scripts/ci/check-vendored-crates.mjs + pester-install: name: PowerShell Install Test (Pester) needs: [changes] @@ -1043,6 +1070,7 @@ jobs: - tinycortex-tests - orch-ip-gate - feature-forwarding-gate + - vendored-crates-gate if: always() runs-on: ubuntu-latest timeout-minutes: 15 @@ -1063,6 +1091,7 @@ jobs: ["TinyCortex Memory Tests"]="${{ needs['tinycortex-tests'].result }}" ["Orchestration IP Gate"]="${{ needs['orch-ip-gate'].result }}" ["Feature Forwarding Gate"]="${{ needs['feature-forwarding-gate'].result }}" + ["Vendored Crates Gate"]="${{ needs['vendored-crates-gate'].result }}" ) failed=0 diff --git a/.gitmodules b/.gitmodules index 40407d23c9..1305f8876a 100644 --- a/.gitmodules +++ b/.gitmodules @@ -27,3 +27,6 @@ [submodule "vendor/tinywallet"] path = vendor/tinywallet url = https://github.com/tinyhumansai/tinywallet +[submodule "vendor/tinydocs"] + path = vendor/tinydocs + url = https://github.com/tinyhumansai/tinydocs diff --git a/AGENTS.md b/AGENTS.md index 3cef8465b6..1d65ea2839 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -199,7 +199,7 @@ crates are therefore synchronous, I/O-free, and runtime-free. | Crate | Owns | OpenHuman keeps | | --- | --- | --- | -| `tinydocs` | the `.docx` spec types, their size limits, validation, and OOXML synthesis (`docx-rs` sits behind it) | the artifact pipeline, the `spawn_blocking` hop, and the generation deadline — `src/openhuman/tools/impl/document/` | +| `tinydocs` | the `.docx` / `.pptx` spec types, their size limits, and validation. OOXML synthesis lives behind its `docx`/`pptx`/`pdf` gates and this host does **not** enable them — it takes the crate with `default-features = false`, for the wire contract only, and synthesis runs in the TinyBus module | the artifact pipeline, the `spawn_blocking` hop, and the generation deadline — `src/openhuman/tools/impl/document/` | | `tinywallet` | the BTC / EVM / Solana / Tron address formats: parsing, validation, encoding conversions | RPC endpoint resolution, transaction assembly and broadcast, key custody — `src/openhuman/web3/` | Consequences worth knowing before touching either seam: @@ -232,6 +232,57 @@ Consequences worth knowing before touching either seam: `default-features = false` — the wire contract, not the writers, which run in the TinyBus module instead (see the module host section). +#### The vendoring guard — `scripts/ci/check-vendored-crates.mjs` + +**An inlined copy of a vendored crate is a silent fork, and this has already +happened once.** On 2026-08-12 `3ee5a3cad` removed the `vendor/tinywallet` +submodule and inlined ~3,700 lines of crate source into +`src/openhuman/web3/wallet/`, rewriting every `crate::` path and collapsing the +crate's granular chain gates onto OpenHuman's single `web3` gate. It was +collateral damage from a larger change, not a decision — the manifest comments +and this file kept describing the crate-based design, so code and docs +contradicted each other and nothing failed. Four fixes then accrued in +OpenHuman's copy that no other host saw, one of them a SLIP-10 bug where a path +segment already carrying the hardening bit was OR-ed with it again, so +`m/44'/501'/2147483648'` and `m/44'/501'/0'` derived **the same key** +(#5533 / tinywallet#16 / tinywallet#17). `tinydocs` was in exactly that state +until #5559 — four comment blocks describing a submodule that did not exist, +including a `git submodule update --init vendor/tinydocs` that could not work. + +The guard asserts, for every crate a manifest comment describes as vendored, +every `path = "…/vendor/"` dependency, and every id in +`modules::registry::ALL`: a `.gitmodules` entry, a **gitlink in the git index** +(mode 160000 — this is the assertion an inlining trips), and a declared path +dependency. It reads the index rather than the filesystem, so the lane needs no +`git submodule update --init` and no Rust toolchain. + +Exemptions live in `INTENTIONALLY_NOT_VENDORED` / `VENDORED_IN_TREE` in +`scripts/lib/vendored-crates.mjs` and **require a reason string** — the same +rule as `INTENTIONALLY_NOT_FORWARDED`, and for the same reason: an explicit +exclusion is the only thing that keeps "deliberate" distinguishable from +"forgotten". Both lists are staleness-checked, so a waiver that stopped being +true fails rather than quietly covering the next regression. Today they hold +`tinyjuice` and `tinyvoice` (module-only — the host links neither crate) and +`motosan-ai-oauth` (in-tree source, no upstream repo). + +#### TinyWallet's host-side residue is deliberate — do not re-litigate it + +Signing and transaction building moved into the `tinywallet` module, and what +stayed behind is a deliberate remainder rather than unfinished migration. The +host takes the crate with `default-features = false` and **`tx-codec` rather +than `tx`**: `tx` is the only gate that pulls the `bitcoin` crate and its +native secp256k1 C build, while `tx-codec` gives the verification half +(`recompute_txid`, `verify_contract`, `digest`, `attach_signature`) with only +`sha2`. `key`, `asset`, `client`, `tx` and `x402` are dropped. The `web3` gate +therefore now sheds 5 crates, not the 25 it once did; the residue +(`bech32`, `keccak`, `ripemd`, `sha3`, `tinywallet`) is the wire contract, +address validation and transaction *verification*, plus RPC endpoint +resolution, assembly/broadcast and key custody on the OpenHuman side. The three +`tinywallet::key::derive` call sites are all `#[cfg(test)]` — production derives +inside the module. Prefer `verify_contract` over `verify_transfer` at any new +call site: the latter is a substring scan over the hex that a decoy field or a +substituted amount defeats. + ### Backend API access — `src/api/` over `tinyhumans-sdk` Calls to the TinyHumans cloud backend go through the vendored diff --git a/Cargo.lock b/Cargo.lock index dfa9b9d2a4..98e957b575 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4187,6 +4187,7 @@ dependencies = [ "tinychannels", "tinycortex", "tinycortex-api", + "tinydocs", "tinyflows", "tinyhumans-sdk", "tinymemory", @@ -6458,6 +6459,14 @@ dependencies = [ "uuid", ] +[[package]] +name = "tinydocs" +version = "0.1.13" +dependencies = [ + "serde", + "thiserror 2.0.18", +] + [[package]] name = "tinyflows" version = "0.8.0" diff --git a/Cargo.toml b/Cargo.toml index cd8bb7852d..639c8edadd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -474,6 +474,11 @@ unicode-width = { version = "0.2", optional = true } # After cloning: `git submodule update --init vendor/tinydocs`. # # Optional: exclusive to the default-ON `documents` feature. +# +# The submodule is pinned at the tag whose module artifact `modules::registry` +# pins (v0.1.13). Host and module then validate against one revision of the +# spec rather than two that merely agree today. +tinydocs = { path = "vendor/tinydocs", default-features = false, optional = true } # TinyWallet — host-agnostic multi-chain wallet primitives. Owns the address # formats themselves: parsing, validation, and the conversions between their @@ -717,7 +722,7 @@ inference = ["dep:cpal"] # reference instead of extracted text # (`agent::multimodal::extract_pdf_text`). Slim / headless builds opt out via # `--no-default-features --features ""`. -documents = ["modules"] +documents = ["modules", "dep:tinydocs"] # The dynamic module host (`openhuman::modules`): the loader that admits a # compiled `cdylib` through tinybus's ABI descriptor, manifest, dependency and # SHA-256 gates, plus the `modules` RPC namespace and the registry of modules diff --git a/app/src-tauri/Cargo.lock b/app/src-tauri/Cargo.lock index d8ab40c6f5..48e5a39bb0 100644 --- a/app/src-tauri/Cargo.lock +++ b/app/src-tauri/Cargo.lock @@ -4427,6 +4427,7 @@ dependencies = [ "tinychannels", "tinycortex", "tinycortex-api", + "tinydocs", "tinyflows", "tinyhumans-sdk", "tinymemory", @@ -7145,6 +7146,14 @@ dependencies = [ "uuid", ] +[[package]] +name = "tinydocs" +version = "0.1.13" +dependencies = [ + "serde", + "thiserror 2.0.20", +] + [[package]] name = "tinyflows" version = "0.8.0" diff --git a/scripts/__tests__/vendored-crates.test.mjs b/scripts/__tests__/vendored-crates.test.mjs new file mode 100644 index 0000000000..475571a4fc --- /dev/null +++ b/scripts/__tests__/vendored-crates.test.mjs @@ -0,0 +1,316 @@ +import assert from 'node:assert/strict'; +import { execFileSync, spawnSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; +import { test } from 'node:test'; +import { fileURLToPath } from 'node:url'; + +import { + checkVendoredCrates, + collectDeclaredDependencies, + collectDocumentedClaims, + collectModuleIds, + parseGitmodulesPaths, + splitTomlComments, +} from '../lib/vendored-crates.mjs'; + +const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '../..'); +const CHECKER = resolve(REPO_ROOT, 'scripts/ci/check-vendored-crates.mjs'); + +// ── claim collection ─────────────────────────────────────────────────────── + +test('a "git submodule update --init" comment is a vendoring claim', () => { + const toml = ` +# After cloning: \`git submodule update --init vendor/tinydocs\`. +tinydocs = { path = "vendor/tinydocs" } +`; + assert.deepEqual([...collectDocumentedClaims(toml).keys()], ['tinydocs']); +}); + +test('a "Vendored as a git submodule" comment is a vendoring claim', () => { + const toml = '# Vendored as a git submodule under `vendor/tinywallet`.\n'; + assert.deepEqual([...collectDocumentedClaims(toml).keys()], ['tinywallet']); +}); + +test('a passing mention of a vendor path is not a claim', () => { + // The shell manifest points at a file inside a vendored Tauri fork purely to + // say where the code lives. Treating that as "this build consumes the crate + // from vendor/tauri-cef" would make the guard fire on prose. + const toml = "# Tauri's vendored dev-server proxy (see `vendor/tauri-cef/.../tauri.rs`)\n"; + assert.deepEqual([...collectDocumentedClaims(toml).keys()], []); +}); + +test('claims survive a comment that trails a real dependency line', () => { + const toml = 'foo = { path = "x" } # submodule at vendor/tinybus\n'; + assert.deepEqual([...collectDocumentedClaims(toml).keys()], ['tinybus']); +}); + +test('a "#" inside a quoted value does not become a comment', () => { + const { code, comments } = splitTomlComments('a = "issue #4901" # real comment\n'); + assert.match(code, /issue #4901/); + assert.equal(comments.length, 1); + assert.match(comments[0], /real comment/); +}); + +test('sub-crate and relative vendor paths collapse onto the vendor root', () => { + const toml = ` +tinymemory-core = { path = "vendor/tinymemory/core" } +tinyplace = { path = "vendor/tinyplace/sdk/rust" } +tinyagents = { path = "../../vendor/tinyagents" } +`; + assert.deepEqual([...collectDeclaredDependencies(toml)].sort(), [ + 'tinyagents', + 'tinymemory', + 'tinyplace', + ]); +}); + +test('a commented-out dependency does not count as declared', () => { + const toml = '# tinydocs = { path = "vendor/tinydocs" }\n'; + assert.deepEqual([...collectDeclaredDependencies(toml)], []); +}); + +test('module ids come out of the compiled-in registry', () => { + const rs = ` +const TINYDOCS: ModuleRecord = ModuleRecord { + id: "tinydocs", + version: "0.1.13", +}; +const TINYWALLET: ModuleRecord = ModuleRecord { + id: "tinywallet", +}; +`; + assert.deepEqual([...collectModuleIds(rs)].sort(), ['tinydocs', 'tinywallet']); +}); + +test('gitmodules paths are parsed', () => { + const text = '[submodule "vendor/tinybus"]\n\tpath = vendor/tinybus\n\turl = https://x\n'; + assert.deepEqual([...parseGitmodulesPaths(text)], ['vendor/tinybus']); +}); + +// ── the assertions ───────────────────────────────────────────────────────── + +/** The healthy shape: submodule, gitlink, path dependency. */ +function vendoredWorld(name = 'tinydocs') { + return { + claims: new Map([[name, { sources: ['a comment in Cargo.toml'], evidence: 'x' }]]), + submodulePaths: new Set([`vendor/${name}`]), + gitlinkPaths: new Set([`vendor/${name}`]), + trackedVendorDirs: new Set(), + declaredDependencies: new Set([name]), + notVendored: {}, + inTree: {}, + }; +} + +test('a properly vendored crate passes', () => { + const result = checkVendoredCrates(vendoredWorld()); + assert.equal(result.ok, true); + assert.deepEqual(result.checked, ['tinydocs']); +}); + +test("3ee5a3cad's shape fails: the comment still claims a submodule, the source is inlined", () => { + // Exactly what `3ee5a3cad` left behind for tinywallet: the manifest comment + // and AGENTS.md kept describing a vendored crate while the submodule was + // gone, the dependency line was gone, and ~3,700 lines of crate source had + // been copied into src/. This is the regression case the guard exists for. + const result = checkVendoredCrates({ + claims: new Map([ + [ + 'tinywallet', + { + sources: ['a comment in Cargo.toml'], + evidence: 'After cloning: `git submodule update --init vendor/tinywallet`.', + }, + ], + ]), + submodulePaths: new Set(), + gitlinkPaths: new Set(['vendor/tinybus']), + trackedVendorDirs: new Set(), + declaredDependencies: new Set(), + notVendored: {}, + inTree: {}, + }); + assert.equal(result.ok, false); + assert.equal(result.failures.length, 1); + assert.match(result.failures[0], /no `path = vendor\/tinywallet` entry in \.gitmodules/); + assert.match(result.failures[0], /no manifest declares a `path = "…\/vendor\/tinywallet"`/); +}); + +test('a submodule replaced by real files is reported as INLINED', () => { + // The other half of the same failure: someone deletes the gitlink and commits + // the crate source under the same path. `.gitmodules` may even survive. + const world = vendoredWorld(); + world.gitlinkPaths = new Set(); + world.trackedVendorDirs = new Set(['vendor/tinydocs']); + const result = checkVendoredCrates(world); + assert.equal(result.ok, false); + assert.match(result.failures[0], /INLINED/); +}); + +test('a submodule nobody depends on fails', () => { + const world = vendoredWorld(); + world.declaredDependencies = new Set(); + const result = checkVendoredCrates(world); + assert.equal(result.ok, false); + assert.match(result.failures[0], /no manifest declares/); +}); + +test('a waiver with a reason suppresses the failure', () => { + const world = vendoredWorld('tinyvoice'); + world.submodulePaths = new Set(); + world.gitlinkPaths = new Set(); + world.declaredDependencies = new Set(); + world.notVendored = { + tinyvoice: 'Module-only; no crate contract is shared.', + }; + const result = checkVendoredCrates(world); + assert.equal(result.ok, true); + assert.equal(result.failures.length, 0); + assert.match(result.waived[0], /Module-only/); +}); + +test('a waiver that stopped being true fails as stale', () => { + // An unratcheted exemption is how a fixed problem quietly comes back: the + // waiver would keep passing a crate that no longer needs it, and the next + // regression would land under its cover. + const world = vendoredWorld(); + world.notVendored = { tinydocs: 'stale reason' }; + const result = checkVendoredCrates(world); + assert.equal(result.ok, false); + assert.match(result.staleWaivers[0], /IS vendored now/); +}); + +test('an in-tree vendor waiver still requires tracked files and a dependency', () => { + const world = vendoredWorld('motosan-ai-oauth'); + world.submodulePaths = new Set(); + world.gitlinkPaths = new Set(); + world.inTree = { + 'motosan-ai-oauth': 'No upstream repo to point a submodule at.', + }; + + world.trackedVendorDirs = new Set(); + assert.match(checkVendoredCrates(world).failures[0], /no tracked files/); + + world.trackedVendorDirs = new Set(['vendor/motosan-ai-oauth']); + assert.equal(checkVendoredCrates(world).ok, true); + + world.declaredDependencies = new Set(); + assert.match(checkVendoredCrates(world).failures[0], /no manifest declares/); +}); + +test('an in-tree waiver for a path that became a real submodule is stale', () => { + const world = vendoredWorld('motosan-ai-oauth'); + world.inTree = { 'motosan-ai-oauth': 'reason' }; + assert.match(checkVendoredCrates(world).staleWaivers[0], /real submodule now/); +}); + +// ── the checker end to end ───────────────────────────────────────────────── + +/** + * Build a throwaway git repo with the file layout the checker reads, and a + * real mode-160000 index entry so the gitlink assertion is exercised for real + * rather than against a hand-built Set. + */ +function fixtureRepo({ inlined }) { + const root = mkdtempSync(join(tmpdir(), 'vendored-crates-')); + execFileSync('git', ['init', '-q', root]); + mkdirSync(join(root, 'app/src-tauri'), { recursive: true }); + mkdirSync(join(root, 'src/openhuman/modules'), { recursive: true }); + mkdirSync(join(root, 'vendor/tinydocs'), { recursive: true }); + + writeFileSync( + join(root, 'Cargo.toml'), + '# After cloning: `git submodule update --init vendor/tinydocs`.\n' + + (inlined ? '' : 'tinydocs = { path = "vendor/tinydocs", default-features = false }\n'), + ); + writeFileSync(join(root, 'app/src-tauri/Cargo.toml'), '[package]\nname = "shell"\n'); + writeFileSync( + join(root, 'src/openhuman/modules/registry.rs'), + 'const TINYDOCS: ModuleRecord = ModuleRecord {\n id: "tinydocs",\n};\n', + ); + // `3ee5a3cad` removed ONE submodule; the rest of vendor/ stayed. The fixture + // keeps an unrelated submodule so the inlined case is a real verdict rather + // than the checker's "no submodules at all" vacuity bail-out. + writeFileSync( + join(root, '.gitmodules'), + '[submodule "vendor/tinybus"]\n\tpath = vendor/tinybus\n\turl = https://x\n' + + (inlined + ? '' + : '[submodule "vendor/tinydocs"]\n\tpath = vendor/tinydocs\n\turl = https://x\n'), + ); + + if (inlined) { + // The crate source, copied in under the same path. + writeFileSync(join(root, 'vendor/tinydocs/lib.rs'), '// inlined\n'); + } + execFileSync('git', ['add', '-A'], { cwd: root }); + execFileSync('git', ['-c', 'user.email=t@t', '-c', 'user.name=t', 'commit', '-q', '-m', 'init'], { + cwd: root, + }); + const sha = execFileSync('git', ['rev-parse', 'HEAD'], { + cwd: root, + encoding: 'utf8', + }).trim(); + const gitlink = path => + execFileSync('git', ['update-index', '--add', '--cacheinfo', `160000,${sha},${path}`], { + cwd: root, + }); + gitlink('vendor/tinybus'); + if (!inlined) gitlink('vendor/tinydocs'); + return root; +} + +test('the checker passes on a repo where the crate is a real submodule', () => { + const root = fixtureRepo({ inlined: false }); + try { + const run = spawnSync(process.execPath, [CHECKER, '--repo', root], { + encoding: 'utf8', + }); + assert.equal(run.status, 0, run.stdout + run.stderr); + assert.match(run.stdout, /Vendored crates verified: tinydocs/); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test('the checker exits 1 on a repo where the crate has been inlined', () => { + const root = fixtureRepo({ inlined: true }); + try { + const run = spawnSync(process.execPath, [CHECKER, '--repo', root], { + encoding: 'utf8', + }); + // exit 2 would mean the guard failed to run; this must be a real verdict. + assert.equal(run.status, 1, run.stdout + run.stderr); + assert.match(run.stdout, /tinydocs/); + assert.match(run.stdout, /INLINED/); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test('the checker refuses to pass vacuously when it parses nothing', () => { + const root = mkdtempSync(join(tmpdir(), 'vendored-crates-empty-')); + try { + execFileSync('git', ['init', '-q', root]); + mkdirSync(join(root, 'app/src-tauri'), { recursive: true }); + mkdirSync(join(root, 'src/openhuman/modules'), { recursive: true }); + writeFileSync(join(root, 'Cargo.toml'), '[package]\nname = "core"\n'); + writeFileSync(join(root, 'app/src-tauri/Cargo.toml'), '[package]\nname = "shell"\n'); + writeFileSync(join(root, 'src/openhuman/modules/registry.rs'), '// no modules\n'); + writeFileSync(join(root, '.gitmodules'), ''); + const run = spawnSync(process.execPath, [CHECKER, '--repo', root], { + encoding: 'utf8', + }); + assert.equal(run.status, 2, run.stdout + run.stderr); + assert.match(run.stderr, /refusing to pass vacuously/); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + +test('the real repo satisfies its own guard', () => { + const run = spawnSync(process.execPath, [CHECKER], { encoding: 'utf8' }); + assert.equal(run.status, 0, run.stdout + run.stderr); +}); diff --git a/scripts/ci/check-vendored-crates.mjs b/scripts/ci/check-vendored-crates.mjs new file mode 100644 index 0000000000..98371a570c --- /dev/null +++ b/scripts/ci/check-vendored-crates.mjs @@ -0,0 +1,161 @@ +#!/usr/bin/env node +// Fails when a crate the build describes as vendored has been inlined into +// this tree instead. +// +// See scripts/lib/vendored-crates.mjs for the three assertions, the three +// sources claims are drawn from, and why they are shaped that way (#5559). +// Short version: `3ee5a3cad` deleted the `vendor/tinywallet` submodule and +// inlined ~3,700 lines of crate source, leaving the manifest comments intact. +// Nothing failed. The fork then hid a SLIP-10 key-derivation bug from every +// other host until #5533. +// +// The gitlink assertion reads the git INDEX, not the filesystem, so this lane +// needs no `git submodule update --init` and no Rust toolchain. +// +// Usage: check-vendored-crates.mjs [--repo ] +import { readFileSync } from 'node:fs'; +import { execFileSync } from 'node:child_process'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { + checkVendoredCrates, + collectDeclaredDependencies, + collectDocumentedClaims, + collectModuleIds, + formatReport, + parseGitmodulesPaths, +} from '../lib/vendored-crates.mjs'; + +const DEFAULT_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '../..'); + +function usage() { + return 'Usage: check-vendored-crates.mjs [--repo ]'; +} + +const args = process.argv.slice(2); +if (args[0] === '--help' || args[0] === '-h') { + console.log(usage()); + process.exit(0); +} +let repoRoot = DEFAULT_ROOT; +if (args[0] === '--repo') { + if (!args[1]) { + console.error(usage()); + process.exit(2); + } + repoRoot = resolve(args[1]); +} else if (args.length > 0) { + console.error(usage()); + process.exit(2); +} + +const MANIFESTS = ['Cargo.toml', 'app/src-tauri/Cargo.toml']; +const REGISTRY = 'src/openhuman/modules/registry.rs'; +const GITMODULES = '.gitmodules'; + +function read(relPath, { optional = false } = {}) { + try { + return readFileSync(resolve(repoRoot, relPath), 'utf8'); + } catch (err) { + if (optional) return ''; + console.error(`Could not read ${relPath}: ${err.message}`); + process.exit(2); + } +} + +/** + * `vendor/` entries in the git index, split into gitlinks (submodules, mode + * 160000) and ordinary tracked files. Reading the index rather than the + * filesystem is what makes an uninitialised submodule indistinguishable from + * an initialised one — and an inlined crate distinguishable from both. + */ +function readVendorIndex() { + let out; + try { + out = execFileSync('git', ['ls-files', '--stage', '--', 'vendor'], { + cwd: repoRoot, + encoding: 'utf8', + maxBuffer: 64 * 1024 * 1024, + }); + } catch (err) { + console.error(`Could not read the git index: ${err.message}`); + process.exit(2); + } + const gitlinkPaths = new Set(); + const trackedVendorDirs = new Set(); + for (const line of out.split('\n')) { + if (!line.trim()) continue; + const match = line.match(/^(\d{6})\s+[0-9a-f]+\s+\d\t(.+)$/); + if (!match) continue; + const [, mode, path] = match; + if (mode === '160000') { + gitlinkPaths.add(path); + } else { + const segments = path.split('/'); + if (segments.length >= 2) trackedVendorDirs.add(`${segments[0]}/${segments[1]}`); + } + } + return { gitlinkPaths, trackedVendorDirs }; +} + +// ── collect claims ───────────────────────────────────────────────────────── + +/** @type {Map} */ +const claims = new Map(); +function claim(name, source, evidence) { + const existing = claims.get(name); + if (existing) { + if (!existing.sources.includes(source)) existing.sources.push(source); + return; + } + claims.set(name, { sources: [source], evidence }); +} + +const declaredDependencies = new Set(); +for (const manifest of MANIFESTS) { + const toml = read(manifest); + for (const [name, evidence] of collectDocumentedClaims(toml)) { + claim(name, `a comment in ${manifest}`, evidence); + } + for (const name of collectDeclaredDependencies(toml)) { + declaredDependencies.add(name); + claim(name, `a path dependency in ${manifest}`, `path = "…/vendor/${name}"`); + } +} + +for (const id of collectModuleIds(read(REGISTRY))) { + claim(id, `the module registry (${REGISTRY})`, `id: "${id}" in modules::registry::ALL`); +} + +// Guard the guard: a scanner that silently found nothing would turn this lane +// into a rubber stamp, which is worse than no lane at all. Treat empty input as +// a failure OF THE CHECK (exit 2), distinct from a real regression (exit 1). +if (claims.size === 0) { + console.error( + 'FAIL: found zero vendored-crate claims across ' + + `${MANIFESTS.join(', ')} and ${REGISTRY}.\n` + + 'Either those files changed shape or the scanner is broken — refusing to pass vacuously.', + ); + process.exit(2); +} + +const { gitlinkPaths, trackedVendorDirs } = readVendorIndex(); +if (gitlinkPaths.size === 0) { + console.error( + 'FAIL: the git index reports no submodules under vendor/ at all.\n' + + 'That is either a broken checkout or a broken reader — refusing to pass vacuously.', + ); + process.exit(2); +} + +const result = checkVendoredCrates({ + claims, + submodulePaths: parseGitmodulesPaths(read(GITMODULES)), + gitlinkPaths, + trackedVendorDirs, + declaredDependencies, +}); + +console.log(formatReport(result)); +process.exit(result.ok ? 0 : 1); diff --git a/scripts/lib/vendored-crates.mjs b/scripts/lib/vendored-crates.mjs new file mode 100644 index 0000000000..9fb14b0e0c --- /dev/null +++ b/scripts/lib/vendored-crates.mjs @@ -0,0 +1,332 @@ +// Detects a crate that the build *claims* to consume from `vendor/` but has +// actually been inlined into this tree. +// +// WHY THIS EXISTS (#5559). On 2026-08-12, `3ee5a3cad` ("refactor: run tiny +// domains as TinyBus modules") removed the `vendor/tinywallet` submodule and +// inlined ~3,700 lines of crate source into `src/openhuman/web3/wallet/`, +// rewriting every `crate::` path. Nothing failed. The manifest comments and +// AGENTS.md kept describing a vendored crate, so code and docs contradicted +// each other and the contradiction was invisible to CI. +// +// An inlined copy of a shared crate is a SILENT FORK. Four fixes accrued in +// OpenHuman's copy that no other host ever saw, one of them a SLIP-10 +// key-derivation bug: a path segment already carrying the hardening bit was +// OR-ed with it again, so `m/44'/501'/2147483648'` and `m/44'/501'/0'` derived +// the same key. Restored in #5533 / tinywallet#16 / tinywallet#17. +// +// The same failure was live again on `tinydocs` when this guard was written: +// four manifest comment blocks described it as vendored — including +// `git submodule update --init vendor/tinydocs`, an instruction that could not +// work — while there was no `vendor/tinydocs`, no `.gitmodules` entry and no +// dependency declaration. Its spec types sat inlined under +// `src/openhuman/tools/impl/document/`. +// +// THREE ASSERTIONS, applied to every claimed crate: +// +// 1. `.gitmodules` declares `vendor/` as a submodule. +// 2. The git index records `vendor/` as a gitlink (mode 160000). This +// is the one that catches an inlining: replacing a submodule with real +// files changes the index entry, and it is checked through the index +// rather than the filesystem so the guard works on a runner that never +// ran `git submodule update --init`. +// 3. Some manifest declares a `path = "…/vendor/"` dependency. A +// submodule nobody depends on is a checkout nobody compiles — which is +// the state `3ee5a3cad` would have left behind had it deleted only the +// dependency line. +// +// WHERE CLAIMS COME FROM — three independent sources, deliberately, because +// each covers a way the previous one can be edited away: +// +// - Manifest comments that describe a crate as vendored. This is what +// `3ee5a3cad` left behind untouched, so it is the source that would have +// caught it on the PR that introduced it. +// - `path = "…/vendor/"` dependency declarations. Catches the reverse +// shape: a real dependency on a vendor directory that is not a submodule. +// - The compiled-in module registry (`src/openhuman/modules/registry.rs`). +// A module-backed capability shares its wire contract with the host as a +// crate; a module id with no vendored crate behind it means the host has +// its own copy of the contract the module validates against, which is the +// drift the extraction existed to prevent. +// +// Like `feature-forwarding.mjs`, this is a deliberately narrow scanner rather +// than a general TOML parser: the repo has no TOML dependency for Node and the +// shapes involved are well known. + +/** + * Crates that are named as vendored (or module-backed) but deliberately have + * NO vendored source dependency, mapped to why. + * + * Adding an entry is a deliberate decision, not a way to silence the guard. + * The reason string is the only thing that keeps "excluded on purpose" + * distinguishable from "forgotten" — which is exactly the ambiguity that let + * `3ee5a3cad`'s inlining sit in the tree with its documentation intact. + * + * A waiver here is checked for staleness: if the crate turns out to be + * properly vendored after all, the entry fails as stale rather than sitting + * around asserting something that stopped being true. + */ +export const INTENTIONALLY_NOT_VENDORED = { + // 'some-crate': 'Reason this crate has no vendored source dependency.', + tinyjuice: + 'Module-only: the host never links the crate. TinyJuice left the dependency graph in 4e4c8ffc1 ("load TinyJuice outside dependency graph"), which in the SAME commit re-declared the wire types host-side in src/openhuman/inference/tokenjuice/types.rs, saying so in that file\'s own doc comment. That is a recorded decision, not the silent contradiction this guard exists to catch — but it does mean two declarations of one contract, so if that pair ever drifts the fix is to share the crate for its types (the tinydocs shape) rather than to widen this waiver.', + tinyvoice: + 'Module-only: the host never links the crate. src/openhuman/modules/voice.rs speaks to ai.tinyhumans.tinyvoice.Voice over the bus with local serde types and base64 framing; no crate-level contract is shared in either direction, so there is nothing for a vendored source dependency to provide.', +}; + +/** + * Vendor directories whose sources are checked into THIS repo rather than + * pulled in as a submodule, mapped to why. + * + * These skip assertions 1 and 2 (no `.gitmodules` entry, no gitlink) but still + * have to be tracked and depended on. The distinction matters: an in-tree + * vendor directory is a fork by design and reviewed as this repo's own code, + * whereas a submodule that quietly became one is the failure this guard is + * here to catch. Also staleness-checked — an entry that has since become a + * real submodule fails rather than silently exempting it. + */ +export const VENDORED_IN_TREE = { + 'motosan-ai-oauth': + "Small OAuth helper carried as in-tree source (vendor/motosan-ai-oauth) rather than a submodule: it has no upstream repo of its own in the tinyhumansai org, so there is nothing to point a submodule at. Reviewed as this repo's own code.", +}; + +/** + * Strip TOML `#` comments while respecting quoted strings, returning the code + * and the comment text separately. + * + * Both halves are needed: claims come out of the comments, dependency + * declarations out of the code. Splitting once here keeps a `#` inside a + * quoted value from being read as a comment in either direction. + */ +export function splitTomlComments(text) { + const code = []; + const comments = []; + for (const line of text.split(/\r?\n/)) { + let quote = null; + let cut = -1; + for (let i = 0; i < line.length; i++) { + const ch = line[i]; + if (quote) { + if (ch === quote && line[i - 1] !== '\\') quote = null; + } else if (ch === '"' || ch === "'") { + quote = ch; + } else if (ch === '#') { + cut = i; + break; + } + } + if (cut === -1) { + code.push(line); + } else { + code.push(line.slice(0, cut)); + comments.push(line.slice(cut + 1)); + } + } + return { code: code.join('\n'), comments }; +} + +/** Trailing punctuation a path picks up from prose: "vendor/tinydocs`." */ +const NAME_RE = /vendor\/([A-Za-z0-9][A-Za-z0-9_.-]*)/g; + +/** + * A comment line only counts as a vendoring CLAIM when it says how the crate + * is consumed — "submodule", or "vendored as". A passing mention of a path + * (`see vendor/tauri-cef/.../tauri.rs`) is documentation of where something + * lives, not an assertion that this build consumes it from there, and treating + * it as one would make the guard fire on prose. + */ +const CLAIM_RE = /\bsubmodules?\b|\bvendored as\b/i; + +/** + * Vendoring claims made in a manifest's comments. + * + * Paths are resolved against the REPO ROOT, not the manifest's directory: + * `vendor/` is a repo-root convention here, and the shell manifest's comments + * say so explicitly ("repo-root vendor/tinyagents"). + * + * @returns {Map} crate name → the comment line that claimed it + */ +export function collectDocumentedClaims(tomlText) { + const { comments } = splitTomlComments(tomlText); + const claims = new Map(); + for (const comment of comments) { + if (!CLAIM_RE.test(comment)) continue; + for (const match of comment.matchAll(NAME_RE)) { + if (!claims.has(match[1])) claims.set(match[1], comment.trim()); + } + } + return claims; +} + +/** + * Crates declared as `path = "…/vendor/[/subpath]"` dependencies. + * + * Sub-crate paths (`vendor/tinymemory/core`, `vendor/tinyplace/sdk/rust`) + * collapse onto their vendor root, which is the unit a submodule tracks. + * + * `[patch.crates-io]` entries count. A patch redirects the whole graph onto + * the vendored source, so a patch pointing at a directory that is not a + * submodule is the same silent fork by another route. + * + * @returns {Set} + */ +export function collectDeclaredDependencies(tomlText) { + const { code } = splitTomlComments(tomlText); + const declared = new Set(); + for (const match of code.matchAll(/path\s*=\s*"([^"]*vendor\/[^"]*)"/g)) { + const parts = match[1].split('vendor/'); + const tail = parts[parts.length - 1]; + const name = tail.split('/')[0]; + if (name) declared.add(name); + } + return declared; +} + +/** + * Module ids from the compiled-in registry (`modules::registry::ALL`). + * + * Read out of the source rather than a build artifact so the check costs + * nothing and runs on a runner with no Rust toolchain. + * + * @returns {Set} + */ +export function collectModuleIds(registrySource) { + const ids = new Set(); + for (const match of registrySource.matchAll(/^\s*id:\s*"([^"]+)"\s*,/gm)) { + ids.add(match[1]); + } + return ids; +} + +/** Submodule paths declared in `.gitmodules`. */ +export function parseGitmodulesPaths(gitmodulesText) { + const paths = new Set(); + for (const match of gitmodulesText.matchAll(/^\s*path\s*=\s*(.+)$/gm)) { + paths.add(match[1].trim()); + } + return paths; +} + +/** + * Run the three assertions over every claimed crate. + * + * @param {object} input + * @param {Map} input.claims + * @param {Set} input.submodulePaths paths from `.gitmodules` + * @param {Set} input.gitlinkPaths index entries with mode 160000 + * @param {Set} input.trackedVendorDirs vendor dirs with tracked files + * @param {Set} input.declaredDependencies + * @param {object} [input.notVendored] allow-list (name → reason) + * @param {object} [input.inTree] allow-list (name → reason) + */ +export function checkVendoredCrates({ + claims, + submodulePaths, + gitlinkPaths, + trackedVendorDirs, + declaredDependencies, + notVendored = INTENTIONALLY_NOT_VENDORED, + inTree = VENDORED_IN_TREE, +}) { + const failures = []; + const waived = []; + const staleWaivers = []; + const ok = []; + + for (const [name, claim] of claims) { + const vendorPath = `vendor/${name}`; + const isSubmodule = submodulePaths.has(vendorPath); + const isGitlink = gitlinkPaths.has(vendorPath); + const isTracked = trackedVendorDirs.has(vendorPath); + const isDeclared = declaredDependencies.has(name); + const where = claim.sources.join(', '); + + if (Object.hasOwn(notVendored, name)) { + if (isSubmodule || isDeclared) { + staleWaivers.push( + `${name}: listed in INTENTIONALLY_NOT_VENDORED, but it IS vendored now ` + + `(${isSubmodule ? '.gitmodules entry' : 'dependency declared'}). Remove the waiver.`, + ); + } else { + waived.push(`${name}: not vendored — ${notVendored[name]}`); + } + continue; + } + + if (Object.hasOwn(inTree, name)) { + if (isSubmodule) { + staleWaivers.push( + `${name}: listed in VENDORED_IN_TREE, but ${vendorPath} is a real submodule now. ` + + 'Remove the waiver so the normal assertions apply.', + ); + } else if (!isTracked) { + failures.push( + `${name}: declared in VENDORED_IN_TREE but ${vendorPath} has no tracked files. ` + + 'An in-tree vendor directory must be committed to this repo.', + ); + } else if (!isDeclared) { + failures.push( + `${name}: ${vendorPath} is committed in-tree but no manifest declares a ` + + `path dependency on it. Vendored source nothing depends on is dead weight.`, + ); + } else { + waived.push(`${name}: vendored in-tree — ${inTree[name]}`); + } + continue; + } + + const missing = []; + if (!isSubmodule) missing.push(`no \`path = ${vendorPath}\` entry in .gitmodules`); + if (!isGitlink) + missing.push( + `${vendorPath} is not a submodule in the git index` + + (isTracked ? ' (it holds tracked files — the crate has been INLINED)' : ' (path absent)'), + ); + if (!isDeclared) missing.push(`no manifest declares a \`path = "…/${vendorPath}"\` dependency`); + + if (missing.length === 0) { + ok.push(name); + continue; + } + failures.push( + `${name}: claimed as vendored by ${where}, but ${missing.join('; ')}.\n` + + ` evidence: ${claim.evidence}`, + ); + } + + return { + ok: failures.length === 0 && staleWaivers.length === 0, + checked: ok, + failures, + waived, + staleWaivers, + }; +} + +/** Human-readable report for the CLI. */ +export function formatReport(result) { + const lines = []; + if (result.checked.length) { + lines.push(`Vendored crates verified: ${result.checked.sort().join(', ')}`); + } + for (const note of result.waived.sort()) lines.push(`WAIVED ${note}`); + for (const stale of result.staleWaivers.sort()) lines.push(`STALE ${stale}`); + for (const failure of result.failures.sort()) lines.push(`FAIL ${failure}`); + if (result.ok) { + lines.push( + '', + 'OK: every crate documented or registered as vendored is a real submodule with a path dependency.', + ); + } else { + lines.push( + '', + 'A crate the build describes as vendored is not actually consumed from vendor/.', + 'That is how `3ee5a3cad` inlined tinywallet — a silent fork that hid a SLIP-10', + 'key-derivation bug from every other host (#5533).', + '', + 'Fix by restoring the submodule and the path dependency, or — if the crate', + 'genuinely must not be vendored — add it to INTENTIONALLY_NOT_VENDORED (or', + 'VENDORED_IN_TREE) in scripts/lib/vendored-crates.mjs WITH A REASON.', + ); + } + return lines.join('\n'); +} diff --git a/src/openhuman/modules/documents.rs b/src/openhuman/modules/documents.rs index 62d3f2053e..fede77ec0b 100644 --- a/src/openhuman/modules/documents.rs +++ b/src/openhuman/modules/documents.rs @@ -24,13 +24,11 @@ //! deadline underneath would make the effective limit the smaller of two numbers //! nobody picked together. -use crate::openhuman::tools::implementations::document::format::spec::{ - DocumentSpec, WirePresentationSpec, -}; use base64::engine::general_purpose::STANDARD as BASE64; use base64::Engine as _; use serde::Deserialize; use tinybus::stream::StreamRef; +use tinydocs::spec::{DocumentSpec, WirePresentationSpec}; use super::{host, ops, registry}; use crate::openhuman::config::Config; diff --git a/src/openhuman/modules/documents_tests.rs b/src/openhuman/modules/documents_tests.rs index 2fb2649857..7e97c5eeb6 100644 --- a/src/openhuman/modules/documents_tests.rs +++ b/src/openhuman/modules/documents_tests.rs @@ -8,9 +8,7 @@ use super::{classify, sha256_hex, DocumentCallError}; use crate::openhuman::config::Config; -use crate::openhuman::tools::implementations::document::format::spec::{ - DocumentSpec, WirePresentationSpec, -}; +use tinydocs::spec::{DocumentSpec, WirePresentationSpec}; /// A config with modules enabled but nothing fetchable. fn offline_config() -> Config { diff --git a/src/openhuman/tools/impl/document/format/error/mod.rs b/src/openhuman/tools/impl/document/format/error/mod.rs deleted file mode 100644 index e46245b59d..0000000000 --- a/src/openhuman/tools/impl/document/format/error/mod.rs +++ /dev/null @@ -1,120 +0,0 @@ -//! Crate-wide error and result types. -//! -//! Every fallible public function in this crate returns [`Result`], and every -//! failure mode is a distinct [`Error`] variant. Add a variant rather than -//! encoding new context into an existing message: callers match on variants, -//! and message text is not a stable API. -//! -//! The variants are deliberately *host-agnostic*. A host that surfaces these -//! to an LLM (the reason [`Error::InvalidInput`] carries a structured -//! `field` / `reason` pair rather than a formatted sentence) maps them onto -//! its own tool-error shape; a host writing to disk maps them onto its own. -//! Nothing here knows about artifacts, timeouts, or async runtimes — those are -//! the host's concerns, because only the host knows its own deadline policy. - -/// Errors returned by this crate. -#[derive(Debug, thiserror::Error, PartialEq, Eq)] -#[non_exhaustive] -pub enum Error { - /// A document spec failed validation before any synthesis was attempted. - /// - /// `field` names the offending path in the spec using the same dotted / - /// indexed notation the JSON input uses (`sections[2].bullets[0]`), so an - /// LLM that produced the spec can self-correct without re-reading the - /// whole schema. `reason` states the violated constraint. - #[error("invalid input for field '{field}': {reason}")] - InvalidInput { - /// Path of the offending field within the spec. - field: String, - /// The constraint that was violated. - reason: String, - }, - - /// The underlying document library failed to synthesise the output. - /// - /// `detail` is the library's own error rendered as text and truncated to a - /// bounded length, so the variant never carries an unbounded payload back - /// to a caller that forwards it to a model. - #[error("document generation failed: {detail}")] - GenerationFailed { - /// Truncated underlying library error. - detail: String, - }, - - /// The underlying library failed to extract text from an input document. - /// - /// Distinct from [`Error::GenerationFailed`] because the two have opposite - /// causes and opposite remedies: generation fails on *our* output path and - /// usually means a bug or an exhausted resource, whereas extraction fails on - /// *someone else's* input and usually means the document is damaged, - /// encrypted, or carries no extractable text layer at all. A caller that - /// retries one should not retry the other. - /// - /// `detail` is truncated on the same bound as `GenerationFailed`. - #[error("text extraction failed: {detail}")] - ExtractionFailed { - /// Truncated underlying library error. - detail: String, - }, -} - -impl Error { - /// Maximum length, in Unicode scalar values, of a [`Error::GenerationFailed`] - /// detail string. - pub const MAX_DETAIL_CHARS: usize = 500; - - /// Suffix appended when a detail string is truncated. - const TRUNCATION_SUFFIX: &'static str = " […truncated]"; - - /// Build a [`Error::GenerationFailed`] with `raw` truncated (UTF-8-safe) to - /// [`Error::MAX_DETAIL_CHARS`]. - /// - /// Truncation counts characters, not bytes, so a multi-byte error message - /// can never be cut mid-codepoint. - #[must_use] - pub fn generation_failed(raw: &str) -> Self { - Self::GenerationFailed { - detail: Self::truncate_detail(raw), - } - } - - /// Truncate `raw` to [`Error::MAX_DETAIL_CHARS`] characters, appending the - /// standard truncation suffix when anything was dropped. - #[must_use] - pub fn truncate_detail(raw: &str) -> String { - if raw.chars().count() <= Self::MAX_DETAIL_CHARS { - return raw.to_string(); - } - let keep = Self::MAX_DETAIL_CHARS.saturating_sub(Self::TRUNCATION_SUFFIX.chars().count()); - let mut out: String = raw.chars().take(keep).collect(); - out.push_str(Self::TRUNCATION_SUFFIX); - out - } - - /// Build an [`Error::ExtractionFailed`] with `raw` truncated (UTF-8-safe) to - /// [`Error::MAX_DETAIL_CHARS`]. - #[must_use] - pub fn extraction_failed(raw: &str) -> Self { - Self::ExtractionFailed { - detail: Self::truncate_detail(raw), - } - } - - /// Build an [`Error::InvalidInput`] for `field` violating `reason`. - #[must_use] - pub fn invalid_input(field: impl Into, reason: impl Into) -> Self { - Self::InvalidInput { - field: field.into(), - reason: reason.into(), - } - } -} - -/// The crate's standard result type. -/// -/// Use this alias in public signatures instead of spelling out -/// `std::result::Result`. -pub type Result = std::result::Result; - -#[cfg(test)] -mod test; diff --git a/src/openhuman/tools/impl/document/format/error/test.rs b/src/openhuman/tools/impl/document/format/error/test.rs deleted file mode 100644 index 7bc5815fc1..0000000000 --- a/src/openhuman/tools/impl/document/format/error/test.rs +++ /dev/null @@ -1,54 +0,0 @@ -//! Unit tests for the crate-wide error type. - -#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] - -use super::Error; - -#[test] -fn short_details_are_left_intact() { - let err = Error::generation_failed("boom"); - assert_eq!( - err, - Error::GenerationFailed { - detail: "boom".to_string() - } - ); -} - -#[test] -fn long_details_are_truncated_with_a_suffix() { - let raw = "x".repeat(Error::MAX_DETAIL_CHARS * 2); - let Error::GenerationFailed { detail } = Error::generation_failed(&raw) else { - panic!("expected GenerationFailed"); - }; - assert_eq!(detail.chars().count(), Error::MAX_DETAIL_CHARS); - assert!(detail.ends_with("[…truncated]")); -} - -#[test] -fn truncation_never_splits_a_multi_byte_character() { - // Every character is 4 bytes, so a byte-based truncation would panic or - // produce invalid UTF-8. Counting characters keeps the boundary valid. - let raw = "🦀".repeat(Error::MAX_DETAIL_CHARS * 2); - let detail = Error::truncate_detail(&raw); - assert_eq!(detail.chars().count(), Error::MAX_DETAIL_CHARS); - assert!(detail.starts_with('🦀')); -} - -#[test] -fn detail_at_exactly_the_cap_is_not_truncated() { - let raw = "y".repeat(Error::MAX_DETAIL_CHARS); - assert_eq!(Error::truncate_detail(&raw), raw); -} - -#[test] -fn invalid_input_carries_the_field_path_verbatim() { - let err = Error::invalid_input("sections[2].bullets[0]", "must be ≤ 10 chars"); - assert_eq!( - err, - Error::InvalidInput { - field: "sections[2].bullets[0]".to_string(), - reason: "must be ≤ 10 chars".to_string(), - } - ); -} diff --git a/src/openhuman/tools/impl/document/format/mod.rs b/src/openhuman/tools/impl/document/format/mod.rs deleted file mode 100644 index 81c45df4d8..0000000000 --- a/src/openhuman/tools/impl/document/format/mod.rs +++ /dev/null @@ -1,29 +0,0 @@ -//! Agent-friendly document synthesis and text extraction in Rust. -//! -//! Typed, validated document contracts shared with the document bus module. -//! They are built for hosts that let a language model produce documents: -//! the spec types are the JSON tool schema, validation rejects a malformed -//! spec with a structured [`Error::InvalidInput`] naming the exact field so -//! the model can self-correct, and synthesis returns a plain byte buffer. -//! -//! # What this module deliberately does not do -//! -//! No filesystem access, no subprocesses, no async runtime, no deadline -//! handling. Synthesis runs in the document bus module; this host module owns -//! the wire contract and validation only. -//! -//! # Layout -//! -//! - [`error`](self::Error) — the crate-wide [`Error`] and [`Result`]. -//! - [`spec`] — the typed document specs and their validation. Compiled in -//! every build, including `--no-default-features`, so a host whose synthesis -//! happens elsewhere still shares one definition of the wire contract. -//! -//! Writer and extractor implementations are intentionally absent: the host -//! sends these contract values over TinyBus. - -mod error; - -pub mod spec; - -pub use error::{Error, Result}; diff --git a/src/openhuman/tools/impl/document/format/spec/document/mod.rs b/src/openhuman/tools/impl/document/format/spec/document/mod.rs deleted file mode 100644 index d96e78f429..0000000000 --- a/src/openhuman/tools/impl/document/format/spec/document/mod.rs +++ /dev/null @@ -1,261 +0,0 @@ -//! The `.docx` document spec: the typed description a caller hands to -//! `docx::generate`, plus the size limits every spec is validated against. -//! -//! The spec is the crate's wire contract. It derives `Serialize` / -//! `Deserialize` with `deny_unknown_fields` because the usual caller is an -//! LLM tool boundary: the same struct that drives synthesis is the one whose -//! JSON schema the model is shown, and a typo'd field name should be a loud -//! rejection rather than a silently ignored key. -//! -//! Limits are public consts rather than private constants so a host can quote -//! the exact number in its own tool description and stay in lockstep with what -//! validation actually enforces. -//! -//! Nothing in this module depends on the `docx` feature or on `docx-rs`: it is -//! `serde` plus the crate error type. A host that only needs to *describe* and -//! *validate* a document — because synthesis happens elsewhere, in another -//! process or behind a message bus — can therefore depend on this crate with -//! `default-features = false` and still share one definition of the contract. - -use serde::{Deserialize, Serialize}; - -use crate::openhuman::tools::implementations::document::format::{Error, Result}; - -/// Maximum number of sections a single document may contain. -/// -/// Bounds generation time and output size; a caller with more material is -/// expected to split it across multiple documents. -pub const MAX_SECTIONS: usize = 128; - -/// Maximum length, in Unicode scalar values, of a short text field — the -/// document title, the author byline, or a section heading. -pub const MAX_TEXT_CHARS: usize = 2_000; - -/// Maximum length, in Unicode scalar values, of a single body paragraph or -/// bullet item. -/// -/// More generous than [`MAX_TEXT_CHARS`]: prose paragraphs legitimately run -/// far longer than a heading. -pub const MAX_PARAGRAPH_CHARS: usize = 20_000; - -/// Maximum number of body paragraphs in a single section. -pub const MAX_PARAGRAPHS_PER_SECTION: usize = 200; - -/// Maximum number of bullet-list items in a single section. -pub const MAX_BULLETS_PER_SECTION: usize = 200; - -/// Aggregate cap on all renderable text across the whole document — the -/// title, the author byline, and every section's heading, paragraphs, and -/// bullets — in Unicode scalar values. -/// -/// The per-field and per-section limits above bound each individual piece, but -/// not their product — `MAX_SECTIONS × MAX_PARAGRAPHS_PER_SECTION × -/// MAX_PARAGRAPH_CHARS` alone is over 500M characters, so a spec satisfying -/// every other limit could still build a multi-hundred-megabyte document in -/// memory. This total keeps the worst case bounded to a few megabytes of text -/// while staying generous for any real document. -pub const MAX_TOTAL_CHARS: usize = 2_000_000; - -/// One section of the document, rendered in spec order. -/// -/// A section is an optional heading followed by any number of body paragraphs -/// and/or a bullet list. At least one of the three must carry renderable text — -/// a wholly blank section is rejected by [`DocumentSpec::validate`] rather than -/// silently rendering nothing. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct DocumentSection { - /// Section heading, rendered as a bold heading paragraph. Optional: a - /// section may be pure body text under the document title. - #[serde(default)] - pub heading: Option, - /// Body paragraphs, each rendered as its own paragraph, in order. - /// Blank and whitespace-only entries are dropped during synthesis. - #[serde(default)] - pub paragraphs: Vec, - /// Bullet-list items, rendered as a single-level bulleted list after the - /// section's body paragraphs. Blank and whitespace-only entries are - /// dropped during synthesis. - #[serde(default)] - pub bullets: Vec, -} - -impl DocumentSection { - /// Returns `true` when the section carries no renderable content at all — - /// the heading is absent or blank, and every paragraph and bullet is blank. - /// - /// Synthesis trims and drops blank entries, so a section holding only - /// `[" "]` would render as nothing despite carrying entries. Validation - /// uses this to reject that case up front. - #[must_use] - pub fn is_blank(&self) -> bool { - let has_heading = self - .heading - .as_deref() - .is_some_and(|h| !h.trim().is_empty()); - let has_paragraph = self.paragraphs.iter().any(|p| !p.trim().is_empty()); - let has_bullet = self.bullets.iter().any(|b| !b.trim().is_empty()); - !(has_heading || has_paragraph || has_bullet) - } -} - -/// A complete `.docx` document spec. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct DocumentSpec { - /// Document title, rendered as the leading title paragraph. Required and - /// non-blank. - pub title: String, - /// Optional author byline, rendered as an italic line beneath the title. - #[serde(default)] - pub author: Option, - /// Sections, in display order. Must contain at least one entry. - #[serde(default)] - pub sections: Vec, -} - -impl DocumentSpec { - /// Total renderable text across the whole spec, in Unicode scalar values. - /// - /// Sums with saturating arithmetic so an adversarial spec cannot overflow - /// the counter into a small value that passes the aggregate check. - #[must_use] - pub fn total_chars(&self) -> usize { - let mut total = self.title.chars().count(); - if let Some(author) = self.author.as_deref() { - total = total.saturating_add(author.chars().count()); - } - for section in &self.sections { - if let Some(heading) = section.heading.as_deref() { - total = total.saturating_add(heading.chars().count()); - } - for paragraph in §ion.paragraphs { - total = total.saturating_add(paragraph.chars().count()); - } - for bullet in §ion.bullets { - total = total.saturating_add(bullet.chars().count()); - } - } - total - } - - /// Check the spec against every documented size limit. - /// - /// Callers do not have to invoke this: `docx::generate` validates before it - /// synthesises anything. It is public so a host can reject a malformed - /// spec at its own boundary — an LLM tool call, say — and hand back the - /// structured [`Error::InvalidInput`] before paying for a blocking hop, a - /// process boundary, or a bus round trip. - /// - /// # Errors - /// - /// Returns [`Error::InvalidInput`] naming the first field that violates a - /// limit. Fields are checked in spec order (title, author, sections, then - /// each section's contents) so the reported field is stable for a given - /// spec. - pub fn validate(&self) -> Result<()> { - if self.title.trim().is_empty() { - return Err(Error::invalid_input("title", "must not be empty")); - } - if self.title.chars().count() > MAX_TEXT_CHARS { - return Err(Error::invalid_input( - "title", - format!("must be ≤ {MAX_TEXT_CHARS} chars"), - )); - } - // Running total across every renderable field — title, author, and all - // section contents — checked as each field is processed. A spec can pass - // every per-field limit yet blow the aggregate budget, and checking - // incrementally rejects it as soon as the budget is crossed without a - // second pass over the whole spec. - let over_budget = || { - Error::invalid_input( - "sections", - format!("total document text must be ≤ {MAX_TOTAL_CHARS} chars"), - ) - }; - let mut total = self.title.chars().count(); - if let Some(author) = self.author.as_deref() { - if author.chars().count() > MAX_TEXT_CHARS { - return Err(Error::invalid_input( - "author", - format!("must be ≤ {MAX_TEXT_CHARS} chars"), - )); - } - total = total.saturating_add(author.chars().count()); - } - if self.sections.is_empty() { - return Err(Error::invalid_input( - "sections", - "must contain at least one section", - )); - } - if self.sections.len() > MAX_SECTIONS { - return Err(Error::invalid_input( - "sections", - format!("must contain ≤ {MAX_SECTIONS} sections"), - )); - } - - for (i, section) in self.sections.iter().enumerate() { - if section.is_blank() { - return Err(Error::invalid_input( - format!("sections[{i}]"), - "must have at least one of heading / paragraphs / bullets", - )); - } - if let Some(heading) = section.heading.as_deref() { - if heading.chars().count() > MAX_TEXT_CHARS { - return Err(Error::invalid_input( - format!("sections[{i}].heading"), - format!("must be ≤ {MAX_TEXT_CHARS} chars"), - )); - } - total = total.saturating_add(heading.chars().count()); - if total > MAX_TOTAL_CHARS { - return Err(over_budget()); - } - } - if section.paragraphs.len() > MAX_PARAGRAPHS_PER_SECTION { - return Err(Error::invalid_input( - format!("sections[{i}].paragraphs"), - format!("must contain ≤ {MAX_PARAGRAPHS_PER_SECTION} paragraphs"), - )); - } - for (p, paragraph) in section.paragraphs.iter().enumerate() { - if paragraph.chars().count() > MAX_PARAGRAPH_CHARS { - return Err(Error::invalid_input( - format!("sections[{i}].paragraphs[{p}]"), - format!("must be ≤ {MAX_PARAGRAPH_CHARS} chars"), - )); - } - total = total.saturating_add(paragraph.chars().count()); - if total > MAX_TOTAL_CHARS { - return Err(over_budget()); - } - } - if section.bullets.len() > MAX_BULLETS_PER_SECTION { - return Err(Error::invalid_input( - format!("sections[{i}].bullets"), - format!("must contain ≤ {MAX_BULLETS_PER_SECTION} bullets"), - )); - } - for (b, bullet) in section.bullets.iter().enumerate() { - if bullet.chars().count() > MAX_PARAGRAPH_CHARS { - return Err(Error::invalid_input( - format!("sections[{i}].bullets[{b}]"), - format!("must be ≤ {MAX_PARAGRAPH_CHARS} chars"), - )); - } - total = total.saturating_add(bullet.chars().count()); - if total > MAX_TOTAL_CHARS { - return Err(over_budget()); - } - } - } - Ok(()) - } -} - -#[cfg(test)] -mod test; diff --git a/src/openhuman/tools/impl/document/format/spec/document/test.rs b/src/openhuman/tools/impl/document/format/spec/document/test.rs deleted file mode 100644 index 724302217e..0000000000 --- a/src/openhuman/tools/impl/document/format/spec/document/test.rs +++ /dev/null @@ -1,272 +0,0 @@ -//! Unit tests for the wire contracts: validation, the blank/aggregate rules, -//! and JSON round-tripping. -//! -//! These are deliberately separate from the format modules' tests. They must -//! pass in a build with every format feature off, because the spec is the half -//! of the crate a bus- or process-boundary host shares without the codec. - -#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] - -use super::{ - DocumentSection, DocumentSpec, MAX_BULLETS_PER_SECTION, MAX_PARAGRAPHS_PER_SECTION, - MAX_PARAGRAPH_CHARS, MAX_SECTIONS, MAX_TEXT_CHARS, MAX_TOTAL_CHARS, -}; -use crate::openhuman::tools::implementations::document::format::Error; - -/// One valid section carrying a heading, a paragraph, and a bullet. -fn section() -> DocumentSection { - DocumentSection { - heading: Some("Overview".to_string()), - paragraphs: vec!["A body paragraph.".to_string()], - bullets: vec!["A bullet".to_string()], - } -} - -/// A minimal valid spec; each test mutates one field to drive a single branch. -fn spec() -> DocumentSpec { - DocumentSpec { - title: "Charter".to_string(), - author: Some("Alice".to_string()), - sections: vec![section()], - } -} - -/// Assert `spec` is rejected with an `InvalidInput` naming `field`. -fn assert_rejects(spec: &DocumentSpec, field: &str) { - match spec.validate() { - Err(Error::InvalidInput { field: f, .. }) => { - assert_eq!(f, field, "unexpected rejected field"); - } - other => panic!("expected InvalidInput({field}), got {other:?}"), - } -} - -#[test] -fn accepts_a_well_formed_spec() { - assert!(spec().validate().is_ok()); -} - -#[test] -fn rejects_a_blank_title() { - let mut s = spec(); - s.title = " ".to_string(); - assert_rejects(&s, "title"); -} - -#[test] -fn rejects_an_over_long_title() { - let mut s = spec(); - s.title = "t".repeat(MAX_TEXT_CHARS + 1); - assert_rejects(&s, "title"); -} - -#[test] -fn rejects_an_over_long_author() { - let mut s = spec(); - s.author = Some("a".repeat(MAX_TEXT_CHARS + 1)); - assert_rejects(&s, "author"); -} - -#[test] -fn rejects_a_spec_with_no_sections() { - let mut s = spec(); - s.sections.clear(); - assert_rejects(&s, "sections"); -} - -#[test] -fn rejects_too_many_sections() { - let mut s = spec(); - s.sections = vec![section(); MAX_SECTIONS + 1]; - assert_rejects(&s, "sections"); -} - -#[test] -fn rejects_a_wholly_blank_section() { - // Every entry is present but whitespace-only, so synthesis would drop all - // of them and render nothing. Validation catches it instead. - let mut s = spec(); - s.sections = vec![DocumentSection { - heading: Some(" ".to_string()), - paragraphs: vec!["\t".to_string()], - bullets: vec![String::new()], - }]; - assert_rejects(&s, "sections[0]"); -} - -#[test] -fn rejects_an_over_long_heading_naming_its_index() { - let mut s = spec(); - s.sections.push(DocumentSection { - heading: Some("h".repeat(MAX_TEXT_CHARS + 1)), - ..section() - }); - assert_rejects(&s, "sections[1].heading"); -} - -#[test] -fn rejects_too_many_paragraphs() { - let mut s = spec(); - s.sections[0].paragraphs = vec!["p".to_string(); MAX_PARAGRAPHS_PER_SECTION + 1]; - assert_rejects(&s, "sections[0].paragraphs"); -} - -#[test] -fn rejects_an_over_long_paragraph_naming_its_index() { - let mut s = spec(); - s.sections[0].paragraphs = vec!["ok".to_string(), "p".repeat(MAX_PARAGRAPH_CHARS + 1)]; - assert_rejects(&s, "sections[0].paragraphs[1]"); -} - -#[test] -fn rejects_too_many_bullets() { - let mut s = spec(); - s.sections[0].bullets = vec!["b".to_string(); MAX_BULLETS_PER_SECTION + 1]; - assert_rejects(&s, "sections[0].bullets"); -} - -#[test] -fn rejects_an_over_long_bullet_naming_its_index() { - let mut s = spec(); - s.sections[0].bullets = vec!["ok".to_string(), "b".repeat(MAX_PARAGRAPH_CHARS + 1)]; - assert_rejects(&s, "sections[0].bullets[1]"); -} - -#[test] -fn rejects_a_spec_over_the_aggregate_character_budget() { - // Each individual field is within its own limit; only the sum is not. One - // section with just enough max-length paragraphs to cross MAX_TOTAL_CHARS - // reproduces that without allocating hundreds of megabytes: repeating a - // whole section MAX_SECTIONS times (the original fixture) built ~512 MB - // of paragraph text before validation ever ran. - let paragraph_count = MAX_TOTAL_CHARS / MAX_PARAGRAPH_CHARS + 1; - assert!(paragraph_count <= MAX_PARAGRAPHS_PER_SECTION); - let paragraph = "x".repeat(MAX_PARAGRAPH_CHARS); - let big = DocumentSection { - heading: Some("Heading".to_string()), - paragraphs: vec![paragraph; paragraph_count], - bullets: vec![], - }; - let s = DocumentSpec { - title: "Huge".to_string(), - author: None, - sections: vec![big], - }; - // Sanity: this spec passes every per-field check. - assert!(s.sections.len() <= MAX_SECTIONS); - assert_rejects(&s, "sections"); -} - -#[test] -fn rejects_an_aggregate_overrun_that_a_bullet_crosses() { - // The heading and paragraph loops each carry their own budget check; so does - // the bullet loop, and only a spec whose overrun lands on a bullet drives - // that third branch. - let bullet = "b".repeat(MAX_PARAGRAPH_CHARS); - let bullet_count = MAX_TOTAL_CHARS / MAX_PARAGRAPH_CHARS + 1; - assert!(bullet_count <= MAX_BULLETS_PER_SECTION); - let s = DocumentSpec { - title: "Bullets".to_string(), - author: None, - sections: vec![DocumentSection { - heading: None, - paragraphs: vec![], - bullets: vec![bullet; bullet_count], - }], - }; - assert_rejects(&s, "sections"); -} - -#[test] -fn rejects_an_aggregate_overrun_that_a_heading_crosses() { - // Headings cannot reach the aggregate cap on their own: MAX_SECTIONS × - // MAX_TEXT_CHARS is 256_000, two orders of magnitude under MAX_TOTAL_CHARS. - // Driving the heading branch therefore means spending the budget down to a - // single character of headroom in an earlier section, then letting a - // perfectly legal heading cross it. - let title = "Headings"; - let filler_count = MAX_TOTAL_CHARS / MAX_PARAGRAPH_CHARS - 1; - assert!(filler_count <= MAX_PARAGRAPHS_PER_SECTION); - let used = title.chars().count() + filler_count * MAX_PARAGRAPH_CHARS; - // Leave exactly one character of headroom. - let tail = MAX_TOTAL_CHARS - used - 1; - assert!(tail <= MAX_PARAGRAPH_CHARS); - - let mut paragraphs = vec!["p".repeat(MAX_PARAGRAPH_CHARS); filler_count]; - paragraphs.push("p".repeat(tail)); - - let s = DocumentSpec { - title: title.to_string(), - author: None, - sections: vec![ - DocumentSection { - heading: None, - paragraphs, - bullets: vec![], - }, - DocumentSection { - // Two characters against one character of headroom. - heading: Some("hh".to_string()), - paragraphs: vec![], - bullets: vec![], - }, - ], - }; - assert!(s.sections.len() <= MAX_SECTIONS); - assert_rejects(&s, "sections"); -} - -#[test] -fn is_blank_reflects_content_presence() { - assert!(!section().is_blank()); - assert!(DocumentSection { - heading: None, - paragraphs: vec![], - bullets: vec![], - } - .is_blank()); - // A heading alone is enough content. - assert!(!DocumentSection { - heading: Some("Only a heading".to_string()), - paragraphs: vec![], - bullets: vec![], - } - .is_blank()); -} - -#[test] -fn total_chars_sums_every_text_field() { - let s = DocumentSpec { - title: "abcd".to_string(), // 4 - author: Some("xy".to_string()), // 2 - sections: vec![DocumentSection { - heading: Some("hij".to_string()), // 3 - paragraphs: vec!["pq".to_string()], // 2 - bullets: vec!["b".to_string()], // 1 - }], - }; - assert_eq!(s.total_chars(), 12); -} - -#[test] -fn spec_round_trips_through_json() { - let s = spec(); - let json = serde_json::to_string(&s).expect("serialises"); - let back: DocumentSpec = serde_json::from_str(&json).expect("deserialises"); - assert_eq!(back, s); -} - -#[test] -fn spec_rejects_unknown_json_fields() { - // `deny_unknown_fields` makes a typo'd key a loud rejection rather than a - // silently ignored one — the whole point at an LLM tool boundary. - let json = r#"{"title":"T","sections":[],"titel":"typo"}"#; - assert!(serde_json::from_str::(json).is_err()); -} - -#[test] -fn spec_defaults_optional_fields() { - let s: DocumentSpec = serde_json::from_str(r#"{"title":"T"}"#).expect("deserialises"); - assert_eq!(s.author, None); - assert!(s.sections.is_empty()); -} diff --git a/src/openhuman/tools/impl/document/format/spec/image/mod.rs b/src/openhuman/tools/impl/document/format/spec/image/mod.rs deleted file mode 100644 index 4fab9d934e..0000000000 --- a/src/openhuman/tools/impl/document/format/spec/image/mod.rs +++ /dev/null @@ -1,161 +0,0 @@ -//! Raster-image identification for specs that embed images. -//! -//! Two formats are supported, PNG and JPEG, and the restriction is deliberate -//! rather than incidental: the OOXML presentation writer this crate drives -//! declares no `webp` default in the generated `[Content_Types].xml`, and its -//! automatic format detection misclassifies `webp` as PNG — producing a part -//! `PowerPoint` refuses to render. Accepting only what can actually be embedded -//! turns that into a clean rejection at the boundary. -//! -//! Identification is done by reading the container header directly, in about a -//! hundred lines and with no dependencies, rather than by pulling in a decoding -//! stack. Nothing here decodes pixels: it answers "which format is this" and -//! "what are its native dimensions", which is all a layout engine needs to -//! place an image with the right aspect ratio. -//! -//! Like the rest of [`crate::openhuman::tools::implementations::document::format::spec`], this module is compiled in every build. A -//! host resolving image bytes has to identify and measure them to *build* a -//! spec, and that must not require the writer. - -use serde::{Deserialize, Serialize}; - -/// A raster image format that can be embedded in a generated document. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -#[serde(rename_all = "UPPERCASE")] -pub enum ImageFormat { - /// Portable Network Graphics. - Png, - /// JPEG / JFIF. - Jpeg, -} - -impl ImageFormat { - /// The format's canonical OOXML name — `"PNG"` or `"JPEG"`. - #[must_use] - pub const fn as_str(self) -> &'static str { - match self { - Self::Png => "PNG", - Self::Jpeg => "JPEG", - } - } - - /// Identify `bytes` by its container header. - /// - /// Returns `None` for a truncated header or any format other than the two - /// embeddable ones — including GIF, WebP and BMP, which are recognisable - /// but not embeddable. - #[must_use] - pub fn sniff(bytes: &[u8]) -> Option { - if bytes.starts_with(&[0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]) { - Some(Self::Png) - } else if bytes.starts_with(&[0xFF, 0xD8, 0xFF]) { - Some(Self::Jpeg) - } else { - None - } - } - - /// Native `(width, height)` of `bytes` in pixels, read from the header. - /// - /// Returns `None` when the header is truncated or malformed, or when either - /// dimension is zero — a degenerate image cannot be placed aspect-correctly - /// and is rejected rather than divided by. - #[must_use] - pub fn dimensions(self, bytes: &[u8]) -> Option<(u32, u32)> { - match self { - Self::Png => png_dimensions(bytes), - Self::Jpeg => jpeg_dimensions(bytes), - } - } -} - -impl std::fmt::Display for ImageFormat { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str(self.as_str()) - } -} - -/// PNG: 8-byte signature, then an `IHDR` chunk whose width / height are -/// big-endian `u32`s at byte offsets 16 and 20. -fn png_dimensions(bytes: &[u8]) -> Option<(u32, u32)> { - if bytes.len() < 24 || &bytes[12..16] != b"IHDR" { - return None; - } - let w = u32::from_be_bytes([bytes[16], bytes[17], bytes[18], bytes[19]]); - let h = u32::from_be_bytes([bytes[20], bytes[21], bytes[22], bytes[23]]); - if w == 0 || h == 0 { - return None; - } - Some((w, h)) -} - -/// JPEG: walk the marker segments until a Start-Of-Frame is hit; its payload -/// carries height then width as big-endian `u16`s. -fn jpeg_dimensions(bytes: &[u8]) -> Option<(u32, u32)> { - let mut i = 2; // skip the leading FF D8 SOI - while i + 3 < bytes.len() { - if bytes[i] != 0xFF { - i += 1; - continue; - } - let marker = bytes[i + 1]; - i += 2; - // Standalone markers carry no length field: padding fill bytes, TEM, - // RSTn, SOI and EOI. Reading the next two bytes as a length here would - // desynchronise the walk and reject a valid file — TEM in particular is - // legal before the frame header. - if marker == 0xFF - || marker == 0x01 - || marker == 0xD8 - || marker == 0xD9 - || (0xD0..=0xD7).contains(&marker) - { - continue; - } - if i + 1 >= bytes.len() { - return None; - } - let seg_len = u16::from_be_bytes([bytes[i], bytes[i + 1]]) as usize; - if seg_len < 2 { - return None; - } - // SOF markers carrying frame dimensions. Excludes 0xC4 (DHT), - // 0xC8 (JPG) and 0xCC (DAC), which share the 0xCn range but are not - // frame headers. - let is_sof = matches!( - marker, - 0xC0 | 0xC1 - | 0xC2 - | 0xC3 - | 0xC5 - | 0xC6 - | 0xC7 - | 0xC9 - | 0xCA - | 0xCB - | 0xCD - | 0xCE - | 0xCF - ); - if is_sof { - // segment: [len_hi len_lo precision h_hi h_lo w_hi w_lo ...] - if i + 6 >= bytes.len() { - return None; - } - let h = u32::from(u16::from_be_bytes([bytes[i + 3], bytes[i + 4]])); - let w = u32::from(u16::from_be_bytes([bytes[i + 5], bytes[i + 6]])); - if w == 0 || h == 0 { - return None; - } - return Some((w, h)); - } - i += seg_len; - } - None -} - -// Visible crate-wide under `cfg(test)`: the `png` / `jpeg` header builders here -// are the fixtures every image-carrying spec and every synthesis test needs, and -// one honest builder beats a base64 blob copied into three files. -#[cfg(test)] -pub(crate) mod test; diff --git a/src/openhuman/tools/impl/document/format/spec/image/test.rs b/src/openhuman/tools/impl/document/format/spec/image/test.rs deleted file mode 100644 index 90206fce8a..0000000000 --- a/src/openhuman/tools/impl/document/format/spec/image/test.rs +++ /dev/null @@ -1,151 +0,0 @@ -//! Unit tests for image identification and header measurement. - -#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] - -use super::{jpeg_dimensions, png_dimensions, ImageFormat}; - -/// A 1×1 PNG assembled byte-for-byte: signature, `IHDR`, `IDAT`, `IEND`. -/// -/// Built literally rather than decoded from base64 so the fixture needs no -/// dependency and the offsets under test are visible in the source. -pub(crate) fn png(width: u32, height: u32) -> Vec { - let mut out = vec![0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]; - out.extend_from_slice(&13u32.to_be_bytes()); // IHDR length - out.extend_from_slice(b"IHDR"); - out.extend_from_slice(&width.to_be_bytes()); - out.extend_from_slice(&height.to_be_bytes()); - out.extend_from_slice(&[0x08, 0x06, 0x00, 0x00, 0x00]); // depth, colour, etc. - out.extend_from_slice(&[0x00, 0x00, 0x00, 0x00]); // CRC placeholder - out.extend_from_slice(&0u32.to_be_bytes()); // empty IDAT - out.extend_from_slice(b"IDAT"); - out.extend_from_slice(&[0x00, 0x00, 0x00, 0x00]); - out.extend_from_slice(&0u32.to_be_bytes()); - out.extend_from_slice(b"IEND"); - out.extend_from_slice(&[0xAE, 0x42, 0x60, 0x82]); - out -} - -/// A minimal JPEG: SOI, an APP0 stub, then an SOF0 declaring `height × width`. -pub(crate) fn jpeg(width: u16, height: u16) -> Vec { - let mut out = vec![ - 0xFF, 0xD8, // SOI - 0xFF, 0xE0, 0x00, 0x04, 0x00, 0x00, // APP0, len=4, 2 payload bytes - 0xFF, 0xC0, 0x00, 0x0B, // SOF0, len=11 - 0x08, // precision - ]; - out.extend_from_slice(&height.to_be_bytes()); - out.extend_from_slice(&width.to_be_bytes()); - out.extend_from_slice(&[0x03, 0x00, 0x00, 0x00]); // components (filler) - out.extend_from_slice(&[0xFF, 0xD9]); // EOI - out -} - -#[test] -fn sniffs_png_and_jpeg() { - assert_eq!(ImageFormat::sniff(&png(1, 1)), Some(ImageFormat::Png)); - assert_eq!(ImageFormat::sniff(&jpeg(7, 5)), Some(ImageFormat::Jpeg)); -} - -#[test] -fn rejects_non_images_and_unembeddable_formats() { - assert_eq!(ImageFormat::sniff(b"not an image"), None); - // GIF and WebP are recognisable, but the writer cannot embed either. - assert_eq!(ImageFormat::sniff(b"GIF89a....."), None); - assert_eq!(ImageFormat::sniff(b"RIFF\0\0\0\0WEBP"), None); - assert_eq!(ImageFormat::sniff(&[]), None); -} - -#[test] -fn reads_png_dimensions() { - assert_eq!(ImageFormat::Png.dimensions(&png(1, 1)), Some((1, 1)), "1x1"); - assert_eq!( - ImageFormat::Png.dimensions(&png(1920, 1080)), - Some((1920, 1080)) - ); -} - -#[test] -fn reads_jpeg_dimensions() { - assert_eq!(ImageFormat::Jpeg.dimensions(&jpeg(7, 5)), Some((7, 5))); -} - -#[test] -fn truncated_headers_yield_none() { - assert_eq!(png_dimensions(&[0x89, 0x50, 0x4E, 0x47]), None); - assert_eq!(jpeg_dimensions(&[0xFF, 0xD8]), None); -} - -#[test] -fn a_png_without_an_ihdr_chunk_yields_none() { - let mut bytes = png(4, 4); - bytes[12..16].copy_from_slice(b"XXXX"); - assert_eq!(png_dimensions(&bytes), None); -} - -#[test] -fn a_zero_dimension_yields_none() { - // Degenerate images cannot be placed aspect-correctly; they are rejected - // rather than divided by. - assert_eq!(png_dimensions(&png(0, 8)), None); - assert_eq!(png_dimensions(&png(8, 0)), None); - assert_eq!(jpeg_dimensions(&jpeg(0, 8)), None); - assert_eq!(jpeg_dimensions(&jpeg(8, 0)), None); -} - -#[test] -fn a_jpeg_with_no_start_of_frame_yields_none() { - // SOI, then an APP0 segment and EOI — a valid marker stream carrying no - // frame header at all. - let bytes = vec![ - 0xFF, 0xD8, 0xFF, 0xE0, 0x00, 0x04, 0x00, 0x00, 0xFF, 0xD9, 0x00, 0x00, - ]; - assert_eq!(jpeg_dimensions(&bytes), None); -} - -#[test] -fn a_jpeg_with_a_degenerate_segment_length_yields_none() { - // A declared segment length below the two length bytes themselves would - // make the walk loop forever if it were trusted. - let bytes = vec![0xFF, 0xD8, 0xFF, 0xE0, 0x00, 0x01, 0x00, 0x00, 0x00]; - assert_eq!(jpeg_dimensions(&bytes), None); -} - -#[test] -fn a_jpeg_skips_standalone_and_non_frame_markers_before_the_frame() { - // Restart markers and a DHT (0xC4, in the 0xCn range but not a frame - // header) must both be stepped over rather than mistaken for an SOF. - let mut bytes = vec![0xFF, 0xD8, 0xFF, 0xD0, 0xFF, 0xFF]; - bytes.extend_from_slice(&[0xFF, 0xC4, 0x00, 0x04, 0x00, 0x00]); // DHT - bytes.extend_from_slice(&[0xFF, 0xC0, 0x00, 0x0B, 0x08]); - bytes.extend_from_slice(&11u16.to_be_bytes()); // height - bytes.extend_from_slice(&22u16.to_be_bytes()); // width - bytes.extend_from_slice(&[0x03, 0x00, 0x00, 0x00, 0xFF, 0xD9]); - assert_eq!(jpeg_dimensions(&bytes), Some((22, 11))); -} - -#[test] -fn a_jpeg_with_a_tem_marker_before_the_frame_is_still_measured() { - // TEM (0xFF01) carries no length field. Reading the next two bytes as one - // desynchronises the walk and rejects a valid file. - let mut bytes = vec![0xFF, 0xD8, 0xFF, 0x01]; - bytes.extend_from_slice(&[0xFF, 0xC0, 0x00, 0x0B, 0x08]); - bytes.extend_from_slice(&33u16.to_be_bytes()); // height - bytes.extend_from_slice(&44u16.to_be_bytes()); // width - bytes.extend_from_slice(&[0x03, 0x00, 0x00, 0x00, 0xFF, 0xD9]); - assert_eq!(jpeg_dimensions(&bytes), Some((44, 33))); -} - -#[test] -fn format_renders_its_ooxml_name() { - assert_eq!(ImageFormat::Png.as_str(), "PNG"); - assert_eq!(ImageFormat::Jpeg.as_str(), "JPEG"); - assert_eq!(ImageFormat::Jpeg.to_string(), "JPEG"); -} - -#[test] -fn format_round_trips_through_json_as_its_ooxml_name() { - let json = serde_json::to_string(&ImageFormat::Png).expect("serialises"); - assert_eq!(json, r#""PNG""#); - let back: ImageFormat = serde_json::from_str(&json).expect("deserialises"); - assert_eq!(back, ImageFormat::Png); -} diff --git a/src/openhuman/tools/impl/document/format/spec/mod.rs b/src/openhuman/tools/impl/document/format/spec/mod.rs deleted file mode 100644 index 21f8a2eaeb..0000000000 --- a/src/openhuman/tools/impl/document/format/spec/mod.rs +++ /dev/null @@ -1,45 +0,0 @@ -//! The wire contracts: typed document specs and their validation, with no -//! dependency on any format writer. -//! -//! Every format module in this crate (`docx`, `pptx`, …) synthesises bytes from -//! a spec defined here. The split matters for two reasons: -//! -//! 1. **A host can share the contract without paying for the codec.** This -//! module is `serde` plus the crate [`Error`](crate::openhuman::tools::implementations::document::format::Error) — nothing else. -//! It is compiled in *every* build, including `--no-default-features`, so a -//! host whose synthesis happens elsewhere (in another process, or behind a -//! message bus) still gets the one authoritative definition of the spec -//! instead of re-declaring it and drifting. -//! 2. **Validation is cheap and belongs at the boundary.** The specs validate -//! themselves without touching a writer, so a host can reject a malformed -//! LLM tool call before paying for a blocking hop or a round trip. -//! -//! # Where things live -//! -//! - [`document`] — `.docx`: [`DocumentSpec`], [`DocumentSection`]. -//! - [`presentation`] — `.pptx`: [`PresentationSpec`], [`SlideSpec`], -//! [`SlideImage`]. -//! - [`image`] — [`ImageFormat`], for specs that embed raster images. -//! -//! **Types are re-exported here; limits are not.** Each format's limits stay -//! inside its own module, because the same name means a different thing in each -//! — `document::MAX_TEXT_CHARS` bounds a heading, `presentation::MAX_TEXT_CHARS` -//! bounds a bullet — and flattening them would put two distinct constants under -//! one name. Reach for `spec::presentation::MAX_SLIDES` and read it as the -//! sentence it is. -//! -//! The format modules re-export both the types and the limits they consume, so -//! `crate::openhuman::tools::implementations::document::format::docx::DocumentSpec` and [`crate::openhuman::tools::implementations::document::format::spec::DocumentSpec`] name the -//! same type. -//! -//! [`crate::openhuman::tools::implementations::document::format::spec::DocumentSpec`]: DocumentSpec - -pub mod document; -pub mod image; -pub mod presentation; - -pub use document::{DocumentSection, DocumentSpec}; -pub use image::ImageFormat; -pub use presentation::wire::{WirePresentationSpec, WireSlideImage, WireSlideSpec}; -#[allow(unused_imports)] -pub use presentation::{PresentationSpec, SlideImage, SlideSpec}; diff --git a/src/openhuman/tools/impl/document/format/spec/presentation/mod.rs b/src/openhuman/tools/impl/document/format/spec/presentation/mod.rs deleted file mode 100644 index 9e030540cc..0000000000 --- a/src/openhuman/tools/impl/document/format/spec/presentation/mod.rs +++ /dev/null @@ -1,340 +0,0 @@ -//! The `.pptx` presentation spec: the typed description a caller hands to -//! `pptx::generate`, plus the size limits every spec is validated against. -//! -//! Same contract rules as [`crate::openhuman::tools::implementations::document::format::spec::document`] — `deny_unknown_fields`, -//! public limits, `validate` before synthesis — with one structural difference -//! worth understanding. -//! -//! # Images are bytes here, not references -//! -//! A [`SlideImage`] carries the image *bytes*, its format, and its native pixel -//! dimensions. It deliberately does **not** carry a path, a URL, or an -//! application-specific identifier, because resolving any of those is host -//! policy this crate has no business holding: which directories an agent may -//! read, whether a given identifier belongs to the caller, and whether fetching -//! a URL is an acceptable request to originate are all questions with different -//! answers in every host. A host resolves indirection under its own rules and -//! hands over the resulting bytes. -//! -//! [`SlideImage::from_bytes`] does the mechanical half of that hand-off: -//! identify the format and read the dimensions, or reject the bytes. It needs -//! no format writer, so a host can build and validate a whole spec in a build -//! with the `pptx` feature off. - -use serde::{Deserialize, Serialize}; - -use crate::openhuman::tools::implementations::document::format::spec::image::ImageFormat; -use crate::openhuman::tools::implementations::document::format::{Error, Result}; - -/// Maximum number of content slides a single deck may contain. -/// -/// Bounds generation time and output size; a caller with more material is -/// expected to split it across multiple decks. -pub const MAX_SLIDES: usize = 64; - -/// Maximum length, in Unicode scalar values, of any single text field — the -/// deck title, the author byline, the theme hint, a slide title, a slide body, -/// one bullet, the speaker notes, or an image caption. -pub const MAX_TEXT_CHARS: usize = 2_000; - -/// Maximum number of bullets on a single slide. -/// -/// Higher counts produce a slide nobody can read, and bloat the output. -pub const MAX_BULLETS_PER_SLIDE: usize = 32; - -/// Maximum number of images attached to a single slide. -/// -/// The single-column layout stacks images vertically in the lower band of the -/// slide; past this count each one is too small to read. -pub const MAX_IMAGES_PER_SLIDE: usize = 6; - -/// Maximum number of images across the whole deck. -/// -/// Bounds the embedded media payload regardless of how the images are -/// distributed across slides. -pub const MAX_IMAGES_PER_DECK: usize = 8; - -/// Maximum size, in bytes, of a single embedded image. -pub const MAX_IMAGE_BYTES: usize = 5 * 1024 * 1024; - -/// One image embedded on a slide. -/// -/// Construct with [`SlideImage::from_bytes`] rather than by hand: it derives -/// `format` and the dimensions from the bytes, which keeps the three fields -/// consistent by construction. [`PresentationSpec::validate`] re-checks that -/// consistency, because a spec can also arrive over a wire. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct SlideImage { - /// The encoded image, as PNG or JPEG bytes. - pub bytes: Vec, - /// The format of `bytes`. - pub format: ImageFormat, - /// Native width in pixels, used to place the image without distorting it. - pub width_px: u32, - /// Native height in pixels, used to place the image without distorting it. - pub height_px: u32, - /// Optional caption, rendered as a bullet beneath the image. - #[serde(default)] - pub caption: Option, -} - -impl SlideImage { - /// Identify and measure `bytes`, producing a consistent [`SlideImage`]. - /// - /// # Errors - /// - /// Returns [`Error::InvalidInput`] when `bytes` is empty, exceeds - /// [`MAX_IMAGE_BYTES`], is not PNG or JPEG, or carries a header this crate - /// cannot measure. - pub fn from_bytes(bytes: Vec, caption: Option) -> Result { - if bytes.is_empty() { - return Err(Error::invalid_input("bytes", "must not be empty")); - } - if bytes.len() > MAX_IMAGE_BYTES { - return Err(Error::invalid_input( - "bytes", - format!("must be ≤ {MAX_IMAGE_BYTES} bytes"), - )); - } - let format = ImageFormat::sniff(&bytes) - .ok_or_else(|| Error::invalid_input("bytes", "must be a PNG or JPEG image"))?; - let (width_px, height_px) = format.dimensions(&bytes).ok_or_else(|| { - Error::invalid_input( - "bytes", - format!("{format} header is truncated or malformed"), - ) - })?; - Ok(Self { - bytes, - format, - width_px, - height_px, - caption, - }) - } -} - -/// One content slide of the deck, rendered in spec order. -/// -/// At least one of `title`, `body`, or `bullets` must carry renderable text. -/// Images alone are not enough — a slide holding only an image and no label -/// reads as a rendering bug rather than a design choice, and synthesis drops -/// blank text anyway. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct SlideSpec { - /// Slide title. May be blank for a visually minimal slide, as long as the - /// body or bullets carry text. - #[serde(default)] - pub title: String, - /// Body text, rendered above the bullets. Plain text only. - #[serde(default)] - pub body: Option, - /// Bullets, rendered after the body text. - #[serde(default)] - pub bullets: Vec, - /// Speaker notes attached to the slide. - #[serde(default)] - pub speaker_notes: Option, - /// Images, stacked in a single column beneath the text. - #[serde(default)] - pub images: Vec, -} - -impl SlideSpec { - /// Returns `true` when the slide carries no renderable text at all — the - /// title, body, and every bullet are absent or blank. - /// - /// Synthesis trims and drops blank entries, so a slide holding only - /// `[" "]` would render without text despite carrying entries. - #[must_use] - pub fn is_textless(&self) -> bool { - let has_title = !self.title.trim().is_empty(); - let has_body = self.body.as_deref().is_some_and(|b| !b.trim().is_empty()); - let has_bullets = self.bullets.iter().any(|b| !b.trim().is_empty()); - !(has_title || has_body || has_bullets) - } -} - -/// A complete `.pptx` presentation spec. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct PresentationSpec { - /// Deck title, rendered on a leading title slide. Required and non-blank. - pub title: String, - /// Optional author byline, rendered beneath the deck title. - #[serde(default)] - pub author: Option, - /// Optional theme hint. - /// - /// Accepted and validated but not yet acted on: synthesis uses the writer's - /// default template regardless. It is part of the contract so a host's tool - /// schema does not have to change when template selection lands. - #[serde(default)] - pub theme: Option, - /// Content slides, in display order. Must contain at least one entry. - #[serde(default)] - pub slides: Vec, -} - -impl PresentationSpec { - /// Total number of images across every slide. - #[must_use] - pub fn image_count(&self) -> usize { - self.slides - .iter() - .map(|slide| slide.images.len()) - .sum::() - } - - /// Check the spec against every documented size limit, and check that each - /// image's declared format and dimensions match its bytes. - /// - /// Callers do not have to invoke this: `pptx::generate` validates before it - /// synthesises anything. It is public so a host can reject a malformed spec - /// at its own boundary — an LLM tool call, say — and hand back the - /// structured [`Error::InvalidInput`] before paying for a blocking hop, a - /// process boundary, or a bus round trip. - /// - /// # Errors - /// - /// Returns [`Error::InvalidInput`] naming the first field that violates a - /// limit. Fields are checked in spec order, so the reported field is stable - /// for a given spec. - pub fn validate(&self) -> Result<()> { - if self.title.trim().is_empty() { - return Err(Error::invalid_input("title", "must not be empty")); - } - Self::check_text_len("title", &self.title)?; - if let Some(author) = self.author.as_deref() { - Self::check_text_len("author", author)?; - } - if let Some(theme) = self.theme.as_deref() { - Self::check_text_len("theme", theme)?; - } - if self.slides.is_empty() { - return Err(Error::invalid_input( - "slides", - "must contain at least one slide", - )); - } - if self.slides.len() > MAX_SLIDES { - return Err(Error::invalid_input( - "slides", - format!("must contain ≤ {MAX_SLIDES} slides"), - )); - } - // Checked across the whole deck rather than per slide: the per-slide cap - // bounds readability, this one bounds the embedded media payload however - // the images are distributed. - if self.image_count() > MAX_IMAGES_PER_DECK { - return Err(Error::invalid_input( - "slides[].images", - format!("deck must contain ≤ {MAX_IMAGES_PER_DECK} images total"), - )); - } - - for (i, slide) in self.slides.iter().enumerate() { - if slide.is_textless() { - return Err(Error::invalid_input( - format!("slides[{i}]"), - "must have at least one of title / body / bullets", - )); - } - Self::check_text_len(format!("slides[{i}].title"), &slide.title)?; - if let Some(body) = slide.body.as_deref() { - Self::check_text_len(format!("slides[{i}].body"), body)?; - } - if slide.bullets.len() > MAX_BULLETS_PER_SLIDE { - return Err(Error::invalid_input( - format!("slides[{i}].bullets"), - format!("must contain ≤ {MAX_BULLETS_PER_SLIDE} bullets"), - )); - } - for (b, bullet) in slide.bullets.iter().enumerate() { - Self::check_text_len(format!("slides[{i}].bullets[{b}]"), bullet)?; - } - if let Some(notes) = slide.speaker_notes.as_deref() { - Self::check_text_len(format!("slides[{i}].speaker_notes"), notes)?; - } - if slide.images.len() > MAX_IMAGES_PER_SLIDE { - return Err(Error::invalid_input( - format!("slides[{i}].images"), - format!("must contain ≤ {MAX_IMAGES_PER_SLIDE} images"), - )); - } - for (m, image) in slide.images.iter().enumerate() { - Self::check_image(&format!("slides[{i}].images[{m}]"), image)?; - } - } - Ok(()) - } - - /// Reject a text field longer than [`MAX_TEXT_CHARS`] scalar values. - fn check_text_len(field: impl Into, value: &str) -> Result<()> { - if value.chars().count() > MAX_TEXT_CHARS { - return Err(Error::invalid_input( - field, - format!("must be ≤ {MAX_TEXT_CHARS} chars"), - )); - } - Ok(()) - } - - /// Re-derive an image's format and dimensions from its bytes and reject any - /// disagreement with what the spec declares. - /// - /// [`SlideImage::from_bytes`] keeps the fields consistent by construction, - /// but a spec can also arrive as deserialized JSON, where the three fields - /// are independent. A declared format that does not match the bytes yields - /// a part the reader refuses to render, and declared dimensions that do not - /// match distort the image silently — both are worth a named rejection. - fn check_image(field: &str, image: &SlideImage) -> Result<()> { - if image.bytes.is_empty() { - return Err(Error::invalid_input( - format!("{field}.bytes"), - "must not be empty", - )); - } - if image.bytes.len() > MAX_IMAGE_BYTES { - return Err(Error::invalid_input( - format!("{field}.bytes"), - format!("must be ≤ {MAX_IMAGE_BYTES} bytes"), - )); - } - let sniffed = ImageFormat::sniff(&image.bytes).ok_or_else(|| { - Error::invalid_input(format!("{field}.bytes"), "must be a PNG or JPEG image") - })?; - if sniffed != image.format { - return Err(Error::invalid_input( - format!("{field}.format"), - format!("declared {} but the bytes are {sniffed}", image.format), - )); - } - let (width_px, height_px) = sniffed.dimensions(&image.bytes).ok_or_else(|| { - Error::invalid_input( - format!("{field}.bytes"), - format!("{sniffed} header is truncated or malformed"), - ) - })?; - if (width_px, height_px) != (image.width_px, image.height_px) { - return Err(Error::invalid_input( - format!("{field}.width_px"), - format!( - "declared {}x{} but the bytes are {width_px}x{height_px}", - image.width_px, image.height_px - ), - )); - } - if let Some(caption) = image.caption.as_deref() { - Self::check_text_len(format!("{field}.caption"), caption)?; - } - Ok(()) - } -} - -pub mod wire; - -#[cfg(test)] -mod test; diff --git a/src/openhuman/tools/impl/document/format/spec/presentation/test.rs b/src/openhuman/tools/impl/document/format/spec/presentation/test.rs deleted file mode 100644 index 9dcfef7fd3..0000000000 --- a/src/openhuman/tools/impl/document/format/spec/presentation/test.rs +++ /dev/null @@ -1,367 +0,0 @@ -//! Unit tests for the presentation wire contract. -//! -//! Format-independent, like the spec itself: these must pass in a build with -//! every format feature off. - -#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] - -use super::{ - PresentationSpec, SlideImage, SlideSpec, MAX_BULLETS_PER_SLIDE, MAX_IMAGES_PER_DECK, - MAX_IMAGES_PER_SLIDE, MAX_IMAGE_BYTES, MAX_SLIDES, MAX_TEXT_CHARS, -}; -use crate::openhuman::tools::implementations::document::format::spec::image::test::{jpeg, png}; -use crate::openhuman::tools::implementations::document::format::spec::image::ImageFormat; -use crate::openhuman::tools::implementations::document::format::Error; - -/// One valid slide carrying a title, a body, and a bullet. -fn slide() -> SlideSpec { - SlideSpec { - title: "Overview".to_string(), - body: Some("The situation so far.".to_string()), - bullets: vec!["A bullet".to_string()], - speaker_notes: Some("Keep it short.".to_string()), - images: vec![], - } -} - -/// A minimal valid spec; each test mutates one field to drive a single branch. -fn spec() -> PresentationSpec { - PresentationSpec { - title: "Quarterly Review".to_string(), - author: Some("Alice".to_string()), - theme: Some("plain".to_string()), - slides: vec![slide()], - } -} - -/// A valid image built from real header bytes. -fn image() -> SlideImage { - SlideImage::from_bytes(png(320, 200), Some("A chart".to_string())).expect("valid png") -} - -/// Assert `spec` is rejected with an `InvalidInput` naming `field`. -fn assert_rejects(spec: &PresentationSpec, field: &str) { - match spec.validate() { - Err(Error::InvalidInput { field: f, .. }) => { - assert_eq!(f, field, "unexpected rejected field"); - } - other => panic!("expected InvalidInput({field}), got {other:?}"), - } -} - -#[test] -fn accepts_a_well_formed_spec() { - assert!(spec().validate().is_ok()); -} - -#[test] -fn accepts_a_spec_with_images() { - let mut s = spec(); - s.slides[0].images = vec![image()]; - assert!(s.validate().is_ok()); -} - -#[test] -fn rejects_a_blank_deck_title() { - let mut s = spec(); - s.title = " ".to_string(); - assert_rejects(&s, "title"); -} - -#[test] -fn rejects_over_long_deck_level_text() { - for (field, mutate) in [("title", 0), ("author", 1), ("theme", 2)] { - let mut s = spec(); - let long = "x".repeat(MAX_TEXT_CHARS + 1); - match mutate { - 0 => s.title = long, - 1 => s.author = Some(long), - _ => s.theme = Some(long), - } - assert_rejects(&s, field); - } -} - -#[test] -fn rejects_a_spec_with_no_slides() { - let mut s = spec(); - s.slides.clear(); - assert_rejects(&s, "slides"); -} - -#[test] -fn rejects_too_many_slides() { - let mut s = spec(); - s.slides = vec![slide(); MAX_SLIDES + 1]; - assert_rejects(&s, "slides"); -} - -#[test] -fn rejects_a_textless_slide() { - // Every text entry is present but whitespace-only, so synthesis would drop - // all of them and render an unlabelled slide. - let mut s = spec(); - s.slides = vec![SlideSpec { - title: " ".to_string(), - body: Some("\t".to_string()), - bullets: vec![String::new()], - speaker_notes: None, - images: vec![], - }]; - assert_rejects(&s, "slides[0]"); -} - -#[test] -fn rejects_a_slide_carrying_only_an_image() { - // Images do not satisfy the "must have text" rule: an unlabelled slide - // reads as a rendering bug rather than a design choice. - let mut s = spec(); - s.slides = vec![SlideSpec { - title: String::new(), - body: None, - bullets: vec![], - speaker_notes: None, - images: vec![image()], - }]; - assert_rejects(&s, "slides[0]"); -} - -#[test] -fn rejects_over_long_slide_text_naming_its_index() { - let long = || "x".repeat(MAX_TEXT_CHARS + 1); - - let mut s = spec(); - s.slides.push(SlideSpec { - title: long(), - ..slide() - }); - assert_rejects(&s, "slides[1].title"); - - let mut s = spec(); - s.slides[0].body = Some(long()); - assert_rejects(&s, "slides[0].body"); - - let mut s = spec(); - s.slides[0].bullets = vec!["ok".to_string(), long()]; - assert_rejects(&s, "slides[0].bullets[1]"); - - let mut s = spec(); - s.slides[0].speaker_notes = Some(long()); - assert_rejects(&s, "slides[0].speaker_notes"); -} - -#[test] -fn rejects_too_many_bullets() { - let mut s = spec(); - s.slides[0].bullets = vec!["b".to_string(); MAX_BULLETS_PER_SLIDE + 1]; - assert_rejects(&s, "slides[0].bullets"); -} - -#[test] -fn rejects_too_many_images_on_one_slide() { - let mut s = spec(); - s.slides[0].images = vec![image(); MAX_IMAGES_PER_SLIDE + 1]; - assert_rejects(&s, "slides[0].images"); -} - -#[test] -fn rejects_too_many_images_across_the_deck() { - // Each slide is within the per-slide cap; only the deck total is not. The - // per-slide cap bounds readability, the deck cap bounds the media payload. - let per_slide = MAX_IMAGES_PER_SLIDE; - let slides_needed = MAX_IMAGES_PER_DECK / per_slide + 1; - let mut s = spec(); - s.slides = vec![ - SlideSpec { - images: vec![image(); per_slide], - ..slide() - }; - slides_needed - ]; - assert!(s.image_count() > MAX_IMAGES_PER_DECK); - assert_rejects(&s, "slides[].images"); -} - -#[test] -fn image_count_sums_across_slides() { - let mut s = spec(); - s.slides = vec![ - SlideSpec { - images: vec![image(), image()], - ..slide() - }, - SlideSpec { - images: vec![image()], - ..slide() - }, - ]; - assert_eq!(s.image_count(), 3); -} - -#[test] -fn rejects_an_over_long_image_caption() { - let mut s = spec(); - let mut img = image(); - img.caption = Some("c".repeat(MAX_TEXT_CHARS + 1)); - s.slides[0].images = vec![img]; - assert_rejects(&s, "slides[0].images[0].caption"); -} - -#[test] -fn from_bytes_derives_format_and_dimensions() { - let img = SlideImage::from_bytes(png(1920, 1080), None).expect("valid png"); - assert_eq!(img.format, ImageFormat::Png); - assert_eq!((img.width_px, img.height_px), (1920, 1080)); - assert_eq!(img.caption, None); - - let img = SlideImage::from_bytes(jpeg(640, 480), Some("j".to_string())).expect("valid jpeg"); - assert_eq!(img.format, ImageFormat::Jpeg); - assert_eq!((img.width_px, img.height_px), (640, 480)); -} - -#[test] -fn from_bytes_rejects_bad_input() { - assert!(matches!( - SlideImage::from_bytes(vec![], None), - Err(Error::InvalidInput { .. }) - )); - assert!(matches!( - SlideImage::from_bytes(b"not an image".to_vec(), None), - Err(Error::InvalidInput { .. }) - )); - // PNG signature with a truncated IHDR: the right format, unmeasurable. - assert!(matches!( - SlideImage::from_bytes(vec![0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A], None), - Err(Error::InvalidInput { .. }) - )); -} - -#[test] -fn from_bytes_rejects_an_oversize_image() { - // A real PNG header followed by enough filler to cross the cap, so the - // rejection is the size check rather than the sniff. - let mut bytes = png(8, 8); - bytes.resize(MAX_IMAGE_BYTES + 1, 0); - assert!(matches!( - SlideImage::from_bytes(bytes, None), - Err(Error::InvalidInput { .. }) - )); -} - -#[test] -fn validate_rejects_an_image_whose_declared_format_contradicts_its_bytes() { - // `from_bytes` cannot produce this, but deserialized JSON can: the three - // fields are independent on the wire. A wrong format yields a part the - // reader refuses to render, so it is worth a named rejection. - let mut s = spec(); - let mut img = image(); - img.format = ImageFormat::Jpeg; - s.slides[0].images = vec![img]; - assert_rejects(&s, "slides[0].images[0].format"); -} - -#[test] -fn validate_rejects_an_image_whose_declared_dimensions_contradict_its_bytes() { - // Declared dimensions that disagree with the bytes distort the image - // silently, which is worse than failing. - let mut s = spec(); - let mut img = image(); - img.width_px += 1; - s.slides[0].images = vec![img]; - assert_rejects(&s, "slides[0].images[0].width_px"); -} - -#[test] -fn validate_rejects_empty_oversize_and_unrecognised_image_bytes() { - let mut s = spec(); - let mut img = image(); - img.bytes.clear(); - s.slides[0].images = vec![img]; - assert_rejects(&s, "slides[0].images[0].bytes"); - - let mut s = spec(); - let mut img = image(); - img.bytes = b"not an image".to_vec(); - s.slides[0].images = vec![img]; - assert_rejects(&s, "slides[0].images[0].bytes"); - - let mut s = spec(); - let mut img = image(); - img.bytes.resize(MAX_IMAGE_BYTES + 1, 0); - s.slides[0].images = vec![img]; - assert_rejects(&s, "slides[0].images[0].bytes"); -} - -#[test] -fn validate_rejects_an_image_with_an_unmeasurable_header() { - // Sniffs as PNG, but the IHDR is gone — measurement fails after the format - // check has already passed, which is a distinct branch. - let mut s = spec(); - let mut img = image(); - img.bytes.truncate(8); - s.slides[0].images = vec![img]; - assert_rejects(&s, "slides[0].images[0].bytes"); -} - -#[test] -fn is_textless_reflects_text_presence() { - assert!(!slide().is_textless()); - assert!(SlideSpec { - title: String::new(), - body: None, - bullets: vec![], - speaker_notes: None, - images: vec![], - } - .is_textless()); - // A title alone is enough. - assert!(!SlideSpec { - title: "Only a title".to_string(), - body: None, - bullets: vec![], - speaker_notes: None, - images: vec![], - } - .is_textless()); - // So is a body alone, or a bullet alone. - assert!(!SlideSpec { - title: String::new(), - body: Some("Body".to_string()), - bullets: vec![], - speaker_notes: None, - images: vec![], - } - .is_textless()); - assert!(!SlideSpec { - title: String::new(), - body: None, - bullets: vec!["Bullet".to_string()], - speaker_notes: None, - images: vec![], - } - .is_textless()); -} - -#[test] -fn spec_round_trips_through_json() { - let mut s = spec(); - s.slides[0].images = vec![image()]; - let json = serde_json::to_string(&s).expect("serialises"); - let back: PresentationSpec = serde_json::from_str(&json).expect("deserialises"); - assert_eq!(back, s); - assert!(back.validate().is_ok()); -} - -#[test] -fn spec_rejects_unknown_json_fields() { - let json = r#"{"title":"T","slides":[],"tilte":"typo"}"#; - assert!(serde_json::from_str::(json).is_err()); -} - -#[test] -fn spec_defaults_optional_fields() { - let s: PresentationSpec = serde_json::from_str(r#"{"title":"T"}"#).expect("deserialises"); - assert_eq!(s.author, None); - assert_eq!(s.theme, None); - assert!(s.slides.is_empty()); -} diff --git a/src/openhuman/tools/impl/document/format/spec/presentation/wire.rs b/src/openhuman/tools/impl/document/format/spec/presentation/wire.rs deleted file mode 100644 index ef9cb8504c..0000000000 --- a/src/openhuman/tools/impl/document/format/spec/presentation/wire.rs +++ /dev/null @@ -1,73 +0,0 @@ -//! The presentation spec as it crosses a bus, where bytes cannot travel inline. -//! -//! A `TinyBus` frame is a 16 MiB JSON document and a deck may legally carry -//! 40 MiB of images, so image bytes ride a stream beside the call rather than -//! inside it. A call has one stream and a deck has many images, so the images -//! are concatenated in slide order and each one declares its `byte_len`; the -//! module splits them apart and resolves each into a real -//! [`super::SlideImage`] — bytes, format and dimensions. -//! -//! The lengths live in the spec rather than in the stream because they are what -//! makes a truncated or over-long transfer a named rejection instead of a deck -//! with a picture assembled from two different images. -//! -//! Only the presentation spec needs this treatment. A document spec is text and -//! its aggregate cap keeps it inside a frame, so a document crosses unchanged. -//! -//! Defined here rather than in the module that serves it so a host driving that -//! module over a bus shares one definition of the shape instead of re-declaring -//! it. Like the rest of [`crate::openhuman::tools::implementations::document::format::spec`] it is serde and nothing else. - -use serde::{Deserialize, Serialize}; - -/// A slide image, as it appears on the bus: one byte range of the concatenated -/// image stream that travels beside the call. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct WireSlideImage { - /// Length of this image's bytes within the concatenated image stream. - pub byte_len: u64, - /// Optional caption, rendered as a bullet beneath the image. - #[serde(default)] - pub caption: Option, -} - -/// One content slide, as it appears on the bus. -/// -/// Identical to [`super::SlideSpec`] apart from `images`. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct WireSlideSpec { - /// Slide title. - #[serde(default)] - pub title: String, - /// Body text, rendered above the bullets. - #[serde(default)] - pub body: Option, - /// Bullets, rendered after the body text. - #[serde(default)] - pub bullets: Vec, - /// Speaker notes attached to the slide. - #[serde(default)] - pub speaker_notes: Option, - /// Images, each naming a staged blob. - #[serde(default)] - pub images: Vec, -} - -/// A deck, as it appears on the bus. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct WirePresentationSpec { - /// Deck title, rendered on a leading title slide. - pub title: String, - /// Optional author byline. - #[serde(default)] - pub author: Option, - /// Optional theme hint. - #[serde(default)] - pub theme: Option, - /// Content slides, in display order. - #[serde(default)] - pub slides: Vec, -} diff --git a/src/openhuman/tools/impl/document/mod.rs b/src/openhuman/tools/impl/document/mod.rs index a221fc01ef..2ccae1e399 100644 --- a/src/openhuman/tools/impl/document/mod.rs +++ b/src/openhuman/tools/impl/document/mod.rs @@ -39,7 +39,6 @@ use crate::openhuman::security::SecurityPolicy; use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolResult}; mod engine; -pub(crate) mod format; mod types; #[cfg(test)] diff --git a/src/openhuman/tools/impl/document/types.rs b/src/openhuman/tools/impl/document/types.rs index 6614f215e8..1222c5a49a 100644 --- a/src/openhuman/tools/impl/document/types.rs +++ b/src/openhuman/tools/impl/document/types.rs @@ -1,11 +1,15 @@ //! Typed input / output / error contracts for the `generate_document` tool. //! //! The *document* half of these contracts — the section spec, its size -//! limits, and the validation rules — lives in the host-local -//! [`format`](super::format) -//! module and is re-exported here. Nothing about "a title, some sections, and a bullet +//! limits, and the validation rules — lives in the vendored [`tinydocs`] crate +//! (`vendor/tinydocs`, taken with `default-features = false`) and is +//! re-exported here. Nothing about "a title, some sections, and a bullet //! list" is OpenHuman-specific, so the definitions live where any host can -//! reach them and this module keeps only what genuinely is ours: +//! reach them — and, crucially, where the `tinydocs` TinyBus module that +//! validates against them reads them from too. A host-local copy of the spec +//! is a fork of the contract by definition; that is why the guard in +//! `scripts/ci/check-vendored-crates.mjs` exists. This module keeps only what +//! genuinely is ours: //! //! - [`GenerateDocumentOutput`] — artifact ids and workspace paths, concepts //! the bus contract has no notion of. @@ -14,7 +18,7 @@ //! the deadline is OpenHuman's policy, applied by [`engine`](super::engine) //! around a synchronous crate call. //! -//! The re-exported [`GenerateDocumentInput`] is the format module's `DocumentSpec` +//! The re-exported [`GenerateDocumentInput`] is `tinydocs`' `DocumentSpec` //! under its historical OpenHuman name. Field names are unchanged, so the JSON //! tool schema the agent sees is byte-identical to before the extraction. @@ -26,7 +30,7 @@ use crate::openhuman::modules::documents::DocumentCallError; // the writer, so the gated `docx` module is not compiled here at all. The types // are the same ones the module validates against, which is the whole reason // `spec` is separable. -pub use crate::openhuman::tools::implementations::document::format::spec::document::{ +pub use tinydocs::spec::document::{ DocumentSpec as GenerateDocumentInput, MAX_BULLETS_PER_SECTION, MAX_PARAGRAPHS_PER_SECTION, MAX_PARAGRAPH_CHARS, MAX_SECTIONS, MAX_TEXT_CHARS, }; @@ -37,7 +41,7 @@ pub use crate::openhuman::tools::implementations::document::format::spec::docume // is private, so the re-export reads as unused to the compiler — hence the // explicit allow rather than dropping a name callers legitimately need. #[allow(unused_imports)] -pub use crate::openhuman::tools::implementations::document::format::spec::DocumentSection; +pub use tinydocs::spec::DocumentSection; /// Tool output returned via [`crate::openhuman::tools::traits::ToolResult`] /// as the JSON `data` field. @@ -81,11 +85,11 @@ impl DocumentError { /// the variant never carries an unbounded payload back to the agent. /// Same cap/suffix as the presentation tool's `truncate_stderr`. pub(super) fn truncate_stderr(raw: &str) -> String { - crate::openhuman::tools::implementations::document::format::Error::truncate_detail(raw) + tinydocs::Error::truncate_detail(raw) } } -impl From for DocumentError { +impl From for DocumentError { /// Map a spec-validation failure onto the agent-facing shape. /// /// This is the *local* path: `validate_input` below checks the spec against @@ -98,15 +102,15 @@ impl From for /// `GenerationTimeout` is deliberately absent: validation has no deadline, /// so only [`engine`](super::engine) produces that variant. /// - /// `crate::openhuman::tools::implementations::document::format::Error` is `#[non_exhaustive]`, so the catch-all arm is + /// `tinydocs::Error` is `#[non_exhaustive]`, so the catch-all arm is /// required by the compiler rather than chosen. It degrades a variant added /// by a future release to `GenerationFailed` carrying that variant's own /// `Display` text, and logs, so a crate bump that introduces a case worth /// handling structurally shows up rather than being swallowed. - fn from(err: crate::openhuman::tools::implementations::document::format::Error) -> Self { + fn from(err: tinydocs::Error) -> Self { match err { - crate::openhuman::tools::implementations::document::format::Error::InvalidInput { field, reason } => Self::InvalidInput { field, reason }, - crate::openhuman::tools::implementations::document::format::Error::GenerationFailed { detail } => Self::GenerationFailed { + tinydocs::Error::InvalidInput { field, reason } => Self::InvalidInput { field, reason }, + tinydocs::Error::GenerationFailed { detail } => Self::GenerationFailed { stderr_truncated: detail, }, other => { @@ -166,7 +170,7 @@ impl From for DocumentError { /// generic engine error. /// /// Delegates to `tinydocs`, which validates again inside -/// [`crate::openhuman::tools::implementations::document::format::docx::generate`]. The double check is intentional: validating +/// `tinydocs::docx::generate`. The double check is intentional: validating /// here lets the tool reject a bad call before allocating an artifact record, /// and the crate-side check keeps `generate` safe for any other caller. pub(super) fn validate_input(input: &GenerateDocumentInput) -> Result<(), DocumentError> { @@ -300,12 +304,10 @@ mod tests { fn tinydocs_invalid_input_keeps_its_field_and_reason() { // The structured pair is what the agent self-corrects on, so the // crate-boundary mapping must not flatten it into a message string. - let mapped = DocumentError::from( - crate::openhuman::tools::implementations::document::format::Error::InvalidInput { - field: "sections[3].bullets[1]".to_string(), - reason: "must be ≤ 20000 chars".to_string(), - }, - ); + let mapped = DocumentError::from(tinydocs::Error::InvalidInput { + field: "sections[3].bullets[1]".to_string(), + reason: "must be ≤ 20000 chars".to_string(), + }); match mapped { DocumentError::InvalidInput { field, reason } => { assert_eq!(field, "sections[3].bullets[1]"); @@ -319,15 +321,10 @@ mod tests { fn tinydocs_generation_failure_maps_without_re_truncating() { // `tinydocs` already truncated this detail; re-truncating would eat // the suffix and misreport how much was dropped. - let detail = - crate::openhuman::tools::implementations::document::format::Error::truncate_detail( - &"x".repeat(10_000), - ); - let mapped = DocumentError::from( - crate::openhuman::tools::implementations::document::format::Error::GenerationFailed { - detail: detail.clone(), - }, - ); + let detail = tinydocs::Error::truncate_detail(&"x".repeat(10_000)); + let mapped = DocumentError::from(tinydocs::Error::GenerationFailed { + detail: detail.clone(), + }); match mapped { DocumentError::GenerationFailed { stderr_truncated } => { assert_eq!(stderr_truncated, detail); @@ -339,10 +336,7 @@ mod tests { #[test] fn truncate_stderr_bounds_the_payload() { let out = DocumentError::truncate_stderr(&"x".repeat(10_000)); - assert_eq!( - out.chars().count(), - crate::openhuman::tools::implementations::document::format::Error::MAX_DETAIL_CHARS - ); + assert_eq!(out.chars().count(), tinydocs::Error::MAX_DETAIL_CHARS); assert!(out.ends_with("[…truncated]")); } diff --git a/src/openhuman/tools/impl/presentation/engine.rs b/src/openhuman/tools/impl/presentation/engine.rs index a1fdd376e1..b33e8663cb 100644 --- a/src/openhuman/tools/impl/presentation/engine.rs +++ b/src/openhuman/tools/impl/presentation/engine.rs @@ -1,7 +1,7 @@ //! Async wrapper around the `tinydocs` module's `.pptx` writer. //! //! The synthesis itself — the slide mapping, the single-column image layout, the -//! EMU geometry — lives in `crate::openhuman::tools::implementations::document::format::pptx` and runs inside the loaded module. +//! EMU geometry — lives in `tinydocs::pptx` and runs inside the loaded module. //! What is left here is the policy only a host can supply: //! //! 1. a deadline, because the module holds no opinion about how long a caller @@ -24,9 +24,7 @@ use std::time::Duration; -use crate::openhuman::tools::implementations::document::format::spec::{ - WirePresentationSpec, WireSlideImage, WireSlideSpec, -}; +use tinydocs::spec::{WirePresentationSpec, WireSlideImage, WireSlideSpec}; use tokio::time::timeout; use super::types::{GeneratePresentationInput, PresentationError, ResolvedSlideImage}; @@ -174,7 +172,7 @@ mod tests { //! What is left to test on this side of the bus. //! //! The deck shape, the image layout and the OOXML container are tested in - //! `crate::openhuman::tools::implementations::document::format::pptx`, where the code now lives — reproducing them here would + //! `tinydocs::pptx`, where the code now lives — reproducing them here would //! assert the same behaviour twice and drift the moment one copy changed. //! //! What only exists here is [`build_request`]: the deck and the concatenated @@ -206,8 +204,7 @@ mod tests { fn resolved(bytes: &[u8], caption: Option<&str>) -> ResolvedSlideImage { ResolvedSlideImage { bytes: bytes.to_vec(), - format: - crate::openhuman::tools::implementations::document::format::spec::ImageFormat::Png, + format: tinydocs::spec::ImageFormat::Png, width_px: 4, height_px: 4, caption: caption.map(str::to_string), diff --git a/src/openhuman/tools/impl/presentation/mod.rs b/src/openhuman/tools/impl/presentation/mod.rs index b833eb1810..1635821080 100644 --- a/src/openhuman/tools/impl/presentation/mod.rs +++ b/src/openhuman/tools/impl/presentation/mod.rs @@ -27,10 +27,10 @@ //! #3026 Files panel, and the orchestrator grounding rule in #3029 //! continue to work without change. -use crate::openhuman::tools::implementations::document::format::spec::ImageFormat; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; +use tinydocs::spec::ImageFormat; use async_trait::async_trait; use serde_json::{json, Value}; @@ -455,7 +455,7 @@ impl PresentationTool { )); } - // Identification and measurement live in `crate::openhuman::tools::implementations::document::format::spec::image`, which + // Identification and measurement live in `tinydocs::spec::image`, which // is ungated: a host resolving image bytes has to do this to build a // spec, and it must not need the writer to do it. One implementation // also means the host and the module cannot disagree about what is diff --git a/src/openhuman/tools/impl/presentation/types.rs b/src/openhuman/tools/impl/presentation/types.rs index 54a3119f78..051cbb0c1d 100644 --- a/src/openhuman/tools/impl/presentation/types.rs +++ b/src/openhuman/tools/impl/presentation/types.rs @@ -1,7 +1,7 @@ //! Typed input / output / error contracts for the `generate_presentation` tool. -use crate::openhuman::tools::implementations::document::format::spec::ImageFormat; use serde::{Deserialize, Serialize}; +use tinydocs::spec::ImageFormat; use crate::openhuman::modules::documents::DocumentCallError; diff --git a/vendor/tinydocs b/vendor/tinydocs new file mode 160000 index 0000000000..6a07dbe825 --- /dev/null +++ b/vendor/tinydocs @@ -0,0 +1 @@ +Subproject commit 6a07dbe8250a85e0ddfd16b75f47d37f95de92e1