Add 3 new MCP tools for inspecting YARN applications NOT submitted
through this service: get_external_job_logs, get_external_job_status,
get_external_job_result. Each takes application_id + connection_name
and queries YARN directly, bypassing the local JobStore.
- spark_executor/tools/external_jobs.py: 3 tool functions
- spark_executor/tools/requests.py: 3 new Pydantic body models
(ExternalJobLogsRequest, ExternalJobStatusRequest,
ExternalJobResultRequest)
- spark_executor/server.py: 3 new POST routes with explicit operation_id
- tests/unit/test_external_jobs.py: 7 unit tests
- tests/integration/test_mcp_routes.py: assert 20 tool routes
- README.md: list the 3 new tools
To make the LLM pick the right tool and not guess at field values,
also:
- Add Pydantic field descriptions for 22 fields across 8 request models
(SaveConnectionRequest, UpdatePendingJobRequest, GetJobLogsRequest,
JobIdRequest, PendingIdRequest, ConnectionNameRequest, plus the new
ExternalJob*Request models).
- Update 12 route descriptions with cross-references, prerequisite
context, and 400 behavior notes.
- Refactor _unknown_job_error: an input that looks like a YARN
application_id (starts with 'application_') now returns HTTP 400
(ValueError) with a hint message naming the right external tool;
other not-found cases still return 404 (KeyError). This catches the
common LLM mistake of passing application_id to the internal
get_job_* / kill_job tools.
- 4 new unit tests for the 400 behavior.
Tests: 356 passed (up from 242).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- main.py: drops the hard-coded McpService class and MCP_SERVICES list.
Lifespan now calls discover_and_filter() which does import + env
filtering. Unset MCP_SERVICES reproduces today's behavior exactly.
- spark_executor/service.py: new file exporting SERVICE = McpService(
name='spark_executor', app=spark_executor.app, mount_path='/spark-executor-mcp').
- files_mcp/service.py: new file exporting SERVICE = McpService(
name='files', app=files_mcp.app, mount_path='/files-mcp').
The McpService dataclass itself moved to common/mcp_service.py in the
prior commit. 327 existing tests pass unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Audit of the previous 'salvage application_id from stderr' change
(e4fb97c) revealed the fix was incomplete:
- The pending record now had pending.application_id set on failure.
- But get_job_logs / get_job_status / kill_job all look up the
application_id through JobStore.get_either(application_id), which
scans the on-disk jobs.json for a Job record matching that
application_id. No Job record was being created on the failure
path, so the recovered application_id was unreachable through
the public tool surface — get_job_logs would 404 with 'No Job
found for id=...application_xxx'.
Fix: when confirm_submit_job's failure branch successfully extracts
an application_id from stderr, it now also creates a Job record (same
shape as the success path: 12-char hex job_id, application_id,
script_path, queue, submit_time, connection, yarn_rm_url). The Job's
only job in this case is to act as the lookup index from the
LLM-facing tools into the underlying YARN app.
The job_id is generated even when application_id recovery fails (so
we always set pending.job_id), but the Job record is only put when
application_id is non-None. That keeps the 'spark-submit died locally
before reaching YARN' case clean: no Job record, get_job_logs gets
the standard 'no logs available' error from yarn_client, not a
404 from JobStore.
Tests:
- test_confirm_recovers_application_id_from_failed_spark_submit now
asserts both that pending.application_id is set AND that a Job
record exists and is findable by both job_id AND application_id.
- test_confirm_failed_spark_submit_without_application_id_keeps_it_none
asserts the negative case: no application_id, no Job record.
Fixture fix: _fresh() now rebinds submit.job_store to use tmp_path.
Previously the test wrote to ./data/jobs.json (the real data dir),
which left stale records between test runs and broke any test that
scanned by application_id. This is a latent bug exposed by the new
assertions; fixing it here also makes every other confirm test
hermetic.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Spark-submit can fail AFTER YARN has already accepted the application
(lost RM connection, PySpark script exited non-zero on the cluster
side, auth expired mid-submit, ...). In those cases the stderr still
contains 'Submitted application <id>', but the previous
confirm_submit_job implementation threw away that signal and recorded
the pending as FAILED with no application_id. The user had no way to
fetch the YARN logs of the failed job.
Fix:
- run_spark_submit now attaches the failed CompletedProcess to the
SparkSubmitError as .result, so callers can still inspect stderr.
- confirm_submit_job's failure branch tries to parse
(application_id, tracking_url) from exc.result.stderr. If found,
it writes them onto the pending record before re-raising. The
status is still FAILED and the error message is still set — the
user sees the failure normally, but the pending now also has a
log target they can pass to get_application_logs.
- If the stderr has no application_id (e.g. spark-submit failed
locally before reaching YARN), the parse attempt returns None and
the pending is FAILED with no log target — exactly the previous
behavior in that case.
Tests:
- test_confirm_marks_failed_on_spark_submit_error: existing test
now also asserts application_id is None (no result attached).
- test_confirm_recovers_application_id_from_failed_spark_submit:
failed run with 'Submitted application X' in stderr → pending is
FAILED + application_id is set.
- test_confirm_failed_spark_submit_without_application_id_keeps_it_none:
failed run with no application_id in stderr → pending is FAILED
+ application_id is None.
Full suite 248 passed (+2 from 246).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
write_job_file now validates that the incoming code string is encodable
as UTF-8 BEFORE doing any file I/O. A lone surrogate (U+D800..U+DFFF) —
typically the result of a broken decode somewhere upstream in the
agent's pipeline — cannot be represented in UTF-8, and would otherwise
either silently produce mojibake on disk or crash with an opaque
UnicodeEncodeError from open().
Behavior:
- try: code.encode('utf-8')
- on UnicodeEncodeError: log a warning and raise ValueError with a
message that points the agent at 're-generate the code as plain
Unicode'. ValueError -> HTTP 400 via the existing server.py
exception handler, so the agent gets a clean error and no file is
written.
- normal CJK / emoji / accented Latin still work — the encode check
is essentially free.
Tests:
- test_write_accepts_valid_utf8_including_cjk_and_emoji: positive
case for the common LLM output patterns.
- test_write_rejects_lone_surrogate: negative case, also asserts
that tmp_path is empty (no half-written .py left behind).
Full suite 246 passed (+2 from 244).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
confirm_submit_job no longer retries on SparkSubmitError. A failed
single attempt is recorded as-is:
- pending.status = 'FAILED' with the error message in pending.error
- the function re-raises the SparkSubmitError so the caller (and the
agent) sees the failure immediately
- the agent can call get_pending_job to see the persisted FAILED
state, including the captured error
The 'retry' code path was hiding real failures behind transient
recovery: a spark-submit that died because the script is broken looks
the same as one that died because YARN was momentarily unreachable.
Without retries, the agent gets a clean FAILED state and a clear
exception to act on, instead of a delayed, ambiguous result.
Manual recovery is still possible: re-calling confirm_submit_job on a
FAILED pending resets it to PENDING and runs a single fresh attempt
(see test_confirm_resets_failed_then_succeeds). The reset path is
useful for 'I fixed the script, try again' flows; it just is no longer
the default on every failure.
Removed:
- the for-attempt loop and time.sleep in submit.py
- import time in submit.py and test_submit_tool.py
- confirm_max_retries and confirm_retry_delay_seconds from
common/config.py (env vars, snapshot, reload)
- 2 retry tests (test_confirm_retries_spark_submit_failure_then_succeeds,
test_confirm_exhausts_retries_and_sets_failed)
- test_confirm_marks_failed_on_spark_submit_error updated to assert
the new single-attempt + raise contract
- test_confirm_resets_failed_and_retries renamed to
test_confirm_resets_failed_then_succeeds (no more retry loop to
exercise)
Full suite 244 passed (3 fewer than before, matching the removed
tests).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
YARN's 307 Location header is always a fully-qualified absolute URL.
The previous implementation used urllib.parse.urljoin to handle the
hypothetical case of a relative Location, but that's a defensive
codepath for a scenario that does not occur in practice. Drop it:
- Remove 'from urllib.parse import urljoin'.
- _request_following_redirects now assigns the Location value
straight to the next request URL.
- Update the docstring to reflect the new contract.
- Remove the test that exercised the relative-Location path.
Full suite 246 passed (1 fewer than the previous commit, matching the
test that was removed).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
httpx follows redirects by default, but it does NOT forward the
Authorization header across host boundaries. In YARN deployments where
the ResourceManager 307-redirects the NodeManager log fetch to a
different host (load balancer, Knox, NM selection), the follow-up
request lands unauthenticated and returns 401/403.
Replace the _request call in _fetch_logs_via_am_container with
_request_following_redirects, which:
- Walks the 3xx Location chain (up to 3 hops).
- Re-applies auth + verify on every hop.
- Resolves relative Location URLs against the current request URL.
- Raises YarnError on 3xx-without-Location (misconfigured server) and
on hop count overflow (redirect loop protection).
301/302/303/307/308 are all treated as 'follow the Location' per
RFC 7231 — the method/body handling for the rest is up to httpx when
we re-issue the request.
Tests: 4 new (cross-host 307, no-Location 307, two-hop chain, relative
Location). Full suite 247 passed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The previous commit (32ee167) added a regex-based HTML directory-listing
parser as a fallback when the NodeManager wraps /stdout in HTML (Knox,
wrapped vendor YARN builds). On review, the parser is too brittle to
trust in production:
- Apache-style listings vary across Hadoop distributions; some add
extra rows, icons, or anchors that change the regex hit set.
- Some clusters return the listing but 4xx each individual log file
(auth layer in the way), so the fallback silently returns empty.
- The behavior the agent observes for the same application_id becomes
environment-dependent in ways that are hard to reason about.
Revert to the simple, predictable path: GET /apps/{appid}, read
amContainerLogs, GET {amContainerLogs}/stdout, return its text. If the
cluster wraps /stdout in HTML, the agent gets a clear
'log-aggregation-enable' error and can ask the user to enable log
aggregation or use a different fetch mechanism — much easier to debug
than a half-parsed directory listing.
Removes:
- _looks_like_html, _parse_log_listing_html helpers
- The bare-URL fallback path and its try/except wrapper
- 4 unit tests that exercised the parser
Full suite back to 243 passed (matches fd0036c).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Some YARN deployments (Knox-proxied, wrapped vendor builds) return an
HTML directory listing page for /node/containerlogs/.../stdout instead of
the raw log bytes. The previous fix assumed a clean text/plain response
and failed every log fetch on those clusters.
Two new private helpers in yarn_client.py:
- _looks_like_html(text): cheap prolog sniff (<!doctype html, <html,
<?xml) on the first ~200 chars.
- _parse_log_listing_html(html): extract plain log-file names via a
href="..." regex; drop ../, absolute paths, sort toggles (?C=N;O=D),
and absolute URLs.
_fetch_logs_via_am_container now does:
1. GET /apps/{appid} (unchanged).
2. Read app.amContainerLogs.
3. Fast path: GET {amContainerLogs}/stdout. If 2xx and NOT HTML, return.
4. Fallback: GET {amContainerLogs} (bare URL), parse the HTML listing,
fetch each log file individually, and concatenate with === filename ===
headers in document order.
5. Raise _logs_unavailable_error if both paths yield nothing.
Tests: 3 new (test_logs_falls_back_to_directory_listing_when_stdout_returns_html,
test_logs_html_filter_strips_sort_links_and_parent_dir,
test_logs_html_filter_handles_prelaunch_files,
test_logs_raises_when_directory_listing_empty) — full suite 247 passed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The previous _fetch_logs_via_nodemanager walked
/apps/{appid}/appattempts -> /apps/{appid}/appattempts/{id}/containers,
but YARN ResourceManager does not expose the second hop. Container
info lives on NodeManager and is not reachable from RM REST, so the
fallback returned _logs_unavailable_error on every cluster without
/aggregated-logs (Hadoop 2.x, log aggregation disabled).
Replace the walk with: GET /apps/{appid} -> read app.amContainerLogs
-> GET {amContainerLogs}/stdout. The URL is provided directly by the
app response, so no container enumeration is needed.
Returns AM (driver) container logs only. Executor container logs
still require yarn.log-aggregation-enable=true (primary /aggregated-logs
path); the existing _logs_unavailable_error message already says that,
so the contract is preserved.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The MCP tool was named `generate_job_file` from Stage 2 but it does
NOT generate PySpark code — the calling LLM writes the code in its own
context, and this tool only persists it to a file under
SPARK_EXECUTOR_JOBS_DIR so `spark-submit` can see it. The misleading
`generate_` prefix sent agents (and humans) looking for a code
generator that doesn't exist.
This commit folds three related polish changes into one (split later
with rebase -i if you want them as separate history):
1. The rename itself:
- `tools/generate.py` → `tools/write_job.py`
- `generate_job_file` → `write_job_file`
- `GenerateJobFileRequest` → `WriteJobFileRequest`
- `/generate_job_file` route → `/write_job_file`
- `operation_id="generate_job_file"` → `operation_id="write_job_file"`
The internal helper `core.job_writer.write_job_file` (which just
writes bytes to disk with no SQL guard) is imported with an
`_write_to_disk` alias to avoid the name collision with the
MCP-exposed function in the same module.
The description for the tool now explicitly states 'this tool
does NOT generate PySpark code. The calling LLM is expected to
have already written the code; this tool only persists it.'
2. Skill for LLM agents operating the service
(`docs/superpowers/skills/spark-executor-mcp-operate/SKILL.md`,
449 lines). Covers the 16 tools, the two-step prepare/confirm
flow, the dual-ID contract (job_id vs application_id), the
PendingSubmission state machine, the Connection profile, the
job-file workflow, the error reference, common pitfalls, and a
full end-to-end word-count example.
3. Default `executor_memory` lowered 4G → 2G
(`_DEFAULTS_TO_CONFIRM` in `server.py`). Mirrors the matching
change in `test_mcp_routes.py` and the 5 unit tests that
reference the default. Aligns with the lighter workloads the
service is sized for in its current container profile.
Also tracked in git for the first time:
- `docs/superpowers/plans/2026-06-24-spark-executor-mcp.md`
(the original Stage 1/2/3 design plan, updated to use the new
tool name throughout).
Test rename:
- `tests/unit/test_generate_tool.py` → `test_write_job_tool.py`
- the new test file picks up an extra assertion that the SQL guard
rejects a `DROP TABLE` statement at write time.
243 tests pass (was 242; +1 new SQL-guard assertion). Zero regressions.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two user-reported bugs, same root cause: the in-memory JobStore + the
'job_id must be the 12-char hex' tool contract.
Bug 1: 'Unknown job_id' reported frequently
JobStore was a process-local dict (spark_executor/core/job_store.py).
Under gunicorn workers > 1, a job created by confirm_submit_job
landing on worker A was invisible to worker B, so a follow-up
get_job_status / get_job_result / get_job_logs / kill_job landing on
a different worker returned 'Unknown job_id'. Same multi-worker
problem that bit the MCP session layer; only the affected data was
different.
Bug 2: 'get_job_logs frequently confuses job_id and application_id'
confirm_submit_job returns BOTH identifiers in SubmitResult, but
get_job_logs (and friends) only accepted the local 12-char job_id
and never said so in their description. When the agent passed the
YARN application_id, the error message itself was misleading:
'Unknown job_id: application_17400000001_0001' — the agent had
passed an id, just the wrong kind.
This change fixes both at the root:
* JobStore is now JSON-backed at data/jobs.json (atomic tempfile +
os.replace), with cross-process safety via fcntl.flock on a sibling
.lock file. Stage 3's SQLite migration is still planned; the file
format is intentionally simple so it is a straight
'for j in read_all(): db.insert(j)'.
* New JobStore.get_either(uid) looks up by job_id first, then
application_id. All four job-lifecycle tools (get_job_status,
get_job_result, get_job_logs, kill_job) call get_either instead
of get(job_id), so the agent can pass either identifier and get
the same answer.
* The 'neither matched' KeyError now spells out both id forms and
what they look like, so the agent isn't left guessing.
* server.py tool descriptions for the four job tools explicitly
state 'job_id accepts BOTH identifiers' so this is visible to the
LLM at tool-selection time, not only at error time.
Tests:
* test_job_store.py: tmp_path isolation, persistence across
instances, human-readable JSON, corrupt-file resilience,
get_either (by job_id, by application_id, collision preference,
unknown), put idempotency.
* test_{logs,status,kill,result}_tool.py: per-test tmp_path fixture,
'accepts application_id' regression for each tool, and an
explicit assertion that the unknown-id error message mentions
BOTH id forms. test_result_raises_keyerror_for_unknown_job's
match pattern updated for the new message.
242 tests pass (was 226; +16 new). Zero regressions.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fastapi-mcp uses each route's OpenAPI operationId as the MCP tool name
exposed to LLM clients (fastapi_mcp/server.py:619-620). When the
operation_id is omitted, FastAPI auto-generates a name like
`prepare_submit_job_prepare_submit_job_post` (function name + path +
method), which is verbose and ugly for the agent.
This change adds an explicit `operation_id=` to all 17 MCP tool
routes in spark_executor/server.py, named after the handler function
(stripping the leading underscore). Concretely, the LLM client now sees
tool names like `prepare_submit_job`, `confirm_submit_job`,
`get_job_status`, etc. — same as the route paths.
`/health` (the only non-MCP GET route) is left untouched; its
auto-generated name (`health_check_health_get`) is never exposed.
Also adds a regression test
`test_every_mcp_route_has_explicit_clean_operation_id` in
tests/integration/test_mcp_routes.py that:
* fetches the live OpenAPI spec,
* asserts every MCP tool route has an operationId,
* rejects FastAPI's auto-generated format (e.g. operationId ending
in `_post` / `_get`),
* asserts no two routes share an operationId (fastapi-mcp keys
its tool registry by name, so duplicates would shadow silently).
All 226 tests pass.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add an update_pending_job MCP tool that lets callers modify parameters
of a PENDING submission before confirm_submit_job:
- queue, executor_memory, executor_cores, num_executors
- app_name
- extra_args
- script_path (re-validates file existence and SQL guard)
Only PENDING submissions can be updated; SUBMITTED/CANCELLED/FAILED are
rejected with HTTP 400.
Older pending records (created before app_name was required) may not
have the field. If such a record exists in the date-sharded pending
store, any operation that loads the store fails Pydantic validation and
blocks prepare/list/get/cancel.
Make app_name optional with default None on the persistence model while
keeping it required in PrepareSubmitJobRequest, so new submissions still
must provide it but old records can be loaded and cancelled.
Harden confirm_submit_job for unreliable test environments and transient
spark-submit failures:
- Add confirm_max_retries and confirm_retry_delay_seconds settings
(env-configurable).
- SUBMITTED pending returns cached result without re-running spark-submit.
- FAILED pending is reset to PENDING and retried.
- CANCELLED pending is still rejected.
- On success, persist pending as SUBMITTED before creating the in-memory Job
so a job_store failure cannot leave the record as PENDING while the YARN
app is already running.
- Persist tracking_url on PendingSubmission for idempotent returns.
Tests cover retry-then-success, max-retries-exceeded, idempotency,
FAILED reset, and pending-saved-before-job-store.
Restore defaults for queue/executor_memory/executor_cores/num_executors
in prepare_submit_job, but require the caller to explicitly confirm
them. If any defaulted field is omitted, the route returns HTTP 400
listing the defaults and asks the caller to resubmit with explicit
values.
app_name remains required (no meaningful default). extra_args remains
optional.
Tests cover rejection of unconfirmed defaults and acceptance of
explicitly confirmed defaults.
Remove defaults from queue, executor_memory, executor_cores,
num_executors, and app_name in prepare_submit_job. All must now be
explicitly confirmed by the caller.
Also add extra_args to the PendingSubmission snapshot so users can
confirm non-conf spark-submit flags (e.g. --jars, --py-files) at
prepare time; confirm_submit_job passes them through to
build_spark_submit_command.
This is an intentional breaking change to the MCP tool contract:
callers can no longer rely on implicit defaults.
- Add optional app_name to PendingSubmission and prepare_submit_job,
passed through PrepareSubmitJobRequest for user-provided tracking.
- Rewrite PendingStore to shard records by UTC date under
data/pending_jobs/YYYY-MM-DD.json instead of a single monolithic
pending_jobs.json.
- Legacy single-file pending_jobs.json remains readable; first save()
migrates it and renames the old file to avoid double reads.
- Tests cover date sharding, multi-date list, legacy read, legacy
migration, and app_name round-trip.
YARN ResourceManager REST can return either JSON or XML depending on
the Accept / Content-Type headers. We always parse responses with
resp.json(), so an ambiguous or XML response would break the client.
Add Accept: application/json to every _request call so the response
format is pinned unambiguously. This is compatible with both Hadoop
2.x/CDH 5 and Hadoop 3.x RM endpoints.
Tests: assert that httpx.request receives headers={"Accept": "application/json"}.
Add per-Connection ssl_verify / ssl_ca_bundle plus global defaults so
CDH 5 / on-prem clusters with self-signed certs or custom CA bundles can
be queried without patching code.
- Connection gets ssl_verify (bool|None) and ssl_ca_bundle (str|None)
- Settings gets ssl_verify_default and ssl_ca_bundle_default
- New YarnClientConfig dataclass carries the resolved verify= value
- _request passes verify= through to httpx.request
- All public yarn_client functions now take YarnClientConfig instead of
a bare yarn_rm_url string; tool call sites resolve the Connection
- SaveConnectionRequest exposes the two new fields
Tests cover per-connection CA bundle, per-connection verify=False,
global default fallback, and connection-not-found error.
The function had hardcoded --master / --deploy-mode / --queue /
--executor-memory / --executor-cores / --num-executors plus a
`spark_conf` dict for --conf, with no way to pass other flags like
--jars, --py-files, --files, --driver-memory, --name, etc.
Add `extra_args: dict[str, str] | None = None` that emits `--{key}
{value}` pairs, placed after the --conf loop and before the script
path. Default None preserves the existing cmd shape exactly.
Structured (dict) instead of raw list[str] so LLM agents writing
tool-call payloads can't accidentally pack flag+value into a single
string. Key order in the dict is preserved.
Tests: 4 cases — None default, single pair, multiple pairs in order,
no collision with spark_conf. uv run pytest -> 175 passed.
`get_job_status` returns YARN state + the raw response blob, so the
terminal fields (finalStatus, diagnostics, trackingUrl, startedTime,
finishedTime) are buried inside `raw` and not surfaced in a structured
form. Add a new tool that parses them.
`get_job_result(job_id)` reuses `yarn_client.get_application_status` and
extracts:
- finalStatus (SUCCEEDED / FAILED / KILLED / UNDEFINED)
- diagnostics (YARN final message)
- tracking_url (Spark Web UI)
- started_time / finished_time (epoch ms)
All five fields are optional: running jobs have no `finishedTime`, and
older YARN versions (CDH 5 / H2) may omit some fields. Missing fields
stay None — never raise.
Coexistence with `get_job_status` is intentional: the latter is for
polling the running YARN state, the former is the terminal view.
Tests: 4 cases — happy path, running job (no finishedTime), bare-minimum
raw (all optionals None), unknown job_id raises KeyError. uv run pytest
-> 171 passed.
`/ws/v1/cluster/apps/{id}/aggregated-logs` is a Hadoop 3+ endpoint; on
CDH 5 / Hadoop 2.6 it returns 404 and the old code surfaced a hard error
to the user. Add capability detection in `get_application_logs`: on 404 or
501 from the aggregated-logs endpoint, walk `appattempts` -> `containers`
-> NodeManager `/node/containerlogs/{id}/{user}/` and concatenate the
results.
5xx on the aggregated-logs endpoint still raises immediately (no fallback
attempted for genuine server errors). The H3 fast path is unchanged.
Constraints honored:
- No `subprocess` / `yarn` CLI. The whole point of yarn_client.py is to
avoid that dependency, so the fallback stays on `httpx` + REST.
- No new pyproject.toml / uv.lock dependencies.
- Public signature of `get_application_logs` unchanged; `tools/logs.py`
needs no changes.
- No version detection — pure capability detection (try, react to status).
Tests: replaced the old "raise on 404" test with 5 new cases (H3 fast
path, NM fallback, NM path empty, multiple containers, 5xx immediate
raise). `uv run pytest` -> 167 passed.
Hardcoding 'spark-submit' as cmd[0] in build_spark_submit_command
breaks for hosts where:
- both Spark 1.x and 2.x/3.x are installed and 'spark-submit' resolves
to the wrong one (use 'spark2-submit' or 'spark3-submit' explicitly)
- the user wants to launch via the PySpark entrypoint ('pyspark')
- a custom wrapper script sits on PATH (e.g. a credentials-injecting
'spark-submit-wrapper')
New env var SPARK_EXECUTOR_SPARK_SUBMIT_BIN. Default is 'spark-submit'
(preserves the current behavior for everyone). Override in .env /
docker-compose.yml to change.
common/config.py:
- new Settings.spark_submit_bin field
- env-var resolution in from_env() with default 'spark-submit'
- included in reload() so tests work
spark_executor/core/spark_submit.py:
- cmd[0] reads settings.spark_submit_bin (was hardcoded 'spark-submit')
.env.example: new section with the override and example values.
docker-compose.yml: forwards the var with the standard 'spark-submit'
default.
Tests: 2 new (settings.spark_submit_bin='spark2-submit', ='pyspark')
plus existing tests updated to use the settings-restore fixture so
mutations don't leak between tests.
165/163 still pass.
Closes the review-and-edit loop for LLM-generated PySpark code:
generate_job_file(code=...) -> {script_path}
read_job_file(script_path=...) -> {content, path, size}
update_job_file(path, content) -> {path, bytes_written}
prepare_submit_job(path) -> {pending_id, ...}
Or, in a single edit cycle:
1. generate (LLM writes initial draft)
2. read (LLM or human inspects)
3. update (overwrite with edited version)
4. prepare (submit for two-step confirmation)
Safety:
- read_job_file has no path restriction (read-only; useful for
inspecting any file the agent can see: scripts, logs/, hadoop-conf/)
- update_job_file is sandboxed to settings.jobs_dir (the same dir
generate_job_file writes to). Rejects paths outside that tree,
including ../-traversal attempts. This protects host-mounted
configs (/etc/passwd, hadoop-conf/*) from being overwritten by
the agent.
- 1 MB cap on both reads and writes so MCP responses stay bounded.
Pydantic body models (ReadJobFileRequest, UpdateJobFileRequest) follow
the Stage 1 pattern so tools/call roundtrips long code strings without
the FastAPI query-length 422.
Tests (13 new):
- 9 unit tests: read success/missing/empty/dir, update success/outside/
relative-escape/missing/oversize/1mb+1, full edit cycle round-trip
- 4 integration tests: read via MCP, missing file 400, write+readback
via MCP, outside-jobs_dir rejection via MCP
163/146 still pass. Live verified end-to-end: generate -> read v1
-> update -> read v2; update /etc/passwd correctly 400'd with
'script_path must be under ... data/jobs/'.
The agent can now write PySpark that runs DROP/DELETE/UPDATE/etc. on
production tables. Add a static guard that rejects anything other than
SELECT and INSERT at two enforcement points:
1. generate_job_file: validates BEFORE writing to disk. Agent gets
immediate feedback ('rewrite to use only SELECT/INSERT') rather
than learning at submit time.
2. prepare_submit_job: re-validates the script content (reads the
file) as a defense-in-depth check. Catches host-mounted files,
manually-edited files, anything that bypassed generate_job_file.
How it works:
- common/sql_guard.py extracts Python string literals whose first
keyword is a SQL verb (catches spark.sql('...'), f-strings, and any
raw SQL literal)
- sqlparse splits each literal into statements; we check the first
keyword against the policy (SELECT/INSERT/WITH allowed; DROP,
DELETE, UPDATE, TRUNCATE, ALTER, CREATE, REPLACE, MERGE, GRANT,
REVOKE, SET, SHOW, KILL, EXEC, etc. forbidden)
- WITH recurses into the CTE body to catch WITH x AS (DROP ...) ...
- The MCP layer maps ValueError -> HTTP 400 (existing handler)
Test coverage:
- 28 unit tests in test_sql_guard.py cover: extraction (single/double/
f-string, English false positives, multi-literal), statement
classification (SELECT, INSERT, DROP, DELETE, UPDATE, TRUNCATE,
ALTER, CREATE, multi-statement, CTE bodies, comments)
- 2 integration tests verify MCP layer returns 400 with the policy
explanation at both generate_job_file and prepare_submit_job
Limitations (documented in sql_guard.py docstring):
- f-strings where the SQL is built at runtime (e.g. f'SELECT * FROM
{user_input}') look like SELECTs at static-analysis time. The
guard catches the static literal; the runtime substitution is the
caller's responsibility.
- pyspark.sql.functions.expr('...') accepts SQL inline; not currently
caught. (Future work.)
146/146 still pass. Live verified: DROP TABLE -> MCP 400 with policy
explanation; SELECT -> MCP 200 + file written to ./data/jobs/.
Before: SPARK_EXECUTOR_DATA_DIR, SPARK_EXECUTOR_JOBS_DIR, and
YARN_RESOURCE_MANAGER_URL were each read directly via os.environ.get()
inside the module that used them. Log level was hardcoded DEBUG in
common/logging.py. There was no single file showing what the full set
of env vars the app reads is.
After: common/config.py defines a single Settings dataclass that
reads all env vars at import time and exposes them as fields on a
module-level singleton. App code uses "from common.config import
settings; settings.data_dir" etc. New SPARK_EXECUTOR_LOG_LEVEL env
var controls stderr + info file verbosity (debug file always gets full
DEBUG).
Improvements:
- One file lists every env var the app reads (was: grep the codebase)
- Tests can monkeypatch fields on the settings singleton directly
instead of monkeypatching the env + reloading
- Adding a new env var means adding one field in config.py, not
editing 3+ call sites
- settings.reload() method for tests that prefer env-var style
Out of scope (kept where they are):
- GUNICORN_* env vars live in gunicorn.conf.py (gunicorn concept)
- PYTHONUNBUFFERED in Dockerfile (Python runtime flag)
- SPARK_SUBMIT_OPTS not in config (JVM flag, not Python)
Test changes:
- test_job_writer.py: settings.jobs_dir instead of monkeypatching
SPARK_EXECUTOR_JOBS_DIR
- test_yarn_client.py: settings.yarn_resource_manager_url instead of
monkeypatching YARN_RESOURCE_MANAGER_URL
- test_generate_tool.py: same as job_writer
- Each test file gets an autouse fixture that snapshots+restores
settings so one test mutation does not leak into the next
116/116 still pass. Live verified: SPARK_EXECUTOR_LOG_LEVEL=INFO
suppresses DEBUG loguru output as expected.
Two problems with the prior prepare_submit_job flow:
1. The agent could pass a script_path that only existed in its own
context (LLM-generated code not yet on disk) or a path on the host
filesystem that's invisible inside the container. The new check
surfaces this as a 400 with a clear remediation hint instead of
letting spark-submit fail later with an opaque FileNotFoundError -> 500.
2. The container-isolation issue: any path the agent gives is interpreted
inside the container. The two ways a file can legitimately exist there
are (a) generate_job_file(code=...) just wrote it to
SPARK_EXECUTOR_JOBS_DIR (the default ./data/jobs/ is the only
gitignored dir that survives restarts), or (b) a host dir was
mounted via -v. The error message spells both out so an agent can
self-correct.
Implementation:
- submit.py: new _check_script_path() that raises ValueError (-> 400)
when the path is missing, empty, or a directory. Called in both
prepare_submit_job and confirm_submit_job (defense in depth).
- prepare_submit_job checks the connection FIRST (KeyError -> 404)
before the script (ValueError -> 400), so an agent with both problems
sees the more fundamental 'unknown connection' error first.
- server.py / requests.py: route description and Pydantic field
description spell out the generate_job_file pattern so an LLM
reading the tool schema learns the right next step.
8 new tests; 8 existing tests adjusted to create real files (they used
synthetic /tmp/*.py paths that don't exist).
Exposes a single new MCP tool: generate_job_file(code) -> {script_path}.
Wiring follows the Stage 1 conventions (Pydantic body model for the
route, loguru DEBUG/INFO logging, summary+description for the startup
log, MCP arg-pattern via the body model so tools/call roundtrips long
code strings without 422-ing on FastAPI query length limits).
Flow:
1. LLM calls generate_job_file(code=...) -> {script_path}
2. LLM calls prepare_submit_job(connection=...,
script_path=...) -> {pending_id}
3. User reviews the file + pending record
4. LLM calls confirm_submit_job(pending_id=...) -> spark-submit runs
The output directory is SPARK_EXECUTOR_JOBS_DIR (default ./data/jobs/,
gitignored, persists across container restarts via the existing
./data volume mount in docker-compose.yml).
3 new tests:
- Unit: env-var override, default fallback in tmp cwd
- Integration: end-to-end body call with a > FastAPI-query-limit code
string (the canary test that would have caught the Stage 1
query-params-422 bug)
Live MCP smoke verified: tools/list shows 14 tools (13 from Stage 1 +
the new one), tools/call generate_job_file returns the absolute path
under ./data/jobs/.
spark_executor/core/job_writer.py provides write_job_file(code, jobs_dir=None)
that resolves the output directory in this order:
1. explicit jobs_dir argument (for tests + programmatic override)
2. SPARK_EXECUTOR_JOBS_DIR environment variable (operator override)
3. DEFAULT_JOBS_DIR = './data/jobs' (gitignored, persists via volume mount)
Filenames are job_<UTC-stamp>_<random-hex>.py — same-second writes still
get distinct names via the random suffix, so concurrent agents won't
clobber each other.
8 tests cover: arg>env>default precedence, dir auto-creation, env-var
override, default fallback in a tmp cwd, and uniqueness under concurrent
calls.
YARN is the dominant use case, and the value of master for YARN is the
literal string 'yarn' (not a URL) — there is no useful variation. Making
it a default saves every YARN user from typing the same value every time:
save_connection(name='prod') # master defaults to 'yarn'
save_connection(name='dev', master='spark://10.0.0.5:7077') # override
Added a field_validator on master that catches common typos
('yarn-cluster', 'http://...', empty string) at save time with a clear
error message instead of letting them reach spark-submit.
Backward compatible: existing Connection records without an explicit
master field default to 'yarn' on load (Pydantic applies defaults on
deserialization), so already-saved data still works.
3 new tests cover the default, the validator, and that other cluster
managers (spark://, k8s://, local[N]) still pass.
main.py lifespan now enumerates every MCP tool with its description at
startup so operators can see at a glance what the server exposes.
The auto-generated '### Responses:' suffix that fastapi-mcp appends to
each tool description is truncated before logging to keep the startup
log scannable.
Docstrings added to all 12 route handlers in server.py so the MCP tool
descriptions are meaningful (without them, fastapi-mcp would emit empty
descriptions). Each route now carries a summary + description used by
the auto-generated OpenAPI schema.
yarn_client.py no longer invokes the 'yarn' binary via subprocess; it uses
httpx against /ws/v1/cluster/apps/* endpoints. This means the runtime image
no longer needs the Hadoop client installation — the only YARN-side
dependency left in the container is the config dir consumed by
spark-submit itself.
New model field:
- Job.yarn_rm_url: str | None
- PendingSubmission.yarn_rm_url: str | None (snapshotted at prepare)
prepare_submit_job snapshots Connection.yarn_rm_url into the pending
record (consistent with the existing master/deploy_mode/spark_conf
snapshot pattern); confirm_submit_job copies it onto the Job so
status/logs/kill can use it without re-looking-up the connection.
Resolution order for the RM URL at runtime:
1. Job.yarn_rm_url (preferred — survives connection edits/deletes)
2. Connection.yarn_rm_url fallback (if a future tool is added that
doesn't go through a Job)
3. YARN_RESOURCE_MANAGER_URL env var
Errors:
- YarnConfigError (HTTP 4xx semantics) when URL is missing/malformed
- YarnError for HTTP 4xx/5xx from the RM, network failures, missing
state field, or unparseable log responses
10 new tests in test_yarn_client.py cover the REST surface:
success, 404, 5xx, missing state field, env-var fallback, malformed
URL, log 404 with log-aggregation hint, kill PUT body shape, and
httpx connection-error wrapping.
- common/logging.py: improve format to timestamp|LEVEL|module:func:line - message
- core/ layer: DEBUG log every subprocess invocation (cmd, rc, byte counts),
JSON load/dump events, parsed application_id. ERROR log on failures.
- tools/ layer: DEBUG log every public tool entry with key parameters,
INFO log on business outcomes (saved/submitted/killed/...).
- New tests/unit/test_logging.py: capture loguru output via in-memory sink
and assert DEBUG + INFO messages are emitted for representative flows.
Tool functions raise KeyError for unknown ids and ValueError for invalid
state transitions. Without handlers, FastAPI returned a bare 500
"Internal Server Error" to MCP clients — useless for an agent that needs
to know whether the id was wrong vs the cluster was down.
KeyError -> 404 with the message ("Unknown job_id: foo").
ValueError -> 400 with the message (e.g. "pending_id p_xyz is in status
'CANCELLED', not PENDING").
The plan's route signatures used query parameters, which worked for direct
HTTP callers and unit tests, but fastapi-mcp's HTTP transport passes
tools/call arguments as a JSON body. dict-typed parameters like
spark_conf arrived as a string and the route returned 422.
Refactor each route to take a single Pydantic body model (saved in
spark_executor/tools/requests.py). Underlying tool functions unchanged.
Integration tests in tests/integration/test_mcp_routes.py now exercise
the full body-based roundtrip (save_connection with spark_conf, prepare
→ list → get, list_connections with empty body).
