20 Commits
Author SHA1 Message Date
ClaudeandClaude Fable 5 8ededc50a9 feat(connection): add history_server_url config field
Add a new optional field to Connection for the Spark History Server
base URL. The field is stored as part of the connection but is not
yet consumed by any tool — for now it's a labeled place to record
where SHS lives on the cluster, and a hook for future SHS-specific
tools. To actually fetch SHS endpoints today, use fetch_url with the
SHS host added to url_allowlist.

The field is Optional[str], default None. Same nullability as
yarn_rm_url. The SHS host and the YARN RM host are usually
different, so this is independent of yarn_rm_url.

Schema:
  - models.py: Connection.history_server_url (str | None, default None)
  - requests.py:
    - SaveConnectionRequest.history_server_url (str | None, default None)
    - UpdateConnectionRequest.history_server_url (str | None, default None)
  - tools/connections.py: save_connection signature gains
    history_server_url with the _UNSET sentinel pattern (same as
    yarn_rm_url, url_allowlist, etc.) so the upsert path correctly
    distinguishes "not provided" from "explicitly None".
  - tools/connections.py: update_connection docstring lists the new
    field in the mutable fields set.

Tests (4 new in tests/unit/test_connection_tools.py):
  - test_save_connection_with_history_server_url
  - test_save_connection_history_server_url_defaults_to_none
  - test_update_connection_changes_history_server_url
  - test_update_connection_keeps_history_server_url_when_omitted

Tests: 398 passed (was 394, +4 net).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-09 16:24:05 +08:00
ClaudeandClaude Fable 5 f43e5b6403 docs(fetch_url): fix 3 misleading description bits
Three small but high-leverage text corrections. No code or test
changes — these only affect what the LLM sees in tools/list and
Pydantic schemas.

1) FetchUrlRequest.connection_name description
   Old: "The Connection's yarn_rm_url defines the allowed host domain."
   New: explicitly says url_allowlist is the host gate, NOT yarn_rm_url.
        yarn_rm_url is only used by get_external_* tools. Without this
        fix the LLM would try to control fetch scope via yarn_rm_url
        (a no-op) instead of url_allowlist.

2) Connection.url_allowlist description
   Added: "Set or change via save_connection (pass url_allowlist on
   create) or update_connection (PATCH the field on an existing
   connection)." Tells the LLM which tools populate the field,
   instead of leaving it to guess.

3) /fetch_url route description
   Old: "30s timeout, redirects followed."
   New: "30s timeout, redirects followed, response body capped at
        1 MB (the response includes a truncated boolean when this
        kicks in)." LLM previously had no way to discover the 1MB
        cap or the truncated indicator; it would just see
        short responses and assume that was the full body.

Tests: 394 passed, no test changes (descriptions are not asserted).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-09 16:18:29 +08:00
ClaudeandClaude 7d4e512cb0 fix: address review findings on fetch-url-tool
- fetch_url: revalidate allowlist on every redirect hop (fixes SSRF where
  302 to disallowed host / 169.254.169.254 / file:// bypassed the
  url_allowlist). Stream response body with iter_bytes and cap at 1MB
  so a multi-GB response from an allowlisted host cannot OOM the service.
  Reuses the manual-redirect-loop pattern from yarn_client.

- list_applications: stop swallowing 404 (YARN returns 200+empty for
  "no match"; 404 means the RM doesn't support the endpoint — surface
  the YarnError instead of hiding it as an empty result). Add
  Field(ge=1, le=10000) to ListApplicationsRequest.limit so a runaway
  limit is rejected at the Pydantic layer with 422.

- save_connection: PATCH semantics for existing records. Re-route to
  update_connection when the name already exists so partial updates
  (e.g. only master) no longer wipe url_allowlist back to []. Uses an
  _UNSET sentinel in the tool function to distinguish "omitted" from
  "None" without breaking the existing parameter list.

- README: drop leading space on 5 new connection-tool table rows that
  was breaking GitHub Flavored Markdown table continuity.

- Indentation: normalize connections.py and requests.py to 4-space
  indent (auth_password/auth_principal/auth_keytab were 3-space).

Co-Authored-By: Claude <noreply@anthropic.com>
2026-07-09 14:23:16 +08:00
ClaudeandClaude Fable 5 7fbad97a87 feat: add list_applications tool for YARN application enumeration
Add a new MCP tool that queries YARN's /ws/v1/cluster/apps endpoint
through a named Connection, returning a list of ApplicationSummary
records. Bypasses the local JobStore — useful for enumerating apps
that were not submitted through this service.

API:
  list_applications(
    connection_name: str,           # required, which YARN cluster
    state: str | None = None,       # YARN state filter: NEW/NEW_SAVING/
                                    # SUBMITTED/ACCEPTED/RUNNING/
                                    # FINISHED/FAILED/KILLED
    queue: str | None = None,       # YARN queue filter
    limit: int = 100,               # cap on returned apps (YARN has no
                                    # offset-based pagination; combine
                                    # state/queue filters for big clusters)
  ) -> list[ApplicationSummary]

Implementation:
  - yarn_client.list_applications(config, *, state, queue, limit) -> list[dict]
    Returns raw YARN app dicts; raises YarnError on 4xx/5xx; returns
    [] on 404 (no apps match). Uses the existing _request helper,
    which now accepts a "params" kwarg for query strings (one-line
    additive change).
  - external_jobs.list_applications(connection_name, state, queue, limit)
    -> list[ApplicationSummary]. Looks up the Connection, builds the
    YarnClientConfig, calls the yarn_client function, maps each raw
    YARN dict to ApplicationSummary (mirroring the manual field-mapping
    style of get_job_result). The yarn_client function is imported
    as "list_applications_yarn" to avoid name collision.
  - ApplicationSummary: 12-field Pydantic model with snake_case names
    (application_id, name, user, queue, state, final_status,
    application_type, application_tags, started_time, finished_time,
    tracking_url, progress). Unused YARN fields (memorySeconds,
    vcoreSeconds, preemptedResource*, etc.) are not exposed.
  - ListApplicationsRequest: Pydantic body model with Field(description=)
    for LLM-facing schema.
  - /list_applications route registered with operation_id=
    "list_applications", placed next to the other external YARN tools.

Tests:
  - 8 new unit tests in test_external_jobs.py (happy path, state/queue/
    limit pass-through, default limit, empty list, missing connection,
    full field mapping).
  - test_mcp_routes.py: assert 23 tool routes.
  - README: list_applications row added to the Spark Executor table.

Tests: 390 passed (was 382, +8 net).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-09 13:40:20 +08:00
ClaudeandClaude Fable 5 a5b9539663 refactor(fetch_url): trust the allowlist, rename to url_allowlist
Two changes:

1) Drop every check except the allowlist lookup.

  Old _validate_url_host did: scheme check, host-presence check,
  IP-literal check, empty-allowlist check, then glob match.

  New _validate_url_host does: parse host, return on glob match,
  raise on miss. That's it. The only remaining structural check is
  'the URL must have a host' (otherwise the glob has nothing to
  test against).

  Security implication: scheme (file://, gopher://, ftp://) and
  IP literals (10.0.0.1, ::1) are NO LONGER rejected by the
  validator. The allowlist is the single source of truth. If the
  user writes ['*.*.*.*'], they have opted in to 4-label hosts
  including IP literals; if they write ['ccam*'], they get ccam1-
  ccam99 and nothing else. The default ['ccam*'] / [] pattern is
  tight by construction.

  Removed: import ipaddress, the scheme/IP rejection branches, the
  'allowlist empty' explicit branch (the empty list naturally
  matches nothing).

2) Rename allowed_url_hosts -> url_allowlist.

  The previous name was a verbose double-negative ('allowed ... hosts').
  The new name is short, modern (allowlist > whitelist), and matches
  the pattern of the field (URL hosts allowed). Renamed in:
    - Connection (models.py)
    - SaveConnectionRequest, UpdateConnectionRequest, FetchUrlRequest
      (requests.py)
    - _validate_url_host, _host_matches_any_glob parameters
      (fetch_url.py)
    - save_connection / update_connection call sites and error
      messages (connections.py, fetch_url.py)
    - Route descriptions (server.py)
    - All test files
    - README

  No backward-compat alias: the field was added in 0291f36 and
  hasn't shipped, so no production migration. Local dev data
  (data/connections.json, gitignored) with allowed_url_hosts set
  will be silently dropped by Pydantic v2 (default for extra
  fields is ignore) — those connections lose their allowlist and
  fetch_url will reject everything until re-saved.

Test cleanup:
  - Removed 9 obsolete tests (scheme/IP/suffix rejection)
  - Renamed allowed_url_hosts -> url_allowlist in 11 surviving tests
  - Added 4 new tests documenting the 'allowlist is the only gate'
    model: IP literal accepted, HTTPS accepted, no-host rejected,
    empty allowlist rejected, error message mentions url_allowlist

  -1 obsolete test, net -4 from 386 -> 382 tests passing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-09 12:09:41 +08:00
ClaudeandClaude Fable 5 6d7387b022 feat: simplify fetch_url allowlist + add update_connection tool
Part 1 — fetch_url validation simplification
  Now that Connection.allowed_url_hosts exists, it's the ONLY check.
  The old 2-label suffix-overlap rule against yarn_rm_url is gone.

  - Connection.allowed_url_hosts: list[str] = Field(default_factory=list)
    (was list[str] | None = None). Default empty list means the
    connection has no fetch access; the user must explicitly opt in
    via save_connection or update_connection.
  - _validate_url_host drops the yarn_rm_url parameter and the
    _host_suffix_overlap helper. Now: scheme/host/IP checks, then
    empty-reject, then glob match. Reject everything else.
  - fetch_url's error message now points the user at
    Connection.allowed_url_hosts as the fix.
  - The label-by-label glob check from the previous commit is kept
    (SSRF guard: 'ccam*' matches 'ccam50' but NOT 'ccam50.evil.com').

Part 2 — update_connection tool
  PATCH-style update for an existing Connection record. Only the
  fields the caller provides are changed. Same pattern as
  update_pending_job: req.model_dump(exclude_none=True), with
  'name' popped before passing to the store.

  To CLEAR a field (e.g. drop auth_password), use delete_connection
  + save_connection. This is YAGNI; the alternative (model_fields_set
  to distinguish 'omitted' from 'null') adds surface for bugs.

  - ConnectionStore.update(name, **fields): get, model_copy(update=...),
    save. Lock + atomic write. Re-validates the patched record.
  - update_connection tool function: passes fields through to the
    store, logs which fields were changed.
  - UpdateConnectionRequest Pydantic model: 12 mutable fields + name.
  - /update_connection route with operation_id='update_connection'.
  - 9 new unit tests in test_connection_tools.py (PATCH, dict/list
    replace-not-merge, unknown name 404, validation of patched record,
    disk persistence).
  - test_fetch_url.py: existing 21 tests updated; 3 new tests for
    the empty/None/missing allowed_url_hosts cases.
  - test_mcp_routes.py: assert 22 tool routes.
  - README: update_connection row added; fetch_url row updated.

Tests: 386 passed (up from 377, +9 net).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-09 11:46:07 +08:00
ClaudeandClaude Fable 5 0291f36b01 feat(fetch_url): per-Connection allowed_url_hosts glob allowlist
The default 'URL host shares >= 2 labels of suffix with yarn_rm_url
host' rule is too strict for clusters whose hostnames are single-label
(e.g. 'ccam1' through 'ccam99'). The user's cluster is reachable at
http://ccam1:8088, and they want fetch_url to work for any ccamN
host — but ccam1 and ccam50 share 0 suffix labels, so the existing
rule rejects everything.

Add an opt-in allowlist field on Connection:

  allowed_url_hosts: list[str] | None

Each entry is an fnmatch glob pattern. The URL host is allowed if it
matches ANY pattern, regardless of the suffix rule. Save a Connection
with ['ccam*'] to allow ccam1, ccam2, ..., ccam99.

SSRF safety: the implementation does NOT use vanilla fnmatch on the
flat host string (because '*' in fnmatch crosses '.', so 'ccam*' would
match 'ccam50.evil.com' — a security hole). Instead, both the pattern
and the host are split on '.' and matched LABEL-BY-LABEL with the
label counts required to match exactly. So 'ccam*' matches 'ccam50'
but NOT 'ccam50.evil.com' (different label counts).

Test coverage:
- 8 new tests in tests/unit/test_fetch_url.py (glob allow, glob deny,
  dot-boundary SSRF test, multiple globs, empty list, None fallback,
  error message hint)
- All 21 fetch_url tests pass; 377 total.

- spark_executor/models.py: Connection.allowed_url_hosts (with description)
- spark_executor/tools/requests.py: SaveConnectionRequest.allowed_url_hosts
- spark_executor/tools/fetch_url.py: new _host_matches_any_glob helper;
  _validate_url_host now takes allowed_hosts and checks globs BEFORE
  the suffix rule
- tests/unit/test_fetch_url.py: 8 new tests
- README.md: fetch_url row mentions the allowlist

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-09 11:22:55 +08:00
ClaudeandClaude Fable 5 627e70f697 feat: add fetch_url tool for proxying HTTP GET to cluster-internal URLs
Add a new MCP tool that lets the agent fetch URLs on the cluster's
network (YARN tracking UI, Spark History Server, NodeManager web UIs)
when the agent is on a different network and cannot reach those hosts
directly.

The MCP service runs on the YARN RM node, so it can reach every host
the cluster knows about — the agent just needs a way to ask.

Security: SSRF guard via host suffix overlap
  - URL host must share >= 2 labels of suffix with the named
    Connection's yarn_rm_url host (e.g. yarn_rm_url='rm.prod.internal'
    allows 'http://nm01.prod.internal/...')
  - IP literals (10.0.0.1, ::1) rejected
  - Non-http(s) schemes (file://, gopher://, ftp://) rejected
  - Connection with no yarn_rm_url cannot use this tool
- Reuses Connection.auth_for_httpx() and verify_for_httpx() so the
  agent does not need cluster credentials
- Response body capped at 1 MB (truncated=true if larger)
- 30s timeout, follows redirects, loguru INFO audit log on every call

- spark_executor/tools/fetch_url.py: new tool + 2 helpers
  (_host_suffix_overlap, _validate_url_host)
- spark_executor/models.py: FetchUrlResult Pydantic model
- spark_executor/tools/requests.py: FetchUrlRequest with descriptions
- spark_executor/server.py: /fetch_url route, operation_id='fetch_url'
- tests/unit/test_fetch_url.py: 13 unit tests covering all guards,
  truncation, auth/SSL pass-through, redirect follow
- tests/integration/test_mcp_routes.py: assert 21 tool routes
- README.md: 1 row in Spark Executor 工具 table

Tests: 369 passed (up from 356).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-09 10:59:25 +08:00
ClaudeandClaude Fable 5 6cf68439a2 feat: add external job tools + improve LLM-facing tool descriptions
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>
2026-07-08 20:03:40 +08:00
ClaudeandClaude Fable 5 523e6a9c76 refactor(mcp): rename generate_job_file to write_job_file to match what it does
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>
2026-06-29 19:13:29 +08:00
Claude b8826f8c90 feat(submit): update_pending_job tool for editing PENDING submissions
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.
2026-06-26 16:14:08 +08:00
Claude f170c3045b feat(submit): confirm defaults instead of removing them
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.
2026-06-26 15:16:43 +08:00
Claude 4b07617af0 feat(submit): require explicit confirmation for all prepare_submit_job params
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.
2026-06-26 15:12:14 +08:00
Claude da9ba657f3 feat(pending): app_name field and date-sharded pending storage
- 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.
2026-06-26 14:36:24 +08:00
Claude 10f075ab7f feat(yarn_client): auth config for YARN REST (none/simple/basic/kerberos)
Add per-Connection authentication so CDH 5 / Kerberos / HTTP Basic
clusters can be queried.

- auth_type: none / simple / basic / kerberos
- auth_user / auth_password for HTTP Basic
- auth_principal / auth_keytab stored for audit/display; actual SPNEGO
  handled by httpx-kerberos using the system Kerberos credential cache
- YarnClientConfig.auth_for_httpx() returns the right httpx.Auth object
- _request passes auth= through to httpx.request alongside verify=

New dependency: httpx-kerberos.

Tests cover none/simple (no auth object), Basic auth header,
Kerberos auth object, missing basic user, invalid auth_type, and
tool-layer config propagation.
2026-06-26 13:57:57 +08:00
Claude ad557b5984 feat(yarn_client): SSL/TLS config for YARN REST connections
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.
2026-06-26 13:50:30 +08:00
Claude 35bff11edf feat: add read_job_file and update_job_file MCP tools
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/'.
2026-06-25 13:18:39 +08:00
Claude dd197019f9 fix: validate script_path exists + tell agent to call generate_job_file
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).
2026-06-25 10:41:19 +08:00
Claude 994c2d67ab feat: add generate_job_file MCP tool (Stage 2 Task 22)
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/.
2026-06-25 10:19:59 +08:00
Claude 6b9f278294 fix: switch FastAPI routes to Pydantic body models for MCP tools/call
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).
2026-06-24 14:56:31 +08:00