fix(runtime): centralize ingest URI validation

[Galabavamsi]

Summary

Fixes #314.

• Add one shared parser returning DataRootRelativeUri.
• Apply the same trimming and safety checks in the CLI, workspace server, and AirflowClient.ingest().
• Pass normalized URIs through in-process execution, scheduled planning, and batch sizing.
• Make resolve_episode_reference() consume the refined value without repairing leading slashes.
• Reject POSIX and Windows-anchored/traversal paths while preserving safe internal segments such as a/../b.

Why

Previously, the server trimmed URI whitespace while the CLI and SDK forwarded it unchanged, and each entry point maintained its own validation logic. Scheduled planning also sent raw URI spellings through storage-key validation, so accepted internal path segments could fail before execution.

The shared boundary trims only surrounding whitespace, rejects blank/absolute/parent-escaping paths, and preserves the original safe URI spelling for trigger configuration and identity. Physical batch-size lookup uses the safely resolved relative path.

Validation

• uvx pre-commit run --all-files
• uv run --python 3.14 --locked ruff check --fix
• uv run --python 3.14 --locked ruff format
• uv run --python 3.14 --locked ty check
• Python 3.11: 1,401 passed, 6 skipped
• Python 3.14: 1,401 passed, 6 skipped
• Focused boundary/regression suite: 124 passed

Checklist

• Existing CLI exit code 2 and server HTTP 400 behavior preserved.
• SDK rejects invalid URIs before making HTTP requests.
• POSIX and Windows URI boundary cases are covered.
• Scheduled planning trims URIs and supports safe internal segments.
• No trigger configuration keys or response schemas changed.
• No generated artifacts, credentials, or recordings added.

[Copilot]

🟡 Changes recommended

The new parser/usage introduces correctness/API issues (notably backslash-containing URIs passing validation but not being resolvable consistently, and a return-type mismatch in outstanding_stage_uris()) that should be addressed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Centralizes validation/normalization of ingest URIs by introducing a shared parser that returns a refined DataRootRelativeUri, then wiring it through the CLI, workspace server, runtime client, and task-side planning/execution so trimming and safety checks are consistent end-to-end.

Changes:

• Added parse_data_root_relative_uri() and DataRootRelativeUri, and adopted it across CLI/server/SDK and task-side helpers.
• Updated stage planning/execution to validate and trim URIs consistently and to size batches using a safe normalized path for lookup.
• Extended/adjusted tests to cover trimming behavior, invalid-URI rejection, and safe internal segments (a/../b).

File summaries

File	Description
tests/test_stage_planning.py	Adds coverage asserting stage filtering returns trimmed URIs.
tests/test_stage_execution.py	Adds coverage for trimming + safe segment sizing and for rejecting leading slashes via the shared parser.
tests/test_runtime_client.py	Adds SDK-side tests ensuring invalid URIs are rejected pre-HTTP and trimming is applied in trigger conf.
tests/test_runtime_cli.py	Adjusts CLI ingest tests for trimming and adds blank-URI rejection coverage.
tests/test_ingest_in_process.py	Updates in-process ingest test to accept surrounding whitespace in URIs.
src/hflow/uri.py	Introduces the shared data-root-relative URI parser and refined type.
src/hflow/stage_planning.py	Uses the shared parser for outstanding-stage filtering and returns validated URIs.
src/hflow/stage_execution.py	Uses refined URIs for resolution and applies validation/normalization for batching and execution.
src/hflow/runtime/_client.py	Validates and trims URIs inside AirflowClient.ingest() before issuing HTTP requests.
src/hflow/runtime/init.py	Re-exports DataRootRelativeUri and parse_data_root_relative_uri from the runtime package.
src/hflow/cli.py	Replaces inline CLI ingest validation with the shared parser while preserving exit code behavior.
packages/hflow-server/tests/test_server_runtime.py	Updates server ingest test to cover trimming at the API boundary.
packages/hflow-server/src/hflow_server/_runtime.py	Replaces server-side inline validation with the shared parser and maps ValueError to HTTP 400.

Review details

• Files reviewed: 11/13 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Galabavamsi]

review requested and help what issues i can work on next which are not beginner level ? or can i raise my own findings and issues to continue forward?

[kstonekuan]

LGTM, merging.

I drove seventeen URI shapes through all three entry points on main and on this branch. The spread on main is wider than #314 describes:

shape                       main: sdk / cli / server        branch (all three)
'  x.mcap  '                conf keeps the spaces / ok / trimmed   trimmed
''  and  '   '              accept / accept / 400           refuse
'/abs/path.mcap'            accept / refuse / refuse        refuse
'../escape.mcap'            accept / refuse / refuse        refuse
'a/../../escape.mcap'       accept / refuse / refuse        refuse
'C:/windows/x.mcap'         accept / accept / accept        refuse
'C:x.mcap'                  accept / accept / accept        refuse
'dir\file.mcap'             accept / accept / accept        refuse
'a/../b.mcap'               accept everywhere               accept, spelling kept
'CC:/notadrive.mcap'        accept everywhere               accept

Three separate problems, not one. The SDK was the open door on every POSIX case. Blank strings were a genuine three-way disagreement. And the Windows shapes sailed through all three, which #314 did not know about.

C:x.mcap is the one I would have gone looking for. Drive-relative, no separator, and is_absolute() returns False for it, so PureWindowsPath(...).anchor is the predicate that catches it. That is the same trap #302 closed in storage.py, and getting it right here without being told is the detail that makes this a proper boundary rather than a moved if.

Keeping the candidate's own spelling and normalizing only the containment check is right, and test_batch_lane_trims_uri_and_sizes_safe_internal_segments covers the awkward consequence: a/../b.mcap stays the identity while the size lookup resolves it, because a storage key is containment-validated and would refuse the literal segments.

Four of the five refusals bite:

backslash separator removed  -> 2 failed
whitespace trim removed      -> 9 failed
parent-escape removed        -> 3 failed
SDK stops validating         -> 7 failed
windows anchor removed       -> 205 passed, nothing noticed

So the newest rule in the parser, the one main accepted everywhere, was the one nothing held. Pushed 2960906 adding the drive-anchored forms with forward slashes, which reach the anchor check rather than the separator check, and acceptance cases for CC:/notadrive/a.mcap and file:with:colons.mcap so the check cannot later be widened into refusing a legal directory named CC:. That mutation now fails 2 tests. Your checklist says the Windows cases are covered; the backslash half was, the anchor half was not.

Same commit drops the .replace("\\", "/") in the size lookup. The parser refuses any URI containing a backslash before it reaches there, so it could never fire; removing it changed nothing in the suite, which is how I confirmed it.

Gate clean: 1409 passed / 6 skipped, and the seventeen-shape behaviour is byte-identical before and after my fixup.

On your question: yes to both, and the second one more than the first. Every issue I have filed in the last two days came out of someone else's PR (#336 from rakesh0x's find, #346 and #355 from Sagar-024's, #351 from ravik453's), so raising your own findings is the normal path here, not a special case. Open advanced work, none of it assigned: #310, #311, #312, #320, and #303 / #304 / #305 for the bucket-backed shapes. #287 is the highest-leverage of the lot: real-corpus friction on Egocentric-10K/100K, filed from actually running the thing, and the kind of issue that generates several more.