Make file shares a first-class source type in the console (#196)

[richardmhope]

Closes #196.

The API has supported fileshare since #145, but the console's form had fields for Confluence and Jira only — so one whole connector was API-only to configure, which contradicts the console's "just another client of the same routes" positioning. #187 stopped that from 500ing; a graceful refusal is not support.

What it is not

It is deliberately not the Confluence form with different labels. A share has:

• no base URL — there is no site, only a mount path as the engine sees it;
• no Cloud-vs-DC choice — there is no HTTP auth to select;
• no credential box — the mount carries the host, the share name and the authentication (Add an SMB and NFS file-share connector #145). The form says so, rather than leaving a gap where the token field is on every other type. There is nothing here to store and nothing this console could rotate.

_connection_form returns early for the type, because FileshareConnection forbids extras and would reject a blob carrying base_url or include_comments — which every save posts, since all three scope blocks stay in the DOM so switching type does not discard typing.

Three details that decide whether it works

• base_url and credential become optional form parameters. The browser posts neither for a share, and a missing required Form() is FastAPI's own 422 — an error page, not the form the analyst is looking at.
• max_file_bytes is read as text and converted in the route, for the same reason: a mistyped ceiling re-renders the form with "must be a whole number of bytes".
• The connectivity-test button is not offered where no probe exists. probe_source already refused a share and explained why; the console showed a button that could only ever produce that refusal. PROBEABLE_TYPES is the API's own answer to "is there a check for this type", asked before offering one.

The chip helpers do not upper-case — roots and globs are paths, where case is significant, unlike a space or project key — and a leading slash is trimmed, since a root is relative to the mount and the API answers 422 for one. Better to take the obvious meaning than to teach the rule with an error.

The details card

Describes a share rather than a site: protocol, mount, roots, include/exclude, the ceiling, and the symlink policy — with Auth reading "carried by the mount, on the engine" rather than the Cloud/Server line, which is meaningless here.

Tests

Seven in apps/api/tests/test_web_screens.py:

• the type select is held against SUPPORTED_SOURCE_TYPES rather than a list repeated in the test — a type the API supports and the select omits is exactly the state this fixes, and it should fail the suite next time;
• a share-shaped blob is posted, with the hidden HTTP block's fields posted alongside and correctly ignored;
• a share is created with no credential at all — no 422, credential_ref is None, and the page explains the absence;
• editing a share saves it (the Fix what a full-codebase review turned up, in one cleanup pass #187 behaviour it replaces);
• no test button on a share, and still one on a Confluence source;
• the details card shows the mount and not <dt>Base URL</dt>;
• a mistyped ceiling re-renders rather than 500ing.

Two superseded tests: test_editing_a_source_the_form_cannot_express_is_refused_not_a_500 pinned the refusal this PR removes and is replaced by "editing a share saves it". test_a_hand_posted_type_without_a_form_is_refused_not_a_500 is rewritten as test_a_body_that_does_not_match_the_posted_type_is_refused_not_a_500 — with every supported type now on the form, the reachable failure is a hand-posted body that doesn't match its type, and that still must not 500. The unreachable else: branch in _connection_form stays, marked # pragma: no cover, as the landing spot for the next connector the API supports before the console catches up.

make check green: ruff, mypy, docs check, 1961 passed / 2 skipped. test_web_shell.py and test_web_invariants.py pass unchanged — the new partial carries no inline script or style, and all behaviour is in tags.js.

---

Generated by Claude Code

[icebergai-review-bot]

Verdict

CHANGES_REQUESTED

Completed bounded review across 1 immutable scope(s). 1 high-severity correctness finding.

Scope health

Convergence: healthy. Review mode: initial.
Recommended action: CONTINUE_INCREMENTAL.

• No escalation signals.

Prior findings

Finding	Status
—	No prior finding state

New findings

Root cause: The Alpine state omits the server-provided max-file-size value.

• BLOCKER · high: Fileshare forms discard the displayed maximum-file-size default — apps/api/src/iceberg_api/web/static/js/tags.js:165
  Status: NEW. Attribution: new_in_scope.
  Alpine initializes maxFileBytes to an empty string and never hydrates it from the existing connection. Because the input is bound with x-model, Alpine replaces the server-rendered value="33554432" (or the saved value) with that empty model value. Submitting an untouched create or edit form therefore sends an empty max_file_bytes, which _fileshare_connection converts to 0.
  Invariant: An untouched edit form must preserve the source configuration, and the displayed default must be the value submitted.
  Ownership: Fileshare source web form state and submission assembly.. Behaviour: Creating or editing a fileshare source from the console without manually re-entering the maximum file size..
  Evidence: tags.js defines maxFileBytes: '' and init() hydrates roots/include/exclude but not maxFileBytes; the fileshare input uses x-model="maxFileBytes". The route converts blank input with int(fields.max_file_bytes.strip() or 0). Tests post max_file_bytes explicitly, so they do not exercise the rendered form behavior.
  Independent assessment: Confirmed: the fileshare input binds x-model="maxFileBytes", while the component initializes it to '' and init() never loads connection.max_file_bytes. Alpine therefore writes the empty model value to the field; submission converts it to 0, and FileshareConnection requires max_file_bytes >= 1, so untouched fileshare create/edit saves fail validation.

Fix-induced regressions

• None evidenced.

Uncertainty

• No material uncertainty recorded.

Validation

• Exact-head CI passed per supplied review data; reviewed the supplied immutable diff.

Residual risks

• None identified.

[icebergai-review-bot]

Verdict

APPROVE

Completed bounded review across 1 immutable scope(s). No remaining findings. The prior fileshare form-state defect is fixed by hydrating Alpine with the persisted/default ceiling and rejecting an explicitly cleared value.

Scope health

Convergence: healthy. Review mode: delta.
Recommended action: CONTINUE_INCREMENTAL.

• No escalation signals.

Prior findings

Finding	Status
F-09C5C55BFC07	Resolved

New findings

No new findings.

Fix-induced regressions

• None evidenced.

Uncertainty

• No material uncertainty recorded.

Validation

• Prior blocker F-09C5C55BFC07 resolved: sources.py adds maxFileBytes from connection.max_file_bytes or DEFAULT_MAX_FILE_BYTES; tags.js hydrates this.maxFileBytes; regression tests cover new-form default, edit-form persistence, and blank-value rejection.
• Reviewed the supplied immutable follow-up diff and re-evaluated the prior blocker.

Residual risks

• None identified.