diff --git a/devtools/e2e_live/scenarios.py b/devtools/e2e_live/scenarios.py index 486a83a88..99d8db69d 100644 --- a/devtools/e2e_live/scenarios.py +++ b/devtools/e2e_live/scenarios.py @@ -475,8 +475,8 @@ def worktree_after_commit(clone: pathlib.Path) -> tuple[bool, str, list[str]]: def _git_show(clone: pathlib.Path, rev: str, path: str) -> str: """The exact text of ``path`` at ``rev`` ('' when absent there).""" - proc = subprocess.run(["git", "show", f"{rev}:{path}"], cwd=str(clone), check=False, capture_output=True, text=True) - return proc.stdout if proc.returncode == 0 else "" + proc = subprocess.run(["git", "show", f"{rev}:{path}"], cwd=str(clone), check=False, capture_output=True) + return proc.stdout.decode("utf-8") if proc.returncode == 0 else "" def release_carriers_desync_at(clone: pathlib.Path, rev: str) -> str: diff --git a/docs/architecture/03-web-ui-pages-and-buttons.md b/docs/architecture/03-web-ui-pages-and-buttons.md index 4aac0c9e6..5c8a4188f 100644 --- a/docs/architecture/03-web-ui-pages-and-buttons.md +++ b/docs/architecture/03-web-ui-pages-and-buttons.md @@ -281,7 +281,7 @@ The synchronous lock-owning apply executor publishes process-local stage observa Settings has Accounts, Secrets, Models, Agents, Behavior, Appearance, Advanced and About tabs — a sequence from connections to runtime detail. Accounts: managed subscriptions and their shared service banner, API providers, custom compatible endpoints, local runtime entry points, and the optional non-loopback network gate. Secrets: known provider/integration secrets, skill-requested keys and owner-defined custom keys, without returning stored values. Models: compact source/model/account role rows, ordered fallbacks, context assertions and effort lanes. Agents: task actors and review lanes, with delegation permissions, per-root and depth limits and subagent path roots; their accounts are managed in Accounts. Behavior: context, safety-supervisor coverage, task acceptance, self-evolution, prompt-cache posture. Appearance: the client-local theme choice described above, never a runtime-settings value. Advanced: process, timeout, local-model, integration, source-control and cleanup controls (worker count is process capacity, so it lives here). About reports application/runtime identity. Keys, defaults and per-key semantics are the §7 Default settings table, not this chapter. Models, Available subagents and Review lanes share ONE grouped source select owned by `web/modules/route_editor_primitives.js` (`routeChoiceGroups`, `configuredApiProviders`): the owner picks a source and the editor composes the stored id, so the provider prefixes (`provider::model`, `claudexor::source=model`, `harness=model`) are serialization only, never owner input. -`settings_catalog.js` owns model-catalog reads and the Accounts subscription: a changed confirmed account or recovery from a failed read refreshes discovery; identical settled status stays quiet, and `settings.js` arms/disposes the subscription with the page. Catalog updates preserve the owner's current model draft. +`settings_catalog.js` owns model-catalog reads and the Accounts subscription: a changed confirmed account or recovery from a failed read refreshes discovery; identical settled status stays quiet, and `settings.js` arms/disposes the subscription with the page. Catalog updates preserve drafts. Per-button request ownership releases busy state independently of global response freshness; a newer background read cannot strand a manual button. The Settings client validates the whole current draft before Save; a local error keeps every value available for correction and sends no partial save (`settings_controls.js` keeps custom-key collection pure and dirty reads passive). Ordinary refresh and failed writes preserve current edits, leaving or explicitly reloading a dirty draft asks first, the write response distinguishes saved, unsaved and unknown outcomes, and there is no durable cross-page draft store or secret persistence. diff --git a/ouroboros/commit_admission.py b/ouroboros/commit_admission.py index 0f4577d2b..a966ff6f1 100644 --- a/ouroboros/commit_admission.py +++ b/ouroboros/commit_admission.py @@ -39,8 +39,9 @@ def changed_worktree_paths( try: result = subprocess.run( ["git", "--no-optional-locks", "status", "--porcelain"] + path_args, - cwd=str(repo_dir), capture_output=True, text=True, timeout=10, + cwd=str(repo_dir), capture_output=True, timeout=10, ) + stdout = result.stdout.decode("utf-8") except Exception: if strict: raise @@ -49,7 +50,7 @@ def changed_worktree_paths( if strict: raise RuntimeError("git status failed") return [] - return parse_changed_paths_from_porcelain(result.stdout) + return parse_changed_paths_from_porcelain(stdout) def auto_sync_release_metadata_if_needed( @@ -104,9 +105,11 @@ def read_release_file(repo_dir, path: str, *, source: str) -> str | None: present.check_returncode() result = subprocess.run( ["git", "show", f":{path}"], cwd=str(repo_dir), capture_output=True, - encoding="utf-8", timeout=10, check=True, + timeout=10, check=True, ) - return result.stdout + # Decode on the caller thread (Windows pipe-reader errors otherwise disappear), + # retaining the universal-newline semantics of worktree read_text(). + return result.stdout.decode("utf-8").replace("\r\n", "\n").replace("\r", "\n") def release_metadata_diagnostics( @@ -133,9 +136,9 @@ def release_metadata_diagnostics( result = subprocess.run( ["git", "--no-optional-locks", "diff", "--cached", "--name-only", "--diff-filter=d", "-z"], cwd=str(repo_dir), - capture_output=True, encoding="utf-8", timeout=10, check=True, + capture_output=True, timeout=10, check=True, ) - touched.update(filter(None, result.stdout.split("\0"))) + touched.update(filter(None, result.stdout.decode("utf-8").split("\0"))) except Exception as exc: unavailable.append(f"Changed {source} paths could not be read ({type(exc).__name__}).") diff --git a/tests/test_release_metadata_diagnostics.py b/tests/test_release_metadata_diagnostics.py index 9cee659b6..c80e96235 100644 --- a/tests/test_release_metadata_diagnostics.py +++ b/tests/test_release_metadata_diagnostics.py @@ -74,6 +74,24 @@ def candidate(tmp_path, monkeypatch): emit_progress_fn=lambda *_: None) +def test_crlf_carriers_have_same_text_semantics_in_index_and_worktree(candidate): + repo = candidate.repo_dir + _git(repo, "config", "core.autocrlf", "false") + for name, text in _release("1.2.4").items(): + (repo / name).write_bytes(text.replace("\n", "\r\n").encode("utf-8")) + _git(repo, "add", ".") + for source in ("worktree", "index"): + result = admission.release_metadata_diagnostics(repo, ["VERSION"], source=source) + assert result["status"] == "clean", result + + +def test_unicode_worktree_discovery_is_not_locale_decoded(candidate): + repo = candidate.repo_dir + _git(repo, "config", "core.quotepath", "false") + (repo / "А.py").write_text("value = 1\n", encoding="utf-8") + assert "А.py" in admission.changed_worktree_paths(repo, strict=True) + + def _broken(repo): files = _release("1.2.4") files["README.md"] = _release()["README.md"] + "".join( @@ -208,8 +226,8 @@ def test_index_read_failure_and_unmerged_entries_are_unavailable(candidate): _git(repo, "add", ".") blob = _git(repo, "rev-parse", ":pyproject.toml") # Install an unmerged optional carrier in this disposable fixture index. - subprocess.run(["git", "update-index", "--index-info"], cwd=repo, check=True, text=True, - input=f"0 {'0' * 40}\tpyproject.toml\n100644 {blob} 1\tpyproject.toml\n100644 {blob} 2\tpyproject.toml\n") + subprocess.run(["git", "update-index", "--index-info"], cwd=repo, check=True, + input=(f"0 {'0' * 40}\tpyproject.toml\n100644 {blob} 1\tpyproject.toml\n100644 {blob} 2\tpyproject.toml\n").encode("utf-8")) report = _diagnose(candidate, "index") assert report["status"] == "unavailable" assert any("pyproject.toml" in item for item in report["unavailable"]) diff --git a/tests/test_ui_smoke_status_attention.py b/tests/test_ui_smoke_status_attention.py index 9fa645f49..e0e12e662 100644 --- a/tests/test_ui_smoke_status_attention.py +++ b/tests/test_ui_smoke_status_attention.py @@ -324,7 +324,9 @@ def test_task_status_stays_factual_in_main_and_project_chat( assert failed.get_attribute("data-finished") == "1" assert phase_text(failed) == "Failed" assert_phase_accessibility(failed, "Task", "Failed") - assert "provider_route_failed" not in failed.locator( + # This synthetic code has no producer phrase. Unknown reasons remain + # visible verbatim; hiding them would discard the only available cause. + assert "provider_route_failed" in failed.locator( ":scope > [data-live-summary-button]" ).inner_text() diff --git a/web/modules/settings_catalog.js b/web/modules/settings_catalog.js index 7d84020d3..9de2e4f4b 100644 --- a/web/modules/settings_catalog.js +++ b/web/modules/settings_catalog.js @@ -3,6 +3,7 @@ import { apiFetch } from './api_client.js'; import { setInlineStatus } from './ui_helpers.js'; export const MODEL_CATALOG_TIMEOUT_MS = 25000; let catalogRefreshSeq = 0; +const buttonRefreshes = new WeakMap(); // Account login/status is the authority for subscription model discovery. Keep // one small signature of the confirmed account facts so a newly settled login @@ -194,6 +195,7 @@ export async function refreshModelCatalog({ button } = {}) { const statusEl = document.getElementById('settings-model-catalog-status'); setCatalogStatus(statusEl, 'Refreshing model catalog...', 'muted'); if (button) { + buttonRefreshes.set(button, refreshSeq); button.disabled = true; button.setAttribute('aria-busy', 'true'); } @@ -241,7 +243,9 @@ export async function refreshModelCatalog({ button } = {}) { return { items: [], errors: [{ provider_id: 'catalog', error: String(message) }] }; } finally { clearTimeout(timeoutId); - if (button && refreshSeq === catalogRefreshSeq) { + // Global freshness governs data, not a particular button's busy lease. + if (button && buttonRefreshes.get(button) === refreshSeq) { + buttonRefreshes.delete(button); button.disabled = false; button.removeAttribute('aria-busy'); } diff --git a/web/tests/settings_action_row.test.js b/web/tests/settings_action_row.test.js index 01a1418a6..563acf425 100644 --- a/web/tests/settings_action_row.test.js +++ b/web/tests/settings_action_row.test.js @@ -38,5 +38,5 @@ test('async actions expose the same busy and status semantics', () => { assert.match(catalogJs, /import \{ setInlineStatus \} from '\.\/ui_helpers\.js'/); assert.match(catalogJs, /setInlineStatus\(statusEl, text, tone\)/); assert.match(catalogJs, /refreshModelCatalog\(\{ button \} = \{\}\)/); - assert.match(catalogJs, /refreshSeq === catalogRefreshSeq/); + assert.match(catalogJs, /buttonRefreshes\.get\(button\) === refreshSeq/); }); diff --git a/web/tests/settings_catalog.test.js b/web/tests/settings_catalog.test.js index d7eecf856..21c7feed6 100644 --- a/web/tests/settings_catalog.test.js +++ b/web/tests/settings_catalog.test.js @@ -136,6 +136,34 @@ test('an older Refresh completion cannot replace newer success or its read facts assert.deepEqual(events[0].items, first.items); }); +test('background refresh cannot strand a superseded manual button busy', async (t) => { + const previousDocument = globalThis.document; + const previousFetch = globalThis.fetch; + t.after(() => { globalThis.document = previousDocument; globalThis.fetch = previousFetch; }); + const document = new EventTarget(); + document.getElementById = () => null; + globalThis.document = document; + const button = { disabled: false, setAttribute() {}, removeAttribute() {} }; + const pending = []; + globalThis.fetch = () => new Promise((resolve) => pending.push(resolve)); + const manual = refreshModelCatalog({ button }); + const background = refreshModelCatalog(); + pending[1]({ ok: true, json: async () => first }); + await background; + assert.equal(button.disabled, true, 'manual request still owns its busy state'); + pending[0]({ ok: true, json: async () => first }); + assert.equal((await manual).stale, true); + assert.equal(button.disabled, false); + const old = refreshModelCatalog({ button }); + const latest = refreshModelCatalog({ button }); + pending[2]({ ok: true, json: async () => first }); + await old; + assert.equal(button.disabled, true, 'older request cannot release a newer button owner'); + pending[3]({ ok: true, json: async () => first }); + await latest; + assert.equal(button.disabled, false); +}); + test('read errors drop the httpx documentation pointer and rows get the compact form', () => { const data = { items: [{ value: 'openai/gpt-x' }],