From 63c15c42dd03fb3a4423e95905daf40d88ade38c Mon Sep 17 00:00:00 2001 From: synsoftworks Date: Wed, 15 Apr 2026 13:24:29 -0700 Subject: [PATCH] refactor(status): decouple status from doctor internals Harden the status path so `gitrole status` and `status --short` no longer depend on diagnosis check-label internals. Also: - add a small shared alignment evaluator in `src/application/alignment.ts` - move role matching into a neutral shared primitive - compute `overall`, `commit`, `remote`, and `auth` directly from observed state, matched role, and repo policy - rework `status` to build from observed state and the new alignment summary instead of calling `doctor()` - keep `doctor` as the detailed explanation surface while reusing the shared role-matching seam - add regression coverage proving status summary behavior is derived from stable evaluation logic rather than diagnosis labels Verified with: - npm run build - npm test - npm run test:e2e - npm run test:release --- src/application/alignment.ts | 133 +++++++++++++++++++++++ src/application/use-cases/diagnosis.ts | 10 +- src/application/use-cases/status.ts | 139 ++++++++++--------------- test/use-cases.test.ts | 85 +++++++++++++++ 4 files changed, 278 insertions(+), 89 deletions(-) create mode 100644 src/application/alignment.ts diff --git a/src/application/alignment.ts b/src/application/alignment.ts new file mode 100644 index 0000000..7ca6829 --- /dev/null +++ b/src/application/alignment.ts @@ -0,0 +1,133 @@ +/* + * Shared alignment primitives consumed by stable status summaries and diagnosis flows. + */ +import { matchesIdentity, type Role } from '../domain/role.js'; +import type { DoctorResult, RepoPolicyEvaluation, StatusResult } from './contracts.js'; +import type { ObservedState } from './observed-state.js'; + +export interface AlignmentSummary { + overall: StatusResult['overall']; + commit: StatusResult['commit']; + remote: StatusResult['remote']; + auth: StatusResult['auth']; +} + +export function findMatchingRole( + roles: Role[], + commitIdentity: DoctorResult['commitIdentity'] +): Role | undefined { + return roles.find((candidate) => + matchesIdentity(candidate, { + fullName: commitIdentity.fullName.value, + email: commitIdentity.email.value + }) + ); +} + +export function summarizeAlignment(input: { + role?: Role; + observedState: ObservedState; + repoPolicy?: RepoPolicyEvaluation; +}): AlignmentSummary { + const commit = getCommitStatus(input); + const remote = getRemoteStatus(input); + const auth = getAuthStatus(input); + const overall = + !input.observedState.repository.isInsideWorkTree || + commit === 'warn' || + remote === 'warn' || + auth === 'warn' || + input.repoPolicy?.status === 'notAllowed' + ? 'warning' + : 'aligned'; + + return { + overall, + commit, + remote, + auth + }; +} + +function getCommitStatus(input: { + role?: Role; + observedState: ObservedState; +}): StatusResult['commit'] { + const { observedState, role } = input; + + if ( + !observedState.commitIdentity.fullName.value || + !observedState.commitIdentity.email.value || + observedState.scope.effective === 'mixed' || + !role + ) { + return 'warn'; + } + + if (hasIdentityDivergence(role, observedState)) { + return 'warn'; + } + + return 'ok'; +} + +function getRemoteStatus(input: { + role?: Role; + observedState: ObservedState; +}): StatusResult['remote'] { + const { observedState, role } = input; + + if (!observedState.repository.isInsideWorkTree) { + return 'na'; + } + + if (!observedState.repository.remote || observedState.repository.hasCommits === false) { + return 'warn'; + } + + if ( + role?.githubHost && + observedState.repository.remote.host && + observedState.repository.remote.host !== role.githubHost + ) { + return 'warn'; + } + + return 'ok'; +} + +function getAuthStatus(input: { + role?: Role; + observedState: ObservedState; +}): StatusResult['auth'] { + const { observedState, role } = input; + + if (!observedState.repository.isInsideWorkTree || !observedState.repository.remote) { + return 'na'; + } + + if (observedState.repository.remote.protocol === 'https') { + return 'warn'; + } + + if (!observedState.sshAuth || !observedState.sshAuth.ok) { + return 'warn'; + } + + if (role?.githubUser && observedState.sshAuth.githubUser !== role.githubUser) { + return 'warn'; + } + + return 'ok'; +} + +function hasIdentityDivergence(role: Role, observedState: ObservedState): boolean { + return Boolean( + observedState.sshAuth?.ok && + observedState.sshAuth.githubUser && + role.githubUser !== undefined && + observedState.sshAuth.githubUser !== role.githubUser && + observedState.commitIdentity.fullName.value && + observedState.commitIdentity.email.value + ); +} diff --git a/src/application/use-cases/diagnosis.ts b/src/application/use-cases/diagnosis.ts index dca4e31..aa54d1f 100644 --- a/src/application/use-cases/diagnosis.ts +++ b/src/application/use-cases/diagnosis.ts @@ -1,7 +1,8 @@ /* * Implements repository diagnosis and post-switch alignment checks. */ -import { matchesIdentity, type Role } from '../../domain/role.js'; +import type { Role } from '../../domain/role.js'; +import { findMatchingRole } from '../alignment.js'; import { getDoctorOverall, DoctorCheck, @@ -30,12 +31,7 @@ export async function doctor( collectObservedState(dependencies), loadOptionalRepoPolicy(dependencies.repository) ]); - const role = roles.find((candidate) => - matchesIdentity(candidate, { - fullName: observedState.commitIdentity.fullName.value, - email: observedState.commitIdentity.email.value - }) - ); + const role = findMatchingRole(roles, observedState.commitIdentity); const evaluatedRepoPolicy = repoPolicy ? evaluateRepoPolicy(repoPolicy, role?.name) : undefined; const checks = buildDoctorChecks({ role, diff --git a/src/application/use-cases/status.ts b/src/application/use-cases/status.ts index 50f1c3e..a85c687 100644 --- a/src/application/use-cases/status.ts +++ b/src/application/use-cases/status.ts @@ -1,13 +1,10 @@ /* * Produces the compact status summary shown by the CLI. */ -import type { - DoctorDependencies, - DoctorResult, - NonMergeCommit, - StatusResult -} from '../contracts.js'; -import { doctor } from './diagnosis.js'; +import { findMatchingRole, summarizeAlignment } from '../alignment.js'; +import type { DoctorDependencies, DoctorResult, NonMergeCommit, StatusResult } from '../contracts.js'; +import { collectObservedState, type ObservedState } from '../observed-state.js'; +import { evaluateRepoPolicy, loadOptionalRepoPolicy } from '../repo-policy.js'; /** * Returns a compact alignment summary for the current environment. @@ -15,37 +12,52 @@ import { doctor } from './diagnosis.js'; export async function getStatus( dependencies: DoctorDependencies ): Promise { - const verification = await collectVerificationContext(dependencies); - const { doctorResult, lastNonMergeCommit } = verification; - const commitIdentity = formatCommitIdentity(doctorResult.commitIdentity); + const verification = await collectStatusContext(dependencies); + const { observedState, role, repoPolicy, lastNonMergeCommit } = verification; + const commitIdentity = formatCommitIdentity(observedState.commitIdentity); + const summary = summarizeAlignment({ + role, + observedState, + repoPolicy + }); return { - roleName: doctorResult.role?.name ?? 'no-role', + roleName: role?.name ?? 'no-role', commitIdentity, - pushAuth: formatPushAuth(doctorResult), - scope: doctorResult.scope.effective, - localOverride: doctorResult.scope.hasLocalOverride, + pushAuth: formatPushAuth(role, observedState), + scope: observedState.scope.effective, + localOverride: observedState.scope.hasLocalOverride, lastNonMergeCommit, - historyNote: formatHistoryNote(doctorResult, lastNonMergeCommit), - overall: doctorResult.overall === 'warning' ? 'warning' : 'aligned', - commit: getCheckGroupStatus(doctorResult.checks, ['role', 'commit', 'identity', 'fix', 'scope']), - remote: getRemoteStatus(doctorResult), - auth: getAuthStatus(doctorResult), - repoPolicy: doctorResult.repoPolicy + historyNote: formatHistoryNote(observedState.commitIdentity, lastNonMergeCommit), + overall: summary.overall, + commit: summary.commit, + remote: summary.remote, + auth: summary.auth, + repoPolicy }; } -async function collectVerificationContext( +async function collectStatusContext( dependencies: DoctorDependencies ): Promise<{ - doctorResult: DoctorResult; + observedState: ObservedState; + role?: DoctorResult['role']; + repoPolicy?: StatusResult['repoPolicy']; lastNonMergeCommit?: NonMergeCommit; }> { - const doctorResult = await doctor(dependencies); - const lastNonMergeCommit = await dependencies.repository.getLatestNonMergeCommit(); + const [roles, observedState, repoPolicySource, lastNonMergeCommit] = await Promise.all([ + dependencies.roleStore.list(), + collectObservedState(dependencies), + loadOptionalRepoPolicy(dependencies.repository), + dependencies.repository.getLatestNonMergeCommit() + ]); + const role = findMatchingRole(roles, observedState.commitIdentity); + const repoPolicy = repoPolicySource ? evaluateRepoPolicy(repoPolicySource, role?.name) : undefined; return { - doctorResult, + observedState, + role, + repoPolicy, lastNonMergeCommit }; } @@ -58,86 +70,49 @@ function formatCommitIdentity(identity: DoctorResult['commitIdentity']): string return `${identity.fullName.value} <${identity.email.value}>`; } -function formatPushAuth(result: DoctorResult): string | undefined { - if (result.sshAuth?.githubUser) { - return `${result.sshAuth.githubUser} via ${result.sshAuth.host}`; +function formatPushAuth( + role: DoctorResult['role'], + observedState: { + repository: DoctorResult['repository']; + sshAuth?: DoctorResult['sshAuth']; + } +): string | undefined { + if (observedState.sshAuth?.githubUser) { + return `${observedState.sshAuth.githubUser} via ${observedState.sshAuth.host}`; } - if (result.repository.remote?.protocol === 'https') { + if (observedState.repository.remote?.protocol === 'https') { return 'unverified (origin uses HTTPS)'; } - if (result.role?.githubUser && result.role?.githubHost) { - return `${result.role.githubUser} via ${result.role.githubHost}`; + if (role?.githubUser && role?.githubHost) { + return `${role.githubUser} via ${role.githubHost}`; } - if (result.role?.githubUser) { - return result.role.githubUser; + if (role?.githubUser) { + return role.githubUser; } - if (result.role?.githubHost) { - return result.role.githubHost; + if (role?.githubHost) { + return role.githubHost; } return undefined; } -function getCheckGroupStatus( - checks: DoctorResult['checks'], - labels: string[] -): 'ok' | 'warn' | 'na' { - const relevantChecks = checks.filter((check) => labels.includes(check.label)); - - if (relevantChecks.length === 0) { - return 'na'; - } - - return relevantChecks.some((check) => check.status === 'warn') ? 'warn' : 'ok'; -} - -function getRemoteStatus(result: DoctorResult): 'ok' | 'warn' | 'na' { - if (!result.repository.isInsideWorkTree) { - return 'na'; - } - - if (!result.repository.remote) { - return 'warn'; - } - - return getCheckGroupStatus(result.checks, ['repo', 'remote', 'host', 'owner', 'history']); -} - -function getAuthStatus(result: DoctorResult): 'ok' | 'warn' | 'na' { - if (!result.repository.isInsideWorkTree || !result.repository.remote) { - return 'na'; - } - - if (result.repository.remote.protocol === 'https') { - return 'warn'; - } - - if (!result.sshAuth || !result.sshAuth.ok) { - return 'warn'; - } - - return result.checks.some((check) => check.label === 'auth' && check.status === 'warn') - ? 'warn' - : 'ok'; -} - function formatHistoryNote( - doctorResult: DoctorResult, + commitIdentity: DoctorResult['commitIdentity'], lastNonMergeCommit?: NonMergeCommit ): string | undefined { - const effectiveIdentity = formatCommitIdentity(doctorResult.commitIdentity); + const effectiveIdentity = formatCommitIdentity(commitIdentity); if (!effectiveIdentity || !lastNonMergeCommit) { return undefined; } const isMismatch = - doctorResult.commitIdentity.fullName.value !== lastNonMergeCommit.authorName || - doctorResult.commitIdentity.email.value !== lastNonMergeCommit.authorEmail; + commitIdentity.fullName.value !== lastNonMergeCommit.authorName || + commitIdentity.email.value !== lastNonMergeCommit.authorEmail; if (!isMismatch) { return undefined; diff --git a/test/use-cases.test.ts b/test/use-cases.test.ts index c0fbefc..34e511b 100644 --- a/test/use-cases.test.ts +++ b/test/use-cases.test.ts @@ -22,6 +22,7 @@ import { useRole } from '../src/application/use-cases/index.js'; import type { AppDependencies, DoctorDependencies } from '../src/application/contracts.js'; +import { summarizeAlignment } from '../src/application/alignment.js'; import { RepoPolicyAlreadyExistsError } from '../src/application/repo-policy.js'; import { parseRemoteUrl } from '../src/adapters/git-repository.js'; import { InvalidRoleNameError, type Role } from '../src/domain/role.js'; @@ -1702,6 +1703,90 @@ test('status warns when the effective role is not allowed by repo policy', async assert.equal(status.overall, 'warning'); }); +test('status summary evaluation derives warnings from observed state, not diagnosis labels', () => { + const role: Role = { + name: 'synsoftworksdev', + fullName: 'synsoftworks', + email: 'synthesissoftworks@gmail.com', + githubUser: 'synsoftworksdev', + githubHost: 'github.com-synsoftworksdev' + }; + + const summary = summarizeAlignment({ + role, + observedState: { + commitIdentity: { + fullName: { value: role.fullName, source: 'local' }, + email: { value: role.email, source: 'local' } + }, + configuredIdentity: { + local: { fullName: role.fullName, email: role.email }, + global: {} + }, + scope: { + effective: 'local', + hasLocalOverride: true + }, + repository: { + isInsideWorkTree: true, + hasCommits: true, + remote: parseRemoteUrl( + 'origin', + 'git@github.com-saraeloop:synsoftworksdev/gitrole.git' + ) + }, + sshAuth: { + ok: true, + host: 'github.com-saraeloop', + githubUser: 'saraeloop' + } + } + }); + + assert.deepEqual(summary, { + overall: 'warning', + commit: 'warn', + remote: 'warn', + auth: 'warn' + }); +}); + +test('status summary evaluation warns outside a git repository without relying on doctor checks', () => { + const role: Role = { + name: 'work', + fullName: 'Alex Developer', + email: 'alex@work.example' + }; + + const summary = summarizeAlignment({ + role, + observedState: { + commitIdentity: { + fullName: { value: role.fullName, source: 'global' }, + email: { value: role.email, source: 'global' } + }, + configuredIdentity: { + local: {}, + global: { fullName: role.fullName, email: role.email } + }, + scope: { + effective: 'global', + hasLocalOverride: false + }, + repository: { + isInsideWorkTree: false + } + } + }); + + assert.deepEqual(summary, { + overall: 'warning', + commit: 'ok', + remote: 'na', + auth: 'na' + }); +}); + test('doctor warns when HTTPS remotes prevent SSH auth verification', async () => { const role: Role = { name: 'work',