feat(cli): add an optional per-job scope to the server job core - #1890
feat(cli): add an optional per-job scope to the server job core#1890viswa-uipath wants to merge 1 commit into
Conversation
github-actions
Bot
added
test:uipath-langchain
There was a problem hiding this comment.
🟡 Changes recommended
There are type-checking and cancellation/cleanup robustness issues in the new test annotations and the async scope teardown that should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds an optional, fail-open per-job async scope hook to the CLI server job core (_run_command_isolated) so external concerns can reliably bracket the exact window a job runs under its applied env/cwd, and backs it with targeted unit tests.
Changes:
- Introduce
register_job_scope_provider()and an internal_job_scope()async context manager to wrap job execution inside the per-job lock after env/cwd are applied. - Add a focused test suite covering ordering, env/cwd visibility, and fail-open behavior on scope enter/exit failures.
- Bump package version to
2.14.14(pyproject + lock).
File summaries
| File | Description |
|---|---|
| packages/uipath/src/uipath/_cli/_server_core.py | Adds optional per-job scope provider and wraps job execution in _job_scope(). |
| packages/uipath/tests/cli/test_server_job_core.py | Adds coverage for the new scope hook contract and ordering/visibility guarantees. |
| packages/uipath/pyproject.toml | Version bump to 2.14.14. |
| packages/uipath/uv.lock | Lockfile update reflecting the version bump. |
Review details
Suppressed comments (5)
packages/uipath/tests/cli/test_server_job_core.py:152
- Bare
listtypes are disallowed by this package’s mypy settings (disallow_any_generics = true) and will fail type-checking. Parameterize the list type here.
events: list = []
packages/uipath/tests/cli/test_server_job_core.py:175
- Bare
listtypes are disallowed by this package’s mypy settings (disallow_any_generics = true) and will fail type-checking. Parameterize the list type here.
seen: list = []
packages/uipath/tests/cli/test_server_job_core.py:197
- Bare
listtypes are disallowed by this package’s mypy settings (disallow_any_generics = true) and will fail type-checking. Parameterize the list type here.
order: list = []
packages/uipath/tests/cli/test_server_job_core.py:220
- Bare
listtypes are disallowed by this package’s mypy settings (disallow_any_generics = true) and will fail type-checking. Parameterize the list type here.
order: list = []
packages/uipath/tests/cli/test_server_job_core.py:285
- Bare
listtypes are disallowed by this package’s mypy settings (disallow_any_generics = true) and will fail type-checking. Parameterize the list type here.
events: list = []
- Files reviewed: 3/4 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
d032f91 to
56cba7f
Compare
🚨 Heads up:
|
The pre-warmed server runs every job through `_run_command_isolated`, which applies the job's env and cwd, runs the command, and restores the baseline. There was no way for a concern outside this module to bracket that window. Add `register_job_scope_provider`: an optional async context manager entered inside the per-job lock, after env/cwd are applied and before the command runs, so the scope observes the same environment and cwd the job does. It is best-effort — a provider that raises on entry or exit is logged and the job runs unaffected — and its teardown is shielded, so a cancel arriving while the job unwinds cannot leave the scope half torn down. With no provider registered the job path is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
c0619c3 to
30670b8
Compare
|
What
The pre-warmed server runs every job through
_run_command_isolated, which applies the job's env and cwd, runs the command, then restores the baseline. Nothing outside that module could bracket the window in which a job actually runs.This adds
register_job_scope_provider(provider): an optional async context manager entered inside the per-job lock, after env/cwd are applied and before the command runs, and exited before that state is restored — so the scope observes the same environment and cwd the job itself sees.With no provider registered (the default) the job path is byte-for-byte unchanged.
Contract
Noneclears it._run_command_isolated(cli_server.py,cli_server_ipc.py).Tests
Eight new cases in
tests/cli/test_server_job_core.py: no-provider path, env/cwd visibility inside the scope, teardown ordering against the env restore, enter → command → exit ordering, exit on the command's failure path, both fail-open paths, and deregistration.Known limitations (follow-ups, not addressed here)
asyncio.to_threadis not cancellable, so a cancelled job unwinds the scope (and the env/cwd restore) while the job thread is still running. Pre-existing for the env restore; the scope inherits it.__aexit__is always called with(None, None, None), so a provider cannot observe that the job failed or was cancelled.threading.localdoes not.🤖 Generated with Claude Code