From c44e4e28fd2419d8555bde9d54783cfc22afb585 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Wed, 11 Nov 2026 19:33:52 -0600 Subject: [PATCH] fix(org): review round for project remote set and check (#1873 P1-6, P1-7) The needs-credential line no longer promises that a binding will supply one; it says the check sends none. A recorded check is shown only for the URL it checked. The Settings section says when the git origin differs from the recorded repository, and a save repairs it. The URL rule message names what is refused. Tests cover a document write that fails after origin is set, the drift line and its repair, and Quick Setup in remote mode. --- packages/cli/README.md | 3 +- packages/cli/tests/org-repo-set.test.mjs | 9 ++++ packages/organization/README.md | 13 +++-- packages/organization/src/org-service.mjs | 9 +++- packages/organization/src/repo-remote.mjs | 7 ++- packages/organization/src/vocabulary.mjs | 2 +- .../organization/tests/repo-remote.test.mjs | 47 ++++++++++++++++--- packages/webui/src/public/project-remote.css | 1 + packages/webui/src/public/project-remote.js | 10 ++-- packages/webui/tests/org-admin.test.mjs | 4 +- .../webui/tests/org-quick-browser.test.mjs | 41 ++++++++++++++++ .../webui/tests/project-repo-browser.test.mjs | 31 +++++++++--- 12 files changed, 149 insertions(+), 28 deletions(-) diff --git a/packages/cli/README.md b/packages/cli/README.md index 06605aace..e26be7b2e 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -1283,7 +1283,8 @@ repo show ID PROJECT [--json]`. `mosaic org project repo set ID PROJECT (--local-only --accept-local-loss-risk | --remote URL) --if-revision N [--json]` changes a project's repository later. A remote is https or ssh; its checkout's `origin` is set and checked once with `git ls-remote`, without credentials, and -the receipt says `reachable`, `needs-credential` or `unreachable`. See +the receipt says `reachable`, `needs-credential` or `unreachable`. A copy +credential binding supplies credentials for https Gitea remotes only. See `packages/organization/README.md` for the URL rules, layout and scope. On existing CLI commands, `--org ID` is an alias for `--business ID`; if both are supplied they must name the same ID. diff --git a/packages/cli/tests/org-repo-set.test.mjs b/packages/cli/tests/org-repo-set.test.mjs index 4c778cee5..ce862b4f2 100644 --- a/packages/cli/tests/org-repo-set.test.mjs +++ b/packages/cli/tests/org-repo-set.test.mjs @@ -45,4 +45,13 @@ test('org project repo init --remote and repo set: origin follows, the check is assert.equal(back.revision, rev + 1); assert.equal(back.check.result, 'unreachable'); assert.match(back.check.message, /Failed to connect|Could not connect/); assert.equal(git(init.root, 'remote', 'get-url', 'origin'), https); assert.deepEqual(ok('org', 'project', 'repo', 'show', 'alpha', 'work', '--json').current, { mode: 'remote', remote: https, origin: https, revision: back.revision, check: back.check, changedAt: back.at }); + assert.equal(back.check.url, https); + + // The acceptance URL (#1873 P1-6): an .invalid host never resolves, so the check is unreachable + // and origin is still set to it. + const invalid = 'https://git.example.invalid/acme/work'; + const named = ok('org', 'project', 'repo', 'set', 'alpha', 'work', '--remote', invalid, '--if-revision', String(back.revision), '--json'); + assert.equal(named.origin, invalid); assert.equal(named.check.result, 'unreachable'); assert.equal(named.check.url, invalid); + assert.match(named.check.message, /resolve|timed out/i); + assert.equal(git(init.root, 'remote', '-v'), `origin\t${invalid} (fetch)\norigin\t${invalid} (push)`); }); diff --git a/packages/organization/README.md b/packages/organization/README.md index e64daea02..9c6c3748c 100644 --- a/packages/organization/README.md +++ b/packages/organization/README.md @@ -577,15 +577,18 @@ back; the project form also reports the binding as it is now (`current`). Remote mode (#1873 P1-6) accepts `https://HOST/OWNER/REPO` and ssh (`ssh://[USER@]HOST[:PORT]/PATH` or `USER@HOST:PATH`) only. It refuses `ext::` and other transport helpers, `file://`, local paths, values that begin -with `-`, and any URL that carries a user password or token -(`src/vocabulary.mjs` `parseRepoRemote`). The checkout's `origin` is set to the +with `-`, any URL that carries a password or token, and any https URL that +carries a user name (`src/vocabulary.mjs` `parseRepoRemote`). The checkout's `origin` is set to the URL and checked once with `git ls-remote` (`src/repo-remote.mjs`): no credential helper, no askpass, no terminal prompt, no redirects, `GIT_ALLOW_PROTOCOL=https:ssh`, no git or ssh config files, no ssh agent or key, and a 15 second limit. The receipt's `check.result` is `reachable`, -`needs-credential` (the credential binding will supply it) or `unreachable` -with git's error summarized. The check never refuses the setup, and nothing is -pushed. Tests reach a local bare repository only through `repoRemoteTestSeam`, +`needs-credential` (the check sends none) or `unreachable` with git's error +summarized; `check.url` names the URL it ran against, and `repo show` reports a +check only for the URL recorded now. The check never refuses the setup, and +nothing is pushed. A copy credential binding (CB lanes) supplies credentials +for https Gitea remotes only; an ssh or other https remote that needs a +credential gets none from it. Tests reach a local bare repository only through `repoRemoteTestSeam`, which no source module may import. `mosaic org project repo set ID PROJECT (--local-only --accept-local-loss-risk | diff --git a/packages/organization/src/org-service.mjs b/packages/organization/src/org-service.mjs index 9b8a08a37..a9a07cff1 100644 --- a/packages/organization/src/org-service.mjs +++ b/packages/organization/src/org-service.mjs @@ -217,7 +217,9 @@ async function setOrigin(root, selection) { else if (origin !== null) await git(root, ['remote', 'remove', 'origin']); return originOf(root); } -const checkFields = (check) => check ? { check: { result: check.result, message: check.message } } : {}; +// The check names the URL it ran against, so a reader can tell a check of an earlier URL from one of +// the URL recorded now (#1873 P1-7 review). +const checkFields = (check) => check ? { check: { result: check.result, message: check.message, url: check.url } } : {}; async function setup(ctx, id, project, opts) { authorize(ctx); const selection = options(opts); if (project !== null) checkId(project); // The check talks to the network, so it runs before any lock is taken. Its result is reported, @@ -256,7 +258,10 @@ async function showSetup(ctx, id, project) { // #1873 P1-7: the binding as it is now, which a later repo set may have changed since init. const binding = current.doc.projects[project].git ?? { mode: item.mode, ...(item.remote ? { remote: item.remote } : {}) }; const last = records.findLast(r => r.op === 'org.project.repo.set' && r.project === project && same(r.subject, current.principal)) ?? null; - return { ...item, current: { mode: binding.mode, ...(binding.mode === 'remote' ? { remote: binding.remote } : {}), origin: await originOf(root), revision: current.revision, ...((last ?? item).check ? { check: (last ?? item).check } : {}), ...(last ? { changedAt: last.at } : {}) } }; + // The last check is reported only when it ran against the URL recorded now: a set whose receipt + // did not land must not lend its check to another URL. + const check = binding.mode === 'remote' ? (last ?? item).check : undefined; + return { ...item, current: { mode: binding.mode, ...(binding.mode === 'remote' ? { remote: binding.remote } : {}), origin: await originOf(root), revision: current.revision, ...(check && check.url === binding.remote ? { check } : {}), ...(last ? { changedAt: last.at } : {}) } }; } // #1873 P1-7: change an initialized project's repository between remote and local-only, or to a // new URL. The same URL rules and check as init; origin follows; nothing is pushed. A receipt diff --git a/packages/organization/src/repo-remote.mjs b/packages/organization/src/repo-remote.mjs index 4135c9a26..f65ed32e7 100644 --- a/packages/organization/src/repo-remote.mjs +++ b/packages/organization/src/repo-remote.mjs @@ -34,6 +34,11 @@ const NEEDS_CREDENTIAL = [ // One plain line from git's stderr: the first line that says what went wrong, without "fatal: ", // control characters or more than 200 characters. +// Plain and true for every remote: the check itself sends none. Whether a credential binding can +// supply one later depends on the remote (copy bindings cover https Gitea remotes only), so this line +// does not promise it. +export const NEEDS_CREDENTIAL_MESSAGE = 'the repository needs a credential; this check uses none'; + export function summarizeGitError(stderr) { const lines = String(stderr).split('\n').map((l) => l.replace(/[\0-\x1f\x7f]/g, '').trim()).filter(Boolean); const line = lines.find((l) => /^(fatal|ssh|error):/i.test(l)) ?? lines[0] ?? ''; @@ -45,7 +50,7 @@ export function classifyCheck({ code, timedOut, spawnError, stderr }) { if (spawnError) return { result: 'unreachable', message: 'git could not be started' }; if (timedOut) return { result: 'unreachable', message: 'no answer before the check timed out' }; if (code === 0) return { result: 'reachable', message: 'the repository answered' }; - if (NEEDS_CREDENTIAL.some((p) => p.test(stderr))) return { result: 'needs-credential', message: 'the repository needs a credential; the credential binding will supply it' }; + if (NEEDS_CREDENTIAL.some((p) => p.test(stderr))) return { result: 'needs-credential', message: NEEDS_CREDENTIAL_MESSAGE }; return { result: 'unreachable', message: summarizeGitError(stderr) }; } diff --git a/packages/organization/src/vocabulary.mjs b/packages/organization/src/vocabulary.mjs index 51986b39a..4aa764ae8 100644 --- a/packages/organization/src/vocabulary.mjs +++ b/packages/organization/src/vocabulary.mjs @@ -254,7 +254,7 @@ const REMOTE_PROBLEMS = Object.freeze({ transport: 'the repository URL must not name a git transport helper (such as ext::)', local: 'the repository URL must not be a local path or a file:// URL', scheme: 'the repository URL must use https or ssh', - credentials: 'the repository URL must not carry a user password or token; credentials come from the credential binding', + credentials: 'the repository URL must not carry a password or token, or a user name in an https URL; a URL never holds a credential', https: 'an https repository URL must be normalized and name a repository, https://HOST/OWNER/REPO', ssh: 'an ssh repository URL must be ssh://[USER@]HOST[:PORT]/PATH or USER@HOST:PATH, with no "." or ".." segment', }); diff --git a/packages/organization/tests/repo-remote.test.mjs b/packages/organization/tests/repo-remote.test.mjs index 6debe7878..a21e76f5d 100644 --- a/packages/organization/tests/repo-remote.test.mjs +++ b/packages/organization/tests/repo-remote.test.mjs @@ -4,11 +4,11 @@ import assert from 'node:assert/strict'; import { createServer } from 'node:http'; import { createServer as createTcpServer } from 'node:net'; import { execFileSync } from 'node:child_process'; -import { mkdtempSync, mkdirSync, readdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { chmodSync, mkdtempSync, mkdirSync, readdirSync, readFileSync, writeFileSync } from 'node:fs'; import { dirname, join, resolve } from 'node:path'; import { tmpdir } from 'node:os'; import { fileURLToPath } from 'node:url'; -import { parseRepoRemote, checkRemote, classifyCheck, summarizeGitError, CHECK_SSH_COMMAND, repoRemoteTestSeam } from '../src/repo-remote.mjs'; +import { parseRepoRemote, checkRemote, classifyCheck, summarizeGitError, CHECK_SSH_COMMAND, NEEDS_CREDENTIAL_MESSAGE, repoRemoteTestSeam } from '../src/repo-remote.mjs'; import { createOrg, templateRender, projectRepoInit, projectRepoShow, projectRepoSet, queueInit, showOrg, updateOrg } from '../src/org-service.mjs'; import { loadBusiness } from '../src/organization-config.mjs'; import { REPO_ROLES } from './helpers.mjs'; @@ -48,7 +48,10 @@ test('remote URL rules: https and ssh only, nothing git could read as an option, test('check outcomes are classified from git\'s own messages', () => { assert.equal(classifyCheck({ code: 0, stderr: '' }).result, 'reachable'); assert.equal(classifyCheck({ code: 128, stderr: "fatal: could not read Username for 'https://git.example.com': terminal prompts disabled\n" }).result, 'needs-credential'); - assert.equal(classifyCheck({ code: 128, stderr: 'git@127.0.0.1: Permission denied (publickey,password).\nfatal: Could not read from remote repository.\n' }).result, 'needs-credential'); + // An ssh remote that wants a key: the line says the check sent none and promises no binding, + // because copy credential bindings cover https Gitea remotes only (#1873 review F1). + assert.deepEqual(classifyCheck({ code: 128, stderr: 'git@127.0.0.1: Permission denied (publickey,password).\nfatal: Could not read from remote repository.\n' }), + { result: 'needs-credential', message: 'the repository needs a credential; this check uses none' }); assert.equal(classifyCheck({ code: 128, stderr: "fatal: unable to access 'https://h/o/r/': The requested URL returned error: 403\n" }).result, 'needs-credential'); const dns = classifyCheck({ code: 128, stderr: "fatal: unable to access 'https://git.example.invalid/o/r.git/': Could not resolve host: git.example.invalid\n" }); assert.deepEqual(dns, { result: 'unreachable', message: "unable to access 'https://git.example.invalid/o/r.git/': Could not resolve host: git.example.invalid" }); @@ -94,7 +97,7 @@ test('a 401 is needs-credential and no credential is sent', async () => { const port = await listen(server); try { const result = await seam.lsRemote(`http://127.0.0.1:${port}/o/r.git`, { protocols: 'http' }); - assert.equal(result.result, 'needs-credential'); assert.match(result.message, /credential binding/); + assert.equal(result.result, 'needs-credential'); assert.equal(result.message, NEEDS_CREDENTIAL_MESSAGE); assert.doesNotMatch(result.message, /binding/); assert.ok(seen.length >= 1); assert.deepEqual(seen.filter(Boolean), []); } finally { server.close(); } }); @@ -138,7 +141,7 @@ test('remote init sets origin and records the check; nothing is pushed', async ( seam.useCheck((u) => { assert.equal(u, url); return seam.lsRemote(`file://${repo}`, { protocols: 'file' }); }); try { const r = await orgWithRepo(c, { mode: 'remote', remote: url }); - assert.equal(r.origin, url); assert.deepEqual(r.check, { result: 'reachable', message: 'the repository answered' }); + assert.equal(r.origin, url); assert.deepEqual(r.check, { result: 'reachable', message: 'the repository answered', url }); assert.equal(git(r.root, 'remote', '-v'), `origin\t${url} (fetch)\norigin\t${url} (push)`); assert.equal(git(repo, 'for-each-ref').length, 0, 'nothing pushed'); const shown = await projectRepoShow(c, 'alpha', 'work'); @@ -150,14 +153,14 @@ test('repo set switches remote to local-only and back, with receipts and the rev const c = context(); const r = await orgWithRepo(c); assert.equal(git(r.root, 'remote', '-v'), ''); let rev = (await showOrg(c, 'alpha')).revision; - seam.useCheck(async () => ({ result: 'needs-credential', message: 'the repository needs a credential; the credential binding will supply it' })); + seam.useCheck(async () => ({ result: 'needs-credential', message: NEEDS_CREDENTIAL_MESSAGE })); try { const ssh = 'git@git.example.com:team/work.git'; await assert.rejects(projectRepoSet(c, 'alpha', 'work', { mode: 'remote', remote: ssh, ifRevision: rev + 1 }), { code: 'revision-conflict' }); await assert.rejects(projectRepoSet(c, 'alpha', 'work', { mode: 'remote', remote: ssh }), { code: 'org-invalid' }); const set = await projectRepoSet(c, 'alpha', 'work', { mode: 'remote', remote: ssh, ifRevision: rev }); assert.equal(set.op, 'org.project.repo.set'); assert.equal(set.revision, rev + 1); assert.deepEqual(set.previous, { mode: 'local-only' }); - assert.equal(set.check.result, 'needs-credential'); assert.equal(git(r.root, 'remote', 'get-url', 'origin'), ssh); + assert.equal(set.check.result, 'needs-credential'); assert.equal(set.check.url, ssh); assert.equal(git(r.root, 'remote', 'get-url', 'origin'), ssh); assert.deepEqual(loadBusiness('alpha', { dir: c.configDir, rolesDir: REPO_ROLES }).projects.work.git, { mode: 'remote', remote: ssh }); rev = set.revision; const https = 'https://git.example.com/team/work'; @@ -180,6 +183,36 @@ test('repo set switches remote to local-only and back, with receipts and the rev } finally { seam.useCheck(null); } }); +// #1873 review F2: origin is set before the document is written. When that write fails, the +// checkout's origin and the recorded binding differ; show reports both as they are, keeps the +// last check only for the URL it ran against, and the same set again repairs it. +test('a set whose document write fails leaves origin ahead of the record, and show says so', async () => { + const c = context(); const r = await orgWithRepo(c); + seam.useCheck(async () => ({ result: 'reachable', message: 'the repository answered' })); + const businesses = join(c.configDir, 'businesses'); + try { + let rev = (await showOrg(c, 'alpha')).revision; + const a = 'https://git.example.com/team/a', b = 'https://git.example.com/team/b'; + rev = (await projectRepoSet(c, 'alpha', 'work', { mode: 'remote', remote: a, ifRevision: rev })).revision; + chmodSync(businesses, 0o500); + try { await assert.rejects(projectRepoSet(c, 'alpha', 'work', { mode: 'remote', remote: b, ifRevision: rev })); } + finally { chmodSync(businesses, 0o700); } + assert.equal(git(r.root, 'remote', 'get-url', 'origin'), b); + let shown = (await projectRepoShow(c, 'alpha', 'work')).current; + assert.deepEqual(shown, { mode: 'remote', remote: a, origin: b, revision: rev, check: { result: 'reachable', message: 'the repository answered', url: a }, changedAt: shown.changedAt }); + // Repeating the recorded URL repairs origin. + await projectRepoSet(c, 'alpha', 'work', { mode: 'remote', remote: a, ifRevision: rev }); + shown = (await projectRepoShow(c, 'alpha', 'work')).current; + assert.equal(shown.origin, a); assert.equal(shown.check.url, a); + // A recorded check for another URL is not lent to the current one. + const receipts = join(r.root, '../../receipts.jsonl'); + const lines = readFileSync(receipts, 'utf8').trim().split('\n').map((l) => JSON.parse(l)); + lines.at(-1).check.url = b; writeFileSync(receipts, lines.map((l) => JSON.stringify(l)).join('\n') + '\n'); + shown = (await projectRepoShow(c, 'alpha', 'work')).current; + assert.equal(shown.check, undefined); assert.equal(shown.remote, a); + } finally { seam.useCheck(null); chmodSync(businesses, 0o700); } +}); + test('repo set refuses bad URLs before any check, and needs an initialized repository', async () => { const c = context(); await createOrg(c, 'alpha', await templateRender('alpha')); let checks = 0; seam.useCheck(async () => { checks++; return { result: 'reachable', message: 'x' }; }); diff --git a/packages/webui/src/public/project-remote.css b/packages/webui/src/public/project-remote.css index 5f70aadfd..6e03588b7 100644 --- a/packages/webui/src/public/project-remote.css +++ b/packages/webui/src/public/project-remote.css @@ -13,6 +13,7 @@ .project-repo-facts { display: grid; grid-template-columns: max-content minmax(0, 1fr); gap: .25rem 1rem; margin: 0 0 1rem; } .project-repo-facts dt { font-weight: 600; } .project-repo-facts dd { margin: 0; overflow-wrap: anywhere; } +.project-repo-drift { margin: 0 0 1rem; padding: .5rem .75rem; border-left: 3px solid var(--warning, currentColor); } .project-repo [role="status"]:empty { display: none; } .project-repo [role="status"]:focus { outline: none; } @media (max-width: 480px) { .project-repo-facts { grid-template-columns: minmax(0, 1fr); } .project-repo-facts dd { margin-bottom: .5rem; } } diff --git a/packages/webui/src/public/project-remote.js b/packages/webui/src/public/project-remote.js index 462f728aa..be56f2f2f 100644 --- a/packages/webui/src/public/project-remote.js +++ b/packages/webui/src/public/project-remote.js @@ -189,10 +189,14 @@ const facts = `
Mode
${cur.mode === 'remote' ? 'Remote' : 'Local only'}
` + `
Repository URL
${cur.mode === 'remote' ? esc(cur.remote) : 'None (local only)'}
` + `
Git origin
${cur.origin ? esc(cur.origin) : 'None'}
` - + `
Last check
${cur.mode === 'remote' ? esc(checkLine(cur.check) ?? 'Not recorded') : 'None (local only has no remote)'}
`; + + `
Last check
${cur.mode === 'remote' ? esc((cur.check?.url === cur.remote && checkLine(cur.check)) || 'Not recorded for this URL') : 'None (local only has no remote)'}
`; + // origin and the recorded binding disagree when a save stopped between setting origin and + // recording it; saving again repairs it. + const drift = (cur.mode === 'remote' ? cur.origin !== cur.remote : cur.origin != null) + ? '

Git origin differs from the recorded repository; save it again to repair.

' : ''; const verb = `

Same as ${esc(value.verb)}

`; const why = state.archived ? '

This project is archived; its repository is shown read only.

' : ''; - section.innerHTML = `${head}${facts}${state.archived ? '' : form()}${why}${status}${verb}`; + section.innerHTML = `${head}${facts}${drift}${state.archived ? '' : form()}${why}${status}${verb}`; } async function refresh() { if (!org || !ID.test(project ?? '')) return; @@ -227,7 +231,7 @@ saving = false; if (result.ok && result.value.receipt) { const r = result.value.receipt, check = checkLine(result.value.check); - outcome = `Saved${r.at ? ` at ${r.at}` : ''}. ${r.mode === 'remote' ? `Origin is ${r.origin ?? 'not set'}.` : 'Origin removed; local only.'}${check ? ` Check: ${check}` : ''} Nothing was pushed.`; + outcome = `Saved${r.at ? ` at ${r.at}` : ''}. ${r.mode === 'remote' ? `Origin is ${r.origin ?? 'not set'}.` : r.origin == null ? 'Origin removed; local only.' : `Origin is still ${r.origin}.`}${check ? ` Check: ${check}` : ''} Nothing was pushed.`; draft = null; } else outcome = refusalLine(result); await refresh(); diff --git a/packages/webui/tests/org-admin.test.mjs b/packages/webui/tests/org-admin.test.mjs index c9d4d0ce6..eec8c6a28 100644 --- a/packages/webui/tests/org-admin.test.mjs +++ b/packages/webui/tests/org-admin.test.mjs @@ -32,7 +32,7 @@ function fakes() { projectRepoShow: async (ctx, id, project) => { const o = get(id); if (!o.doc.projects?.[project]) throw new OrgError('org-unknown', 'unknown project'); const r = o.repos[project]; if (!r) throw new OrgError('org-unavailable', 'setup receipt unavailable'); return r; }, projectRepoInit: async (ctx, id, project, opts) => { calls.push(['projectRepoInit', id, project, opts]); const o = get(id); if (fail.project === 'before') throw new Error('crashed'); o.repos[project] = receipt('org.project.repo.init', id, { project, mode: opts.mode }); if (fail.project === 'after') throw new Error('timeout'); return o.repos[project]; }, // #1873 P1-7: like C2 projectRepoSet, the revision guards the change and origin follows the mode. - projectRepoSet: async (ctx, id, project, opts) => { calls.push(['projectRepoSet', id, project, opts]); const o = get(id); if (fail.repoSet) throw new Error('timeout'); if (o.revision !== opts.ifRevision) throw new OrgError('revision-conflict'); const r = o.repos[project]; o.revision++; const remote = opts.mode === 'remote' ? opts.remote : undefined; o.repos[project] = { ...r, current: { mode: opts.mode, ...(remote ? { remote } : {}), origin: remote ?? null, revision: o.revision, ...(remote ? { check: { result: 'needs-credential', message: 'the repository needs a credential; the credential binding will supply it' } } : {}) } }; return receipt('org.project.repo.set', id, { project, mode: opts.mode, ...(remote ? { remote } : {}), root: `/data/org-runtime/${id}/projects/${project}`, revision: o.revision, origin: remote ?? null, ...(remote ? { check: o.repos[project].current.check } : {}) }); }, + projectRepoSet: async (ctx, id, project, opts) => { calls.push(['projectRepoSet', id, project, opts]); const o = get(id); if (fail.repoSet) throw new Error('timeout'); if (o.revision !== opts.ifRevision) throw new OrgError('revision-conflict'); const r = o.repos[project]; o.revision++; const remote = opts.mode === 'remote' ? opts.remote : undefined; o.repos[project] = { ...r, current: { mode: opts.mode, ...(remote ? { remote } : {}), origin: remote ?? null, revision: o.revision, ...(remote ? { check: { result: 'needs-credential', message: 'the repository needs a credential; this check uses none' } } : {}) } }; return receipt('org.project.repo.set', id, { project, mode: opts.mode, ...(remote ? { remote } : {}), root: `/data/org-runtime/${id}/projects/${project}`, revision: o.revision, origin: remote ?? null, ...(remote ? { check: o.repos[project].current.check } : {}) }); }, }; const registry = { listOrgs: async () => [...orgs.keys()].map(id => ({ id, state: orgs.get(id).reserved ? 'reserved' : 'active' })) }; let running = null; @@ -469,7 +469,7 @@ test('against the real C2 service: create, set up, resume and refuse an overwrit const registry = await import('../../organization/src/org-registry.mjs'); // #1873 P1-6: the remote check goes through the test seam, so this test never leaves the host. const { repoRemoteTestSeam } = await import('../../organization/src/repo-remote.mjs'); - const checked = []; repoRemoteTestSeam.useCheck(async url => { checked.push(url); return { result: 'needs-credential', message: 'the repository needs a credential; the credential binding will supply it' }; }); + const checked = []; repoRemoteTestSeam.useCheck(async url => { checked.push(url); return { result: 'needs-credential', message: 'the repository needs a credential; this check uses none' }; }); t.after(() => repoRemoteTestSeam.useCheck(null)); const { execFileSync } = await import('node:child_process'); const { mkdtempSync, mkdirSync, existsSync } = await import('node:fs'); diff --git a/packages/webui/tests/org-quick-browser.test.mjs b/packages/webui/tests/org-quick-browser.test.mjs index dcd322d9f..08ff80370 100644 --- a/packages/webui/tests/org-quick-browser.test.mjs +++ b/packages/webui/tests/org-quick-browser.test.mjs @@ -115,3 +115,44 @@ test('an unreadable success reply stops quick setup and a new draft gets fresh c assert.deepEqual(await controls(), [false, false, false]); assert.equal(await b.evaluate(`document.querySelector('#org-risk-accept').checked`), false); }); + +// #1873 P1-6: Quick Setup in remote mode sets the project checkout's origin, checks it once (here +// through the test seam, so nothing leaves the host) and shows the outcome on the project step. +test('quick setup in remote mode sets origin and shows the check', { timeout: 45000 }, async t => { + const { mkdtempSync, mkdirSync, rmSync } = await import('node:fs'); + const { tmpdir } = await import('node:os'); const { join } = await import('node:path'); const { execFileSync } = await import('node:child_process'); + const registry = await import('../../organization/src/org-registry.mjs'); + const { repoRemoteTestSeam } = await import('../../organization/src/repo-remote.mjs'); + const remote = 'https://git.example.invalid/alpha/work'; + const checked = []; repoRemoteTestSeam.useCheck(async url => { checked.push(url); return { result: 'unreachable', message: 'Could not resolve host: git.example.invalid' }; }); + t.after(() => repoRemoteTestSeam.useCheck(null)); + const dir = mkdtempSync(join(tmpdir(), 'quick-')); + const configDir = join(dir, 'config'), dataRoot = join(dir, 'data'); mkdirSync(configDir, { mode: 0o700 }); mkdirSync(dataRoot, { mode: 0o700 }); + let starts = 0; + const bus = { busStatus: async () => ({ healthy: false }), busStart: async () => { starts++; return { healthy: true }; } }; + const admin = createOrgAdmin({ service, registry, bus, context: async () => ({ configDir, dataRoot, instance: 'test' }) }); + const projects = []; + const organizations = { + list: async () => starts ? [{ id: 'alpha', label: 'alpha', busHealthy: true }] : [], + resolve: async id => id === 'alpha' ? { send: async () => ({}), busHealthy: true, chat: { + projects: async () => projects, threads: async () => [], + createProject: async args => { const row = { id: '20000000-0000-4000-8000-000000000002', ...args }; projects.push(row); return row; }, + } } : null, + }; + const server = await startServer({ port: 0, sso: createDevOwner(), organizations, orgAdmin: admin, send: async () => ({}), board: 'http://127.0.0.1:9', log: () => {} }); t.after(() => close(server)); + const b = await browser({ machine: false, args: ['--disable-extensions'] }); t.after(() => b.close()); + t.after(() => rmSync(dir, { recursive: true, force: true })); + const wait = expression => b.evaluate(`(async()=>{for(let i=0;i<300;i++){if(${expression})return;await new Promise(r=>setTimeout(r,30))}throw Error(${JSON.stringify(expression)})})()`); + await b.navigate(`http://127.0.0.1:${server.address().port}/#/settings/organizations`); + await wait('document.querySelector("#org-new-form [data-write]") && !document.querySelector("#org-new-form [data-write]").disabled'); + await b.evaluate(`document.querySelector('#org-new-id').value='alpha';document.querySelector('#org-new-form [data-write]').click()`); + await wait('document.querySelector("#org-quick-open") && !document.querySelector("#org-paths").hidden'); + await b.evaluate(`document.querySelector('#org-quick-open').click();document.querySelector('#org-project-remote').value=${JSON.stringify(remote)};document.querySelector('#org-risk-accept').click();document.querySelector('#org-quick-run').click()`); + await wait(`document.querySelector('[data-quick-step="5"] .org-pill')?.textContent === 'Done'`); + assert.equal((await b.evaluate(`document.querySelector('[data-remote-check]')?.textContent ?? ''`)).trim(), `Remote check: unreachable, Could not resolve host: git.example.invalid. Origin: ${remote}.`); + assert.deepEqual(checked, [remote]); + const git = (...args) => execFileSync('git', ['-C', join(dataRoot, 'org-runtime/alpha/projects/work'), ...args], { env: { PATH: process.env.PATH, HOME: '/nonexistent', GIT_CONFIG_NOSYSTEM: '1', GIT_CONFIG_GLOBAL: '/dev/null' }, encoding: 'utf8' }).trim(); + assert.equal(git('remote', '-v'), `origin\t${remote} (fetch)\norigin\t${remote} (push)`); + // The organization checkout stays local only. + assert.equal(execFileSync('git', ['-C', join(dataRoot, 'org-runtime/alpha/checkout'), 'remote', '-v'], { env: { PATH: process.env.PATH, HOME: '/nonexistent', GIT_CONFIG_NOSYSTEM: '1', GIT_CONFIG_GLOBAL: '/dev/null' }, encoding: 'utf8' }).trim(), ''); +}); diff --git a/packages/webui/tests/project-repo-browser.test.mjs b/packages/webui/tests/project-repo-browser.test.mjs index 945d249bb..dc5e7af5b 100644 --- a/packages/webui/tests/project-repo-browser.test.mjs +++ b/packages/webui/tests/project-repo-browser.test.mjs @@ -6,7 +6,7 @@ // MOSAIC_UX_SHOTS= also writes light and dark shots at 1440x900 and 390x844. import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { mkdtempSync, mkdirSync } from 'node:fs'; +import { mkdtempSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { execFileSync } from 'node:child_process'; @@ -20,7 +20,7 @@ import { browser } from './browser.mjs'; import { close } from './fixture.mjs'; const P = '10000000-0000-4000-8000-0000000000c1'; -const NEEDS = { result: 'needs-credential', message: 'the repository needs a credential; the credential binding will supply it' }; +const NEEDS = { result: 'needs-credential', message: 'the repository needs a credential; this check uses none' }; const waitFor = b => expression => b.evaluate(`(async()=>{for(let i=0;i<300;i++){if(${expression})return true;await new Promise(r=>setTimeout(r,30))}throw Error(${JSON.stringify(expression)}+' | '+(document.querySelector('main')?.innerText||'').slice(0,1500))})()`); const text = (b, selector) => b.evaluate(`(document.querySelector(${JSON.stringify(selector)})?.innerText ?? '').replace(/\\s+/g, ' ').trim()`); const click = (b, selector) => b.evaluate(`(document.querySelector(${JSON.stringify(selector)}).click(), true)`); @@ -55,8 +55,9 @@ async function console_(t, { send = true, width = 1440, height = 900, root = nul await b.viewport(width, height); const base = `http://127.0.0.1:${server.address().port}/`; const checkout = join(dataRoot, 'org-runtime/acme/projects/work'); - const remotes = () => execFileSync('git', ['-C', checkout, 'remote', '-v'], { env: { PATH: process.env.PATH, HOME: '/nonexistent', GIT_CONFIG_NOSYSTEM: '1', GIT_CONFIG_GLOBAL: '/dev/null' }, encoding: 'utf8' }).trim(); - return { b, base, wait: waitFor(b), errors, root, remotes }; + const git = (...args) => execFileSync('git', ['-C', checkout, ...args], { env: { PATH: process.env.PATH, HOME: '/nonexistent', GIT_CONFIG_NOSYSTEM: '1', GIT_CONFIG_GLOBAL: '/dev/null' }, encoding: 'utf8' }).trim(); + const remotes = () => git('remote', '-v'); + return { b, base, wait: waitFor(b), errors, root, remotes, git, receipts: join(dataRoot, 'org-runtime/acme/receipts.jsonl') }; } // Create the organization and set the project up local-only, as Quick Setup does, through the routes. @@ -73,6 +74,8 @@ const settings = async ({ b, base, wait }) => { await b.navigate(`${base}#/orgs/acme/projects/${P}/settings`); await wait('document.querySelector("[data-project-repo]") && !/Reading the repository/.test(document.querySelector("[data-project-repo]").textContent)'); }; +// A fresh page load, so the section reads the repository again rather than reusing its last view. +const reread = async p => { await p.b.evaluate('location.reload(), true'); await new Promise(r => setTimeout(r, 200)); await settings(p); }; const facts = b => b.evaluate(`['mode', 'url', 'origin', 'check'].map(k => document.querySelector('[data-repo-' + k + ']')?.textContent ?? null)`); const save = async ({ b, wait }, pattern) => { await b.evaluate(`document.querySelector('[data-project-repo-status]').textContent = '', true`); @@ -97,9 +100,9 @@ test('the owner edits the repository on Settings: remote, refusals, local only a const https = 'https://git.example.invalid/acme/work'; await fill(b, '#project-repo-url', https); const saved = await save(p, '/Saved/'); - assert.match(saved, new RegExp(`^Saved at \\S+\\. Origin is ${https.replace(/[.]/g, '\\.')}\\. Check: Needs a credential: the repository needs a credential; the credential binding will supply it\\. Nothing was pushed\\.$`)); + assert.match(saved, new RegExp(`^Saved at \\S+\\. Origin is ${https.replace(/[.]/g, '\\.')}\\. Check: Needs a credential: the repository needs a credential; this check uses none\\. Nothing was pushed\\.$`)); assert.equal(await b.evaluate(`document.activeElement?.hasAttribute('data-project-repo-status')`), true); - assert.deepEqual(await facts(b), ['Remote', https, https, 'Needs a credential: the repository needs a credential; the credential binding will supply it.']); + assert.deepEqual(await facts(b), ['Remote', https, https, 'Needs a credential: the repository needs a credential; this check uses none.']); assert.equal(p.remotes(), `origin\t${https} (fetch)\norigin\t${https} (push)`); assert.deepEqual(checked, [https]); @@ -144,6 +147,22 @@ test('the owner edits the repository on Settings: remote, refusals, local only a assert.equal(p.remotes(), `origin\t${scp} (fetch)\norigin\t${scp} (push)`); const receipts = (await api(b, '/api/orgs/acme/repo/project/work')).body; assert.equal(receipts.current.mode, 'remote'); assert.equal(receipts.current.origin, scp); + assert.equal(await b.evaluate(`document.querySelector('[data-repo-drift]')`), null, 'origin matches: no drift line'); + + // #1873 review F2: origin moved away from the recorded URL (a save that stopped after setting + // origin). The page says so; saving the recorded URL again repairs it. + p.git('remote', 'set-url', 'origin', 'https://elsewhere.invalid/x/y'); + await reread(p); + assert.deepEqual(await facts(b), ['Remote', scp, 'https://elsewhere.invalid/x/y', 'Needs a credential: the repository needs a credential; this check uses none.']); + assert.equal(await text(b, '[data-repo-drift]'), 'Git origin differs from the recorded repository; save it again to repair.'); + await save(p, '/Saved/'); + assert.equal(p.remotes(), `origin\t${scp} (fetch)\norigin\t${scp} (push)`); + assert.equal(await b.evaluate(`document.querySelector('[data-repo-drift]')`), null); + // A recorded check of another URL is not shown as this URL's check. + const lines = readFileSync(p.receipts, 'utf8').trim().split('\n').map(l => JSON.parse(l)); + lines.at(-1).check.url = 'https://git.example.invalid/acme/other'; writeFileSync(p.receipts, lines.map(l => JSON.stringify(l)).join('\n') + '\n'); + await reread(p); + assert.equal((await facts(b))[3], 'Not recorded for this URL'); assert.doesNotMatch(await b.evaluate('document.body.innerText'), new RegExp(p.root.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')), 'no server path reaches the page'); assert.deepEqual(p.errors, []); });