Skip to content

fix(apple): validate keyed archive selectors consistently - #7728

Merged
gensokyo-dev merged 1 commit into
mainfrom
fix/7649-keyed-archive-empty-selector
Oct 28, 2026
Merged

gensokyo-dev merged 1 commit into
mainfrom
fix/7649-keyed-archive-empty-selector

Conversation

@gensokyo-dev

@gensokyo-dev gensokyo-dev commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Fixes #7649. inspect-keyed-archive BUNDLE "" --json previously reported an artifact I/O failure and file-access remediation. It now returns the normal CLI VALIDATION_ERROR with archive / too_small correction facts before target admission. Raw provider calls retain invalid_request / invalid_input, field issues and inspect_input_contract recovery instead of losing schema failures to I/O translation.

Change and scope

The CLI reuses all four fields of the domain-owned selector schema, removing duplicated constraints/defaults. Its shared numeric fields use z.number().int(), preserving integer semantics and Incur's numeric argv conversion. A single typed input parser replaces both untyped reader parses for bundle and standalone archives.

  • Semantic owner: domain selector contract; CLI binds its archive alias, and the artifact read boundary projects schema errors before generic artifact translation. Existing resource ownership and cleanup stay intact.
  • Ordinary bug fix within existing read-only capability; no new provider, execution, mutation, authority or maintainer scope decision is involved.
  • Domain JSON Schema is unchanged, and exported CLI options Schema is unchanged. The CLI archive argument now advertises the existing nonempty/NUL-free constraint. Evidence identity, provenance, graph paging, package/install behavior and ignored generated outputs retain their existing contracts.
  • Nonempty semantic selection errors still use provider field path; CLI alias attribution is the separate issue [Bug] CLI keyed-archive selection errors identify the bundle argument instead of archive #7630. This fix removes schema drift and untyped schema failures, without introducing an error-alias mechanism.
  • Read existing files or captured data. Verification used offline artifact reads; no app was launched.

Evidence and validation

Executed on Linux x64, Node 24.22.1/npm 11.16.0, current locked dependencies:

  • Base 0e4b806db: added regressions produced three failures (CLI, bundle provider, standalone provider all returned artifact_operation_failed / io) and one passing valid-selector control.
  • npm run test:focused -- tests/acceptance/keyedArchiveInputValidation.test.ts tests/boundary/filesystem/keyedArchiveInputValidation.test.ts tests/boundary/filesystem/keyedArchiveSelection.test.ts tests/boundary/filesystem/keyedArchiveStandaloneSelection.test.ts tests/boundary/cli/keyedArchiveSelectedPathCli.test.ts — 29 pass. Coverage includes empty/NUL selectors, missing-target admission order, valid retries on the same provider client, default standalone selection, aliases, and explicit --offset, --limit, --root CLI options.
  • The actual --schema --json output compiles in Ajv 2020 and rejects empty/NUL selectors. Separate executed comparisons confirm unchanged domain JSON Schema and CLI options Schema.
  • npm run check:fast — typecheck and lint pass.
  • Real XLD 20250302 (157.2): both revisions successfully inspect Contents/PlugIns/XLDLameOutput.bundle/Contents/Resources/zh_CN.lproj/XLDLameOutput.nib with --limit 3: 448 total objects, 3 returned, truncated=true, next_offset=3; archive digest matches source bytes (ed93b37bbd57fd379884f524a75cf8b37e0c1d14897c606ecc9b95db6bc6f116). The empty-selector failure is reproduced on the real app before the fix and corrected afterward.

Applicable boundary cases are missing/malformed values, public error delivery, retained source identity and paging, and reuse after refusal. No lifecycle change. Native macOS/Windows execution and Hopper/Ghidra were not exercised; XLD was inspected statically on Linux.

Performance evidence

Completed two-call task on the real app: invalid empty selection, then the known archive's three-object page. Two sequential rounds per revision, first / repeated fresh processes, without controlled filesystem-cold caches:

Metric Base Fixed
Combined elapsed 6.10 / 5.95 s 8.02 / 5.70 s
Combined CPU 7.60 / 7.40 s 9.46 / 6.47 s
Sampled peak process-family RSS 267.3 / 255.3 MiB 261.7 / 265.2 MiB

Family RSS sums command descendants sampled about every 20ms, excluding the measurement wrapper; brief peaks can be missed. The invalid call alone falls from 244.0 / 255.3 to 208.5 / 208.3 MiB. Timing varies and does not establish a general speedup. Tokenx stdout estimates for the two-call task are 1423 → 1410; optional CLI schema retrieval is 256 → 272 (schema plus task: 1679 → 1682). These are payload estimates, not whole-agent tokens; schema retrieval time/CPU and controlled cold-cache costs were not measured. No new cache, retained state, or analysis pass is introduced.

GitHub CI run 40651977394 passed: source/static checks, documentation, packaging, Windows native-control build, Linux native package baseline, real APK signature lane and all eight test shards. Greptile reviewed head d08dc6b99 at 5/5 with no actionable findings; Codex review completed with no findings. The human declaration gate remains intentionally unsatisfied under the user’s explicit waiver.

Author readiness

Human owner: @gensokyo-dev

The user explicitly requested a ready-for-review PR and waived the human declaration gate. No human diff/evidence review is claimed; those declarations remain unchecked.

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-28T07:45:32.967600Z d08dc6b PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@greptile-apps

greptile-apps Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable defect was found.

Summary

Shares keyed-archive selector rules between the domain and CLI. Both archive readers now turn schema failures into typed input errors before generic artifact error translation.

  • Keyed-archive selectors return input errors when their values are invalid.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  CLI[CLI archive argument and options] --> Shared[Domain selector fields]
  Shared -->|Invalid| CLIError[CLI VALIDATION_ERROR]
  Shared -->|Valid| Provider[Provider call]
  Raw[Raw provider call] --> Provider
  Provider --> Parser[Shared typed input parser]
  Parser -->|Invalid| InputError[AnalysisInputError]
  InputError --> Translation[Artifact error translation preserves typed error]
  Parser -->|Valid| Read[Existing file selection and bounded read]
Loading

Reviews (1) · Last reviewed commit: "fix(apple): validate keyed archive selec..." · Reviewed by Greptile

@gensokyo-dev
gensokyo-dev merged commit 6a226bd into main Oct 28, 2026
40 of 42 checks passed
@gensokyo-dev
gensokyo-dev deleted the fix/7649-keyed-archive-empty-selector branch October 28, 2026 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Empty archive member is classified as an I/O failure instead of invalid input

1 participant