fix: preserve Git text decoding and catalog button ownership

This commit is contained in:
Ouroboros 2026-09-23 16:26:35 +03:00
parent 33da7a4f3c
commit ccaae88702
8 changed files with 69 additions and 14 deletions

View file

@ -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:

View file

@ -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.

View file

@ -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__}).")

View file

@ -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"])

View file

@ -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()

View file

@ -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');
}

View file

@ -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/);
});

View file

@ -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' }],