diff --git a/.devcontainer/devcontainer.env b/.devcontainer/devcontainer.env index ac60b770..346a1e7c 100644 --- a/.devcontainer/devcontainer.env +++ b/.devcontainer/devcontainer.env @@ -12,6 +12,9 @@ DF_JPLAG_REPORT_DIR=/jplag/results DF_JPLAG_SKIP_CLUSTER_CHECK=true DF_JPLAG_MAX_SHOWN_COMPARISONS=-1 +# Peer Progress Indicator local development defaults +DF_PPI_MINIMUM_COHORT_SIZE=21 +DF_PPI_STALE_AFTER_HOURS=48 # Overseer - enabled! OVERSEER_ENABLED=1 diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 00000000..1cc4d7d7 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1,5 @@ +# The App key is available only to the default-branch policy job. Keep every +# workflow and the evaluator itself under lead review. +/.github/workflows/ @ontrack-features-t2-2026/ontrack-leads +/.github/review-policy/ @ontrack-features-t2-2026/ontrack-leads +/.github/CODEOWNERS @ontrack-features-t2-2026/ontrack-leads diff --git a/.github/review-policy/README.md b/.github/review-policy/README.md new file mode 100644 index 00000000..bcf9e76f --- /dev/null +++ b/.github/review-policy/README.md @@ -0,0 +1,51 @@ +# OnTrack pull-request review policy + +The required status context `ontrack/review-policy` passes when the current pull +request head has either: + +- one approval from a current `ontrack-leads` member; or +- two approvals from distinct current `ontrack-contributors` members. + +Approvals from the pull-request author, bots, stale commits, dismissed reviews, +or reviewers whose latest actionable review requests changes do not count. + +## Security model + +`ontrack-review-policy-signal.yml` is unprivileged and never checks out pull-request +code. A completed signal wakes `ontrack-review-policy.yml` through `workflow_run`. +The evaluator workflow checks out only this directory from the protected default +branch, then mints a short-lived token for the organization-owned GitHub App. + +The App is installed only on the three OnTrack Doubtfire repositories and has: + +- organization Members: read; +- repository Metadata: read (mandatory); +- repository Pull requests: read; and +- repository Commit statuses: write. + +The App has no contents, workflow, administration, merge, or webhook permission. +Its private key is held in `ONTRACK_REVIEW_APP_PRIVATE_KEY` in the +`ontrack-review-policy` environment, which only permits the protected `11.0.x` +branch. Its numeric App ID is held in `ONTRACK_REVIEW_APP_ID`. + +The evaluator reports on the pull-request head commit. GitHub gates on the test merge +commit whenever that commit carries a status and only falls back to the head when it +carries none, so reporting on the test merge commit would move the merge gate onto a +commit that carries none of this repository's other checks. The head is also stable, +where the test merge commit is recomputed every time the base branch moves. +A five-minute reconciliation covers team membership and base-branch changes that +do not emit a pull-request review event. Unchanged results are not republished, +which avoids GitHub's per-commit status limit. + +## Ruleset integration + +Keep the native one-overall-approval rule, stale-review dismissal, and conversation +resolution. Require `ontrack/review-policy` with the OnTrack Review Policy App as +its expected source. Remove the native `ontrack-leads` required-reviewer entry only +after the App status has been observed and made required; otherwise GitHub combines +the native team rules with AND semantics. + +Changes to any workflow, the evaluator, or CODEOWNERS should continue to require +one `ontrack-leads` approval through a path-specific native reviewer rule. This is +necessary because any default-branch workflow could otherwise reference the App's +environment secret. diff --git a/.github/review-policy/evaluate.mjs b/.github/review-policy/evaluate.mjs new file mode 100644 index 00000000..f9dfa6fc --- /dev/null +++ b/.github/review-policy/evaluate.mjs @@ -0,0 +1,529 @@ +import { createSign } from 'node:crypto'; +import { readFile } from 'node:fs/promises'; +import { pathToFileURL } from 'node:url'; + +const API_VERSION = '2022-11-28'; +const POLICY_CONTEXT = 'ontrack/review-policy'; +const APP_BOT_LOGIN = 'ontrack-review-policy-t2-2026[bot]'; +const PAGE_SIZE = 100; +const MAX_PAGES = 50; +const ALLOWED_REPOSITORIES = new Set([ + 'doubtfire-deploy', + 'doubtfire-api', + 'doubtfire-web', +]); + +function normalizeLogin(login) { + return String(login || '').toLowerCase(); +} + +function encodeJson(value) { + return Buffer.from(JSON.stringify(value)).toString('base64url'); +} + +export function createAppJwt(appId, privateKey, nowSeconds = Math.floor(Date.now() / 1000)) { + if (!/^\d+$/.test(String(appId))) { + throw new Error('ONTRACK_REVIEW_APP_ID must be a numeric GitHub App ID.'); + } + if (!String(privateKey).includes('PRIVATE KEY')) { + throw new Error('ONTRACK_REVIEW_APP_PRIVATE_KEY is missing or invalid.'); + } + + const header = encodeJson({ alg: 'RS256', typ: 'JWT' }); + const payload = encodeJson({ + iat: nowSeconds - 60, + exp: nowSeconds + 540, + iss: String(appId), + }); + const unsigned = `${header}.${payload}`; + const signer = createSign('RSA-SHA256'); + signer.update(unsigned); + signer.end(); + const signature = signer.sign(privateKey).toString('base64url'); + return `${unsigned}.${signature}`; +} + +export function approvedReviewers(reviews, headSha, authorLogin) { + const author = normalizeLogin(authorLogin); + const latestActionableReview = new Map(); + const actionableStates = new Set(['APPROVED', 'CHANGES_REQUESTED', 'DISMISSED']); + + const ordered = [...reviews].sort((left, right) => { + const leftTime = Date.parse(left.submitted_at || 0) || 0; + const rightTime = Date.parse(right.submitted_at || 0) || 0; + return leftTime - rightTime || Number(left.id || 0) - Number(right.id || 0); + }); + + for (const review of ordered) { + const login = normalizeLogin(review.user?.login); + const state = String(review.state || '').toUpperCase(); + if (!login || login === author || review.user?.type === 'Bot') { + continue; + } + // A comment after an approval does not revoke the approval. + if (!actionableStates.has(state)) { + continue; + } + latestActionableReview.set(login, review); + } + + return new Set( + [...latestActionableReview.entries()] + .filter(([, review]) => ( + String(review.state || '').toUpperCase() === 'APPROVED' + && review.commit_id === headSha + )) + .map(([login]) => login), + ); +} + +export function evaluatePolicy(approved, leadMembers, contributorMembers) { + const leads = new Set([...leadMembers].map(normalizeLogin)); + const contributors = new Set([...contributorMembers].map(normalizeLogin)); + let leadApprovals = 0; + let contributorApprovals = 0; + + for (const login of approved) { + const normalized = normalizeLogin(login); + if (leads.has(normalized)) { + leadApprovals += 1; + } + if (contributors.has(normalized)) { + contributorApprovals += 1; + } + } + + return { + leadApprovals, + contributorApprovals, + passes: leadApprovals >= 1 || contributorApprovals >= 2, + }; +} + +export function pullRequestNumbersFromWorkflowRun(workflowRun) { + const numbers = new Set(); + for (const pullRequest of workflowRun?.pull_requests || []) { + const number = Number(pullRequest?.number); + if (Number.isSafeInteger(number) && number > 0) { + numbers.add(number); + } + } + + const title = String(workflowRun?.display_title || ''); + const titleMatch = title.match(/\bPR #([1-9]\d{0,9})\b/); + if (titleMatch) { + const number = Number(titleMatch[1]); + if (Number.isSafeInteger(number)) { + numbers.add(number); + } + } + return [...numbers]; +} + +function safeError(error) { + return String(error?.message || error || 'Unknown error') + .replace(/gh[opsu]_[A-Za-z0-9_]+/g, '[redacted token]') + .replace( + /-----BEGIN [^-]*PRIVATE KEY-----[\s\S]*?-----END [^-]*PRIVATE KEY-----/g, + '[redacted private key]', + ) + .slice(0, 500); +} + +function repositoryParts(repository) { + const match = String(repository || '').match(/^([A-Za-z0-9_.-]+)\/([A-Za-z0-9_.-]+)$/); + if (!match) { + throw new Error('GITHUB_REPOSITORY is invalid.'); + } + return { owner: match[1], repo: match[2] }; +} + +class GitHubApi { + constructor(apiUrl, token) { + this.apiUrl = String(apiUrl || 'https://api.github.com').replace(/\/$/, ''); + this.token = token; + } + + async request(path, { method = 'GET', body, expected = [200] } = {}) { + const response = await fetch(`${this.apiUrl}${path}`, { + method, + headers: { + Accept: 'application/vnd.github+json', + Authorization: `Bearer ${this.token}`, + 'User-Agent': 'ontrack-review-policy', + 'X-GitHub-Api-Version': API_VERSION, + }, + body: body === undefined ? undefined : JSON.stringify(body), + }); + + if (!expected.includes(response.status)) { + const requestId = response.headers.get('x-github-request-id'); + throw new Error( + `GitHub API ${method} ${path} returned ${response.status}` + + (requestId ? ` (request ${requestId})` : ''), + ); + } + + if (response.status === 204) { + return null; + } + const text = await response.text(); + return text ? JSON.parse(text) : null; + } + + async paginate(path) { + const items = []; + const separator = path.includes('?') ? '&' : '?'; + for (let page = 1; page <= MAX_PAGES; page += 1) { + const batch = await this.request( + `${path}${separator}per_page=${PAGE_SIZE}&page=${page}`, + ); + if (!Array.isArray(batch)) { + throw new Error(`Expected a list from GitHub API path ${path}.`); + } + items.push(...batch); + if (batch.length < PAGE_SIZE) { + return items; + } + } + throw new Error(`GitHub API pagination exceeded ${MAX_PAGES} pages for ${path}.`); + } +} + +async function mintInstallationToken({ apiUrl, owner, repo, appId, privateKey }) { + const appJwt = createAppJwt(appId, privateKey); + const appApi = new GitHubApi(apiUrl, appJwt); + const installation = await appApi.request( + `/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/installation`, + ); + const tokenResponse = await appApi.request( + `/app/installations/${installation.id}/access_tokens`, + { + method: 'POST', + expected: [201], + body: { + repositories: [repo], + permissions: { + members: 'read', + pull_requests: 'read', + statuses: 'write', + }, + }, + }, + ); + + if (!tokenResponse?.token) { + throw new Error('GitHub did not return an installation access token.'); + } + // Generated tokens are not repository secrets, so mask them explicitly. + console.log(`::add-mask::${tokenResponse.token}`); + return tokenResponse.token; +} + +async function teamMembers(api, owner, teamSlug) { + const members = await api.paginate( + `/orgs/${encodeURIComponent(owner)}/teams/${encodeURIComponent(teamSlug)}/members`, + ); + return new Set(members.map((member) => normalizeLogin(member.login))); +} + +async function openPullRequests(api, owner, repo) { + return api.paginate( + `/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/pulls?state=open`, + ); +} + +async function pullRequest(api, owner, repo, number) { + return api.request( + `/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/pulls/${number}`, + ); +} + +async function reviewsForPullRequest(api, owner, repo, number) { + return api.paginate( + `/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/pulls/${number}/reviews`, + ); +} + +function runUrl(repository, runId) { + return `https://github.com/${repository}/actions/runs/${runId}`; +} + +async function setPolicyStatus(api, owner, repo, sha, state, description, targetUrl) { + if (!/^[0-9a-f]{40}$/i.test(String(sha || ''))) { + throw new Error('Cannot publish the review policy without a valid commit SHA.'); + } + const clippedDescription = description.slice(0, 140); + try { + const statuses = await api.paginate( + `/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}` + + `/commits/${encodeURIComponent(sha)}/statuses`, + ); + const latest = statuses.find((status) => ( + status.context === POLICY_CONTEXT + && normalizeLogin(status.creator?.login) === APP_BOT_LOGIN + )); + if (latest?.state === state && latest?.description === clippedDescription) { + return false; + } + } catch (error) { + // Deduplication is only an optimization. Always attempt the fail-closed write + // when status history cannot be read but the status endpoint may still work. + console.warn(`::warning::Status deduplication failed: ${safeError(error)}`); + } + + await api.request( + `/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/statuses/${sha}`, + { + method: 'POST', + expected: [201], + body: { + state, + context: POLICY_CONTEXT, + description: clippedDescription, + target_url: targetUrl, + }, + }, + ); + return true; +} + +export { setPolicyStatus }; + +// Report on the pull-request head. GitHub gates on the test merge commit whenever that +// commit carries a status and only falls back to the head when it carries none, and the +// test merge commit carries none of this repository's checks. Reporting there would move +// the whole merge gate onto a commit that CI never sees. The head is also stable while +// the test merge commit is recomputed every time the base branch moves. +export function statusShaForPullRequest(pullRequestToReport) { + return pullRequestToReport.head?.sha; +} + +function reviewDigest(reviews) { + return reviews + .map((review) => ( + `${review.id}:${review.state}:${review.commit_id}:` + + `${review.submitted_at}:${review.user?.id}:${review.user?.login}` + )) + .sort() + .join('|'); +} + +function samePullRequestVersion(left, right) { + return ( + left.state === right.state + && left.draft === right.draft + && left.head?.sha === right.head?.sha + && left.base?.ref === right.base?.ref + && left.base?.sha === right.base?.sha + && left.mergeable === right.mergeable + ); +} + +async function evaluatePullRequest({ + api, + owner, + repo, + pullRequest: current, + leads, + contributors, + targetUrl, + attempt = 0, +}) { + // Re-fetch before evaluating so a delayed workflow_run never trusts its event's + // old head or merge SHA. + current = await pullRequest(api, owner, repo, current.number); + if (current.state !== 'open') { + return; + } + + const statusSha = statusShaForPullRequest(current); + if (current.draft) { + await setPolicyStatus( + api, + owner, + repo, + statusSha, + 'pending', + 'Waiting for the pull request to be marked ready for review', + targetUrl, + ); + console.log(`PR #${current.number}: draft`); + return; + } + + const firstReviews = await reviewsForPullRequest(api, owner, repo, current.number); + const checked = await pullRequest(api, owner, repo, current.number); + const secondReviews = await reviewsForPullRequest(api, owner, repo, current.number); + const live = await pullRequest(api, owner, repo, current.number); + if (checked.state !== 'open' || live.state !== 'open') { + return; + } + const liveStatusSha = statusShaForPullRequest(live); + if ( + !samePullRequestVersion(current, checked) + || !samePullRequestVersion(checked, live) + || reviewDigest(firstReviews) !== reviewDigest(secondReviews) + || liveStatusSha !== statusSha + ) { + if (attempt >= 1) { + throw new Error('Pull request changed repeatedly during evaluation.'); + } + console.log(`PR #${current.number}: changed during evaluation; retrying once`); + return evaluatePullRequest({ + api, + owner, + repo, + pullRequest: live, + leads, + contributors, + targetUrl, + attempt: attempt + 1, + }); + } + + const approved = approvedReviewers( + secondReviews, + live.head.sha, + live.user?.login, + ); + const result = evaluatePolicy(approved, leads, contributors); + const state = result.passes ? 'success' : 'pending'; + const description = result.passes + ? `Passed: ${result.leadApprovals}/1 lead or ${result.contributorApprovals}/2 contributors` + : `Waiting: ${result.leadApprovals}/1 lead or ${result.contributorApprovals}/2 contributors`; + + await setPolicyStatus(api, owner, repo, liveStatusSha, state, description, targetUrl); + console.log( + `PR #${current.number}: lead=${result.leadApprovals}, ` + + `contributors=${result.contributorApprovals}, status=${state}`, + ); +} + +async function eventPayload() { + const payloadPath = process.env.GITHUB_EVENT_PATH; + if (!payloadPath) { + return {}; + } + return JSON.parse(await readFile(payloadPath, 'utf8')); +} + +async function pullRequestsToEvaluate(api, owner, repo, eventName, payload) { + const open = await openPullRequests(api, owner, repo); + if (eventName === 'workflow_run') { + // Treat workflow_run fields only as untrusted locators. Match them against + // live open PRs fetched with the App token before evaluating anything. + const numbers = new Set(pullRequestNumbersFromWorkflowRun(payload.workflow_run)); + const headSha = payload.workflow_run?.head_sha; + const linked = open.filter((candidate) => ( + numbers.has(candidate.number) + || candidate.head?.sha === headSha + || candidate.merge_commit_sha === headSha + )); + return linked.length > 0 ? linked : open; + } + return open; +} + +export async function main() { + const repository = process.env.GITHUB_REPOSITORY; + const { owner, repo } = repositoryParts(repository); + if (owner !== 'ontrack-features-t2-2026' || !ALLOWED_REPOSITORIES.has(repo)) { + throw new Error('This evaluator only runs for the three approved OnTrack repositories.'); + } + + const appId = process.env.ONTRACK_REVIEW_APP_ID; + const privateKey = process.env.ONTRACK_REVIEW_APP_PRIVATE_KEY; + const apiUrl = process.env.GITHUB_API_URL || 'https://api.github.com'; + const token = await mintInstallationToken({ + apiUrl, + owner, + repo, + appId, + privateKey, + }); + const api = new GitHubApi(apiUrl, token); + const payload = await eventPayload(); + const eventName = process.env.GITHUB_EVENT_NAME || ''; + const pullRequests = await pullRequestsToEvaluate(api, owner, repo, eventName, payload); + + if (pullRequests.length === 0) { + console.log('No open pull requests require review-policy evaluation.'); + return; + } + + const leadTeam = process.env.ONTRACK_LEAD_TEAM || 'ontrack-leads'; + const contributorTeam = process.env.ONTRACK_CONTRIBUTOR_TEAM || 'ontrack-contributors'; + let leads; + let contributors; + try { + [leads, contributors] = await Promise.all([ + teamMembers(api, owner, leadTeam), + teamMembers(api, owner, contributorTeam), + ]); + } catch (error) { + const targetUrl = runUrl(repository, process.env.GITHUB_RUN_ID); + for (const current of pullRequests) { + try { + const sha = statusShaForPullRequest(current); + await setPolicyStatus( + api, + owner, + repo, + sha, + 'error', + 'OnTrack team membership could not be verified', + targetUrl, + ); + } catch (statusError) { + console.error(`::error::${safeError(statusError)}`); + } + } + throw error; + } + + const targetUrl = runUrl(repository, process.env.GITHUB_RUN_ID); + const failures = []; + for (const current of pullRequests) { + try { + await evaluatePullRequest({ + api, + owner, + repo, + pullRequest: current, + leads, + contributors, + targetUrl, + }); + } catch (error) { + failures.push(error); + try { + const sha = statusShaForPullRequest(current); + await setPolicyStatus( + api, + owner, + repo, + sha, + 'error', + 'OnTrack review policy evaluation failed', + targetUrl, + ); + } catch (statusError) { + console.error(`::error::${safeError(statusError)}`); + } + console.error(`::error::PR #${current.number}: ${safeError(error)}`); + } + } + + if (failures.length > 0) { + throw new Error(`${failures.length} pull-request evaluation(s) failed.`); + } +} + +const invokedPath = process.argv[1] ? pathToFileURL(process.argv[1]).href : ''; +if (import.meta.url === invokedPath) { + main().catch((error) => { + console.error(`::error::${safeError(error)}`); + process.exitCode = 1; + }); +} diff --git a/.github/review-policy/evaluate.test.mjs b/.github/review-policy/evaluate.test.mjs new file mode 100644 index 00000000..0c093524 --- /dev/null +++ b/.github/review-policy/evaluate.test.mjs @@ -0,0 +1,236 @@ +import assert from 'node:assert/strict'; +import { generateKeyPairSync, verify } from 'node:crypto'; +import { test } from 'node:test'; + +import { + approvedReviewers, + createAppJwt, + evaluatePolicy, + pullRequestNumbersFromWorkflowRun, + setPolicyStatus, + statusShaForPullRequest, +} from './evaluate.mjs'; + +function review({ + id, + login, + state = 'APPROVED', + commit = 'head', + submitted = `2026-08-24T00:00:${String(id).padStart(2, '0')}Z`, + type = 'User', +}) { + return { + id, + state, + commit_id: commit, + submitted_at: submitted, + user: { login, type }, + }; +} + +test('one lead approval passes', () => { + const result = evaluatePolicy( + new Set(['lead']), + new Set(['lead']), + new Set(['lead', 'contributor']), + ); + assert.equal(result.passes, true); + assert.equal(result.leadApprovals, 1); +}); + +test('two distinct contributor approvals pass', () => { + const result = evaluatePolicy( + new Set(['contributor-a', 'contributor-b']), + new Set(['lead']), + new Set(['lead', 'contributor-a', 'contributor-b']), + ); + assert.equal(result.passes, true); + assert.equal(result.contributorApprovals, 2); +}); + +test('one contributor approval does not pass', () => { + const result = evaluatePolicy( + new Set(['contributor-a']), + new Set(['lead']), + new Set(['lead', 'contributor-a']), + ); + assert.equal(result.passes, false); +}); + +test('duplicate approvals from one reviewer count once', () => { + const approved = approvedReviewers([ + review({ id: 1, login: 'Contributor-A' }), + review({ id: 2, login: 'contributor-a' }), + ], 'head', 'author'); + assert.deepEqual([...approved], ['contributor-a']); +}); + +test('a later comment does not revoke an approval', () => { + const approved = approvedReviewers([ + review({ id: 1, login: 'contributor-a' }), + review({ id: 2, login: 'contributor-a', state: 'COMMENTED' }), + ], 'head', 'author'); + assert.deepEqual([...approved], ['contributor-a']); +}); + +test('a later changes-requested review revokes an approval', () => { + const approved = approvedReviewers([ + review({ id: 1, login: 'contributor-a' }), + review({ id: 2, login: 'contributor-a', state: 'CHANGES_REQUESTED' }), + ], 'head', 'author'); + assert.deepEqual([...approved], []); +}); + +test('stale, author, and bot approvals are ignored', () => { + const approved = approvedReviewers([ + review({ id: 1, login: 'stale', commit: 'old-head' }), + review({ id: 2, login: 'author' }), + review({ id: 3, login: 'review-bot[bot]', type: 'Bot' }), + ], 'head', 'author'); + assert.deepEqual([...approved], []); +}); + +test('workflow run PR numbers are deduplicated and validated', () => { + assert.deepEqual( + pullRequestNumbersFromWorkflowRun({ + display_title: 'OnTrack review policy signal for PR #42', + pull_requests: [{ number: 42 }, { number: 17 }, { number: 0 }], + }), + [42, 17], + ); +}); + +test('workflow run ignores unsafe or implausibly large PR numbers', () => { + assert.deepEqual( + pullRequestNumbersFromWorkflowRun({ + display_title: 'OnTrack review policy signal for PR #12345678901', + pull_requests: [{ number: Number.MAX_SAFE_INTEGER + 1 }, { number: -4 }], + }), + [], + ); +}); + +test('GitHub App JWT has a valid RSA signature and bounded lifetime', () => { + const { privateKey, publicKey } = generateKeyPairSync('rsa', { modulusLength: 2048 }); + const privateKeyPem = privateKey.export({ type: 'pkcs8', format: 'pem' }); + const now = 1_800_000_000; + const jwt = createAppJwt('4699573', privateKeyPem, now); + const [header, payload, signature] = jwt.split('.'); + assert.equal( + verify( + 'RSA-SHA256', + Buffer.from(`${header}.${payload}`), + publicKey, + Buffer.from(signature, 'base64url'), + ), + true, + ); + const claims = JSON.parse(Buffer.from(payload, 'base64url').toString('utf8')); + assert.equal(claims.iss, '4699573'); + assert.equal(claims.iat, now - 60); + assert.equal(claims.exp, now + 540); +}); + +test('unchanged App status is not republished and spoofed sources are ignored', async () => { + const posts = []; + const api = { + async paginate() { + return [ + { + context: 'ontrack/review-policy', + state: 'success', + description: 'Passed', + creator: { login: 'not-the-policy-app[bot]' }, + }, + { + context: 'ontrack/review-policy', + state: 'pending', + description: 'Waiting', + creator: { login: 'ontrack-review-policy-t2-2026[bot]' }, + }, + ]; + }, + async request(path, options) { + posts.push({ path, options }); + return {}; + }, + }; + + assert.equal( + await setPolicyStatus( + api, + 'owner', + 'repo', + 'a'.repeat(40), + 'pending', + 'Waiting', + 'https://example.test/run', + ), + false, + ); + assert.equal(posts.length, 0); + + assert.equal( + await setPolicyStatus( + api, + 'owner', + 'repo', + 'a'.repeat(40), + 'success', + 'Passed', + 'https://example.test/run', + ), + true, + ); + assert.equal(posts.length, 1); +}); + +test('status-history failure does not suppress a fail-closed write', async () => { + const posts = []; + const warnings = []; + const originalWarn = console.warn; + console.warn = (message) => warnings.push(message); + const api = { + async paginate() { + throw new Error('history unavailable'); + }, + async request(path, options) { + posts.push({ path, options }); + return {}; + }, + }; + + try { + assert.equal( + await setPolicyStatus( + api, + 'owner', + 'repo', + 'b'.repeat(40), + 'error', + 'Evaluation failed', + 'https://example.test/run', + ), + true, + ); + assert.equal(posts.length, 1); + assert.equal(posts[0].options.body.state, 'error'); + assert.equal(warnings.length, 1); + } finally { + console.warn = originalWarn; + } +}); + +test('the status is reported on the head, never on the test merge commit', () => { + assert.equal( + statusShaForPullRequest({ head: { sha: 'head' }, merge_commit_sha: 'test-merge' }), + 'head', + ); +}); + +test('a conflicting pull request with no test merge commit still reports on its head', () => { + assert.equal( + statusShaForPullRequest({ head: { sha: 'head' }, merge_commit_sha: null }), + 'head', + ); +}); diff --git a/.github/workflows/notify-teams-pr.yml b/.github/workflows/notify-teams-pr.yml new file mode 100644 index 00000000..164a85b2 --- /dev/null +++ b/.github/workflows/notify-teams-pr.yml @@ -0,0 +1,260 @@ +# NOTE ON THE BASE BRANCH. GitHub loads a `pull_request_target` workflow from the +# repository DEFAULT branch (11.0.x), not from the pull request's base branch. That +# changed on 2025-12-08. So this file is merged into 11.0.x deliberately, against the +# usual CONTRIBUTING rule, and a copy on feature/notifications would be dead code. +# It fires for pull requests into every base branch, which is what we want. +name: Notify Teams when a pull request opens + +on: + pull_request_target: + types: + - opened + - reopened + - ready_for_review + # - review_requested # matches "reviewer alerts" most directly, but adds one + # # message per requested reviewer. Decide, do not default. + +# Removes all GITHUB_TOKEN scopes, which this workflow does not need. It does NOT +# restrict secrets.* - the webhook is protected by the guards below, not by this. +permissions: {} + +jobs: + notify-teams: + name: Post pull request notification to Teams + # The job guard makes the file inert if it ever travels to thoth-tech or + # doubtfire-lms. The step below decides who gets a notification: a pull request + # from a branch in this repository always does, and a fork pull request only if + # its author is in one of the two OnTrack teams. + if: github.repository_owner == 'ontrack-features-t2-2026' + runs-on: ubuntu-latest + timeout-minutes: 2 + + steps: + - name: Check pull request author team membership + id: team-membership + shell: bash + env: + TEAM_MEMBERSHIP_TOKEN: ${{ secrets.TEAM_MEMBERSHIP_ACCESS }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + PR_HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} + REPOSITORY: ${{ github.repository }} + run: | + set -euo pipefail + + # A branch in this repository can only be pushed by someone who already + # has write access, so those pull requests need no lookup. That is also + # the only reason Dependabot notifies: dependabot[bot] is in neither + # team and 404s on both. The team check below is for fork pull requests, + # which is where the webhook actually needed protecting. + if [[ "${PR_HEAD_REPO}" == "${REPOSITORY}" ]]; then + echo "eligible=true" >> "${GITHUB_OUTPUT}" + exit 0 + fi + + if [[ -z "${TEAM_MEMBERSHIP_TOKEN:-}" ]]; then + echo "::error::The TEAM_MEMBERSHIP_ACCESS secret is not configured." + exit 1 + fi + + # The login goes into a URL path segment, so percent-encode it. curl also + # reads [ ] { } * in a URL as a glob and exits before sending anything, so + # --globoff below. Between them a login like dependabot[bot] is safe. + author_encoded="$(python3 -c 'import os, urllib.parse; print(urllib.parse.quote(os.environ["PR_AUTHOR"], safe=""))')" + + for team in ontrack-contributors ontrack-leads; do + http_status="$( + curl \ + --proto '=https' \ + --tlsv1.2 \ + --globoff \ + --silent \ + --show-error \ + --connect-timeout 10 \ + --max-time 30 \ + --header 'Accept: application/vnd.github+json' \ + --header "Authorization: Bearer ${TEAM_MEMBERSHIP_TOKEN}" \ + --header 'X-GitHub-Api-Version: 2022-11-28' \ + --output "${RUNNER_TEMP}/team-membership.json" \ + --write-out '%{http_code}' \ + "https://api.github.com/orgs/ontrack-features-t2-2026/teams/${team}/memberships/${author_encoded}" \ + )" || http_status="000" + + case "${http_status}" in + 200) + membership_state="$(python3 -c 'import json,sys; print(json.load(sys.stdin)["state"])' \ + < "${RUNNER_TEMP}/team-membership.json")" + # "pending" is a teammate who has not accepted the org invitation + # yet. They are on the team and they review, so notify them too. + if [[ "${membership_state}" == "active" || "${membership_state}" == "pending" ]]; then + echo "eligible=true" >> "${GITHUB_OUTPUT}" + exit 0 + fi + ;; + 404) ;; + *) + echo "::error::GitHub returned HTTP ${http_status} while checking team membership." + exit 1 + ;; + esac + done + + # A skip is a decision, so say so on the run summary. Without this the job + # goes green and looks the same as one that posted a message. + echo "::warning::No Teams notification sent. ${PR_AUTHOR} opened this pull request from a fork and is in neither ontrack-contributors nor ontrack-leads." + echo "eligible=false" >> "${GITHUB_OUTPUT}" + + - name: Build and send Teams notification + if: steps.team-membership.outputs.eligible == 'true' + shell: bash + env: + TEAMS_WEBHOOK_URL: ${{ secrets.TEAMS_PR_WEBHOOK_URL }} + PAYLOAD_PATH: ${{ runner.temp }}/teams-pr-notification.json + + REPOSITORY: ${{ github.repository }} + PR_ACTION: ${{ github.event.action }} + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_TITLE: ${{ github.event.pull_request.title }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + PR_URL: ${{ github.event.pull_request.html_url }} + PR_DRAFT: ${{ github.event.pull_request.draft }} + PR_HEAD: ${{ github.event.pull_request.head.label }} + PR_BASE: ${{ github.event.pull_request.base.ref }} + + run: | + set -euo pipefail + + if [[ -z "${TEAMS_WEBHOOK_URL:-}" ]]; then + echo "::error::The TEAMS_PR_WEBHOOK_URL Actions secret is not configured." + exit 1 + fi + + python3 - <<'PY' + import json + import os + import re + + + def plain(value): + """Teams renders these fields as Markdown, and titles and branch names + come from anyone who can open a pull request. Removing [ ] < > and + backticks stops a title forging a Markdown link or code span. + Parentheses are left alone so `feat(scope): ...` still reads + properly. Note this does NOT stop Teams auto-linking a bare URL in + a title, it only stops the link TEXT being controlled.""" + value = " ".join(value.split()) + return re.sub(r"[\[\]<>`]", " ", value)[:200] + + + headline = { + "opened": "New pull request opened", + "reopened": "Pull request reopened", + "ready_for_review": "Pull request is ready for review", + }.get(os.environ["PR_ACTION"], "Pull request updated") + + status = ( + "Draft" + if os.environ.get("PR_DRAFT", "").lower() == "true" + else "Ready for review" + ) + + card = { + "$schema": "http://adaptivecards.io/schemas/adaptive-card.json", + "type": "AdaptiveCard", + "version": "1.2", + "body": [ + { + "type": "TextBlock", + "size": "Medium", + "weight": "Bolder", + "wrap": True, + "text": headline, + }, + { + "type": "TextBlock", + "wrap": True, + "text": f"PR #{os.environ['PR_NUMBER']}: {plain(os.environ['PR_TITLE'])}", + }, + { + "type": "FactSet", + "facts": [ + { + "title": "Repository", + "value": plain(os.environ["REPOSITORY"]), + }, + { + "title": "Author", + "value": plain(os.environ["PR_AUTHOR"]), + }, + { + "title": "Status", + "value": status, + }, + { + "title": "Branches", + "value": ( + f"{plain(os.environ['PR_HEAD'])} -> " + f"{plain(os.environ['PR_BASE'])}" + ), + }, + ], + }, + ], + "actions": [ + { + "type": "Action.OpenUrl", + "title": "Open pull request", + "url": os.environ["PR_URL"], + } + ], + } + + payload = { + "type": "message", + "attachments": [ + { + "contentType": "application/vnd.microsoft.card.adaptive", + "contentUrl": None, + "content": card, + } + ], + } + + with open( + os.environ["PAYLOAD_PATH"], + "w", + encoding="utf-8", + ) as payload_file: + json.dump( + payload, + payload_file, + ensure_ascii=False, + ) + PY + + http_status="$( + curl \ + --proto '=https' \ + --tlsv1.2 \ + --silent \ + --show-error \ + --connect-timeout 10 \ + --max-time 30 \ + --header 'Content-Type: application/json' \ + --data-binary "@${PAYLOAD_PATH}" \ + --output "${RUNNER_TEMP}/teams-response.txt" \ + --write-out '%{http_code}' \ + --url "${TEAMS_WEBHOOK_URL}" + )" || http_status="000" + + # Uncomment to debug a failing webhook. The body comes from a third party + # and lands in a PUBLIC Actions log, and GitHub only masks an exact + # full-value match of a secret. Read it once, then comment it out again. + # cat "${RUNNER_TEMP}/teams-response.txt" + + if [[ "${http_status}" != 2* ]]; then + echo "::error::Teams webhook returned HTTP ${http_status}." + exit 1 + fi + + echo "Teams webhook returned HTTP ${http_status} for PR #${PR_NUMBER}." + echo "That means the request was accepted. It does not prove the message rendered in the channel." diff --git a/.github/workflows/ontrack-review-policy-signal.yml b/.github/workflows/ontrack-review-policy-signal.yml new file mode 100644 index 00000000..d87949cb --- /dev/null +++ b/.github/workflows/ontrack-review-policy-signal.yml @@ -0,0 +1,32 @@ +name: OnTrack review policy signal +run-name: "OnTrack review policy signal for PR #${{ github.event.pull_request.number || 'all' }}" + +on: + pull_request_target: + types: + - opened + - reopened + - synchronize + - edited + - ready_for_review + - converted_to_draft + pull_request_review: + types: + - submitted + - edited + - dismissed + workflow_dispatch: + +# This workflow deliberately has no permissions and never checks out pull-request +# code or secrets. Its completion wakes the trusted evaluator on the default +# branch; manual runs safely reconcile every open pull request. +permissions: {} + +jobs: + signal: + if: github.repository_owner == 'ontrack-features-t2-2026' + runs-on: ubuntu-24.04 + timeout-minutes: 1 + steps: + - name: Signal policy evaluation + run: ':' diff --git a/.github/workflows/ontrack-review-policy.yml b/.github/workflows/ontrack-review-policy.yml new file mode 100644 index 00000000..61c194b1 --- /dev/null +++ b/.github/workflows/ontrack-review-policy.yml @@ -0,0 +1,56 @@ +name: OnTrack review policy evaluator + +on: + workflow_run: + workflows: + - OnTrack review policy signal + types: + - completed + push: + branches: + - 11.0.x + schedule: + # Reconcile team membership and base-branch changes even when no PR event fires. + - cron: '3-58/5 * * * *' + +# The repository token is used only to check out the trusted evaluator from the +# default branch. The GitHub App token is separately scoped to member/PR reads and +# commit-status writes. +permissions: + contents: read + +concurrency: + # Serialize signals for the same PR without allowing unrelated PR activity to + # replace a queued revocation. Scheduled and base-push reconciliations share a + # separate group. Every run re-reads authoritative GitHub state. + group: ontrack-review-policy-${{ github.repository }}-${{ github.event.workflow_run.pull_requests[0].number || github.event.workflow_run.display_title || 'reconcile' }} + cancel-in-progress: false + +jobs: + evaluate: + if: github.repository_owner == 'ontrack-features-t2-2026' + runs-on: ubuntu-24.04 + timeout-minutes: 5 + environment: + name: ontrack-review-policy + deployment: false + + steps: + - name: Check out the trusted policy evaluator + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + with: + ref: refs/heads/11.0.x + fetch-depth: 1 + persist-credentials: false + sparse-checkout: .github/review-policy + + - name: Verify policy logic + run: node --test .github/review-policy/evaluate.test.mjs + + - name: Evaluate current pull-request approvals + env: + ONTRACK_REVIEW_APP_ID: ${{ vars.ONTRACK_REVIEW_APP_ID }} + ONTRACK_REVIEW_APP_PRIVATE_KEY: ${{ secrets.ONTRACK_REVIEW_APP_PRIVATE_KEY }} + ONTRACK_CONTRIBUTOR_TEAM: ontrack-contributors + ONTRACK_LEAD_TEAM: ontrack-leads + run: node .github/review-policy/evaluate.mjs diff --git a/.github/workflows/required-ci.yml b/.github/workflows/required-ci.yml new file mode 100644 index 00000000..e9e69fc1 --- /dev/null +++ b/.github/workflows/required-ci.yml @@ -0,0 +1,44 @@ +name: Required CI + +on: + pull_request: {} + push: + branches: + - 11.0.x + workflow_dispatch: {} + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + validate: + name: deploy-validation + runs-on: ubuntu-latest + timeout-minutes: 5 + + steps: + - name: Check out repository + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + + - name: Reject whitespace errors + run: git diff --check + + - name: Check shell syntax + run: git ls-files -z '*.sh' | xargs -0 -n1 bash -n + + - name: Validate development Compose configurations + run: | + docker compose -f development/docker-compose.yml config --quiet + docker compose -f development/docker-compose.yml -f development/docker-compose.full.yml config --quiet + docker compose -f development/docker-compose.yml -f development/docker-compose.local-paths.yml config --quiet + docker compose -f development/docker-compose.yml -f development/docker-compose.podman.yml config --quiet + + - name: Validate dev-container Compose configuration + run: docker compose -f .devcontainer/docker-compose.yml config --quiet + + - name: Validate production Compose configuration + run: docker compose -f production/docker-compose.yml config --quiet diff --git a/.github/workflows/weekly-integration-prs.yml b/.github/workflows/weekly-integration-prs.yml new file mode 100644 index 00000000..ef5031ce --- /dev/null +++ b/.github/workflows/weekly-integration-prs.yml @@ -0,0 +1,98 @@ +name: Weekly integration PRs + +on: + schedule: + - cron: "17 9 * * 1" + timezone: Australia/Melbourne + workflow_dispatch: + +permissions: + contents: read + +jobs: + ensure-pr: + if: github.repository_owner == 'ontrack-features-t2-2026' + name: ${{ matrix.source }} -> ${{ matrix.target }} + runs-on: ubuntu-24.04 + timeout-minutes: 10 + + strategy: + fail-fast: false + max-parallel: 3 + matrix: + include: + - source: config/ppi-production-values-20260824 + target: 11.0.x + - source: fix/production-ready-compose-20260824 + target: 11.0.x + - source: integration/deploy-all-features-foundation-20260824 + target: 11.0.x + + concurrency: + group: ${{ github.workflow }}-${{ matrix.target }}-${{ matrix.source }} + cancel-in-progress: false + + steps: + - name: Ensure pull request exists + shell: bash + env: + GH_TOKEN: ${{ secrets.INTEGRATION_BOT_TOKEN }} + SOURCE_BRANCH: ${{ matrix.source }} + TARGET_BRANCH: ${{ matrix.target }} + run: | + set -euo pipefail + + if [[ -z "${GH_TOKEN:-}" ]]; then + echo "::error::INTEGRATION_BOT_TOKEN is not configured." + exit 1 + fi + + existing="$( + gh api --method GET "repos/$GITHUB_REPOSITORY/pulls" \ + -H "Accept: application/vnd.github+json" \ + -H "X-GitHub-Api-Version: 2026-03-10" \ + -f state=open \ + -f head="${GITHUB_REPOSITORY_OWNER}:${SOURCE_BRANCH}" \ + -f base="$TARGET_BRANCH" \ + -F per_page=1 \ + --jq '.[0].html_url // empty' + )" + + if [[ -n "$existing" ]]; then + echo "Pull request already open: $existing" + exit 0 + fi + + target_ref="$(jq -rn --arg ref "$TARGET_BRANCH" '$ref | @uri')" + source_ref="$(jq -rn --arg ref "$SOURCE_BRANCH" '$ref | @uri')" + + for ref in "$target_ref" "$source_ref"; do + if ! gh api "repos/$GITHUB_REPOSITORY/branches/${ref}" \ + -H "Accept: application/vnd.github+json" \ + -H "X-GitHub-Api-Version: 2026-03-10" \ + --silent >/dev/null 2>&1; then + echo "Branch $ref no longer exists in $GITHUB_REPOSITORY; skipping." + exit 0 + fi + done + comparison="$( + gh api "repos/$GITHUB_REPOSITORY/compare/${target_ref}...${source_ref}" \ + -H "Accept: application/vnd.github+json" \ + -H "X-GitHub-Api-Version: 2026-03-10" \ + --jq '[.ahead_by, (.files | length)] | @tsv' + )" + read -r ahead changed_files <<< "$comparison" + + if [[ "$ahead" == "0" || "$changed_files" == "0" ]]; then + echo "Nothing to merge from $SOURCE_BRANCH into $TARGET_BRANCH." + exit 0 + fi + + gh api --method POST "repos/$GITHUB_REPOSITORY/pulls" \ + -H "Accept: application/vnd.github+json" \ + -H "X-GitHub-Api-Version: 2026-03-10" \ + -f title="Integration: $SOURCE_BRANCH into $TARGET_BRANCH" \ + -f head="$SOURCE_BRANCH" \ + -f base="$TARGET_BRANCH" \ + -f body="Created by the scheduled integration workflow. The source branch is intentionally retained after merge." \ + --jq '.html_url' diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b704a576..c96068e3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -12,6 +12,8 @@ This guide provides high-level details on how to contribute to the Doubtfire rep - [Table of Contents](#table-of-contents) - [Getting started](#getting-started) - [Development Containers](#development-containers) + - [Common setup](#common-setup) + - [Working with Dev Containers](#working-with-dev-containers) - [Working with Docker Compose](#working-with-docker-compose) - [Forking workflow](#forking-workflow) - [About the Doubtfire Branch Structure](#about-the-doubtfire-branch-structure) @@ -27,7 +29,7 @@ This guide provides high-level details on how to contribute to the Doubtfire rep The **doubtfire-deploy** project provides the base repository containing submodules for each of the specific subprojects. - [doubtfire-api](https://github.com/doubtfire-lms/doubtfire-api) contain the backend RESTful API. This uses Rails' [active model](https://guides.rubyonrails.org/active_model_basics.html) with the [Grape REST api framework](https://github.com/ruby-grape/grape). -- [doubtfire-web](https://github.com/doubtfire-lms/doubtfire-web) hosts the frontend code implemented in [Angular](https://angular.io) and [AngularJS](https://angularjs.org). This implements the web application that connects to the backend api. +- [doubtfire-web](https://github.com/doubtfire-lms/doubtfire-web) hosts the frontend code implemented in Angular 22. This implements the web application that connects to the backend API. - [doubtfire-overseer](https://github.com/doubtfire-lms/doubtfire-overseer) provides facilities to run automated tasks on student submissions. Please get in touch with the core team if you want access to this repository. You can make contributions without access to this repository. Development of Doubtfire uses Docker containers to remove the need to install a range of native tools used within the project. The Doubtfire Deploy project helps when working across multiple components of the Doubtfire application, and is used for testing and publishing versions for deployment. @@ -41,85 +43,69 @@ There are several docker compose setups to aid in speeding up the development. - The **docker-compose.yml** file contains the most likely setup with development setups for both the api and web projets. This should be used when working on both the api and the web front end. You can run this using **run-api-web.sh**. - The **docker-compose.full.yml** contains a setup with all of the containers needed to run Doubtfire with overseer. This requires access to the overseer repository. You can run this using **run-full.sh** -### Working with Dev Containers - -This is the primary method for setting up your development enviroment: - -Pre requisittes: Vscode, Docker -OS: Windows/Linux/Mac OS +### Common setup -1. Fork [doubtfire-deploy](https://github.com/doubtfire-lms/doubtfire-deploy), [doubtfire-api](https://github.com/doubtfire-lms/doubtfire-api), and [doubtfire-web](https://github.com/doubtfire-lms/doubtfire-web) +1. Fork [doubtfire-deploy](https://github.com/doubtfire-lms/doubtfire-deploy), [doubtfire-api](https://github.com/doubtfire-lms/doubtfire-api), and [doubtfire-web](https://github.com/doubtfire-lms/doubtfire-web). - To push your contributions, you will need a fork of each repository. Contributions can then be made by making pull requests back into the main repositories. + To push your contributions, you will need a fork of each repository. Contributions can then be made by making pull requests back into the main repositories. 2. Clone your [doubtfire-deploy](https://github.com/doubtfire-lms/doubtfire-deploy). Make sure to fetch submodules to get the subprojects. - `git clone --recurse-submodules https://github.com/YOUR_USERNAME/doubtfire-deploy` + `git clone --recurse-submodules https://github.com/YOUR_USERNAME/doubtfire-deploy` -3. Open a Terminal that supports `sh` scripts (on Windows, you will need WSL, Msys2, or Cygwin). Run the following command to set your fork as the remote. +3. Open a Terminal that supports `sh` scripts (on Windows, you will need WSL, MSYS2, or Cygwin). Run the following command to set your fork as the remote. - `./change_remotes.sh` + `./change_remotes.sh` -4. In Visual studio press F1: Find Dev Containers: Open folder in Container (This will reopen the repo you cloned in a container) +4. Open a web browser and navigate to: -5. The container will automaticlly setup the DB, Frontend, Backend and your development enviroment ready for use. + - [http://localhost:3000/api/docs/](http://localhost:3000/api/docs/) to interact with the API using [Swagger](https://swagger.io). + - [http://localhost:4200](http://localhost:4200) to use the web application. -6. Open a web browser and navigate to: + The database will include a number of default users, each with password being "password". - - [http://localhost:3000/api/docs/](http://localhost:3000/api/docs/) to interact with the API using [Swagger](https://swagger.io). - - [http://localhost:4200](http://localhost:4200) to use the web application. + - Admin user: **aadmin** + - Convenor user: **aconvenor** + - Tutor user: **atutor** + - Students: **student_1** - The database will include a number of default users, each with password being "password". - - Admin user: **aadmin** - - Convenor user: **aconvenor** - - Tutor user: **atutor** - - Students: **student_1** +### Working with Dev Containers -### Working with Docker Compose +This is the primary method for setting up your development environment. -Alternative setup using Docker-Compose: +Prerequisites: VS Code, Docker +OS: Windows/Linux/macOS -1. Fork [doubtfire-deploy](https://github.com/doubtfire-lms/doubtfire-deploy), [doubtfire-api](https://github.com/doubtfire-lms/doubtfire-api), and [doubtfire-web](https://github.com/doubtfire-lms/doubtfire-web) +Follow the [Common setup](#common-setup) steps first, then: - To push your contributions, you will need a fork of each repository. Contributions can then be made by making pull requests back into the main repositories. +1. In Visual Studio Code, press F1 and select **Dev Containers: Open Folder in Container**. This will reopen the repository you cloned in a container. -2. Clone your [doubtfire-deploy](https://github.com/doubtfire-lms/doubtfire-deploy). Make sure to fetch submodules to get the subprojects. +2. The container will automatically set up the DB, frontend, backend, and your development environment ready for use. - `git clone --recurse-submodules https://github.com/YOUR_USERNAME/doubtfire-deploy` +### Working with Docker Compose -3. Open a Terminal that supports `sh` scripts (on Windows, you will need WSL, Msys2, or Cygwin). Run the following command to set your fork as the remote. +Alternative setup using Docker Compose. - `./change_remotes.sh` +Follow the [Common setup](#common-setup) steps first, then: -4. Change into the **development** directory and use [Docker Compose](https://docs.docker.com/compose/) to setup the database. +1. Change into the **development** directory and use [Docker Compose](https://docs.docker.com/compose/) to set up the database. - ```bash - cd development - docker compose run --rm doubtfire-api bash - # now in the container run... - bundle exec rails db:environment:set RAILS_ENV=development - bundle exec rake db:populate - exit - ``` + ```bash + cd development + docker compose run --rm doubtfire-api bash + # now in the container run... + bundle exec rails db:environment:set RAILS_ENV=development + bundle exec rake db:populate + exit + ``` -5. Now you can use `docker compose` to start a running environment. +2. Use `docker compose` to start a running environment. ```bash # Run in the development folder docker compose up ``` -6. Open a web browser and navigate to: - - - [http://localhost:3000/api/docs/](http://localhost:3000/api/docs/) to interact with the API using [Swagger](https://swagger.io). - - [http://localhost:4200](http://localhost:4200) to use the web application. - - The database will include a number of default users, each with password being "password". - - Admin user: **aadmin** - - Convenor user: **aconvenor** - - Tutor user: **atutor** - - Students: **student_1** - To interact with the rails console, or other rails command line applications: - Connect to a **doubtfire-api** container: @@ -134,7 +120,7 @@ Alternative setup using Docker-Compose: - Run all unit tests using: `bundle exec rails test` - Run tests from a single file: `bundle exec rails test test/models/break_test.rb` - Run a single test: `bundle exec rails test test/api/auth_test.rb:107` - - Setup the databse: + - Set up the databse: - Reset the database: `bundle exec rake db:reset db:migrate` - Migrate the database on schema changes: `bundle exec rake db:migrate` - Add a new migration: `bundle exec rails g migration migration-name` @@ -152,7 +138,7 @@ Alternative setup using Docker-Compose: Some things to know about the setup: - The containers link to `../data` as a volume to store database details, tmp files, and student work. - - If you do not gracefully terminal the api you may need to remove the `pid` file from the tmp folder. You can use `rm ../data/tmp/pids/server.pid` to do this. + - If you do not gracefully terminate the API you may need to remove the `pid` file from the tmp folder. You can use `rm ../data/tmp/pids/server.pid` to do this. - When you bring up the *doubtfire-web* project, it will run `npm install` to setup the node_modules. If you change the package.json in *doubtfire-web* you can just restart the container to update the node modules. ## Forking workflow diff --git a/DEMO.md b/DEMO.md new file mode 100644 index 00000000..ec2959a9 --- /dev/null +++ b/DEMO.md @@ -0,0 +1,214 @@ +# Demo handover — notifications + +Everything needed to run the Monday demo, for whoever is presenting. + +One branch per repo, all called `demo/notifications`. It carries the whole +notification feature: email delivery, the mail catcher, push storage, push +delivery, the service worker, and the push opt-in button. + +**Treat it as frozen.** It is a snapshot for the demo, not somewhere to work. +Review happens on the individual `email/*` and `push/*` branches, which may be +rebased. `demo/notifications` will not follow them, and that is the point. + +--- + +## Setup + +All three repos must sit side by side in one folder with their original names. +The compose file hardcodes `../../doubtfire-api` and `../../doubtfire-web`. + + ontrack/ + doubtfire-deploy/ + doubtfire-api/ + doubtfire-web/ + +Same branch in all three: + + cd ontrack/doubtfire-api && git fetch origin && git checkout demo/notifications + cd ../doubtfire-web && git fetch origin && git checkout demo/notifications + cd ../doubtfire-deploy && git fetch origin && git checkout demo/notifications + +Then, from `doubtfire-deploy/development`: + + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml up -d --build + +**`--build` is not optional.** This branch adds the `web-push` gem, and a +container built from the old image crash-loops with "Could not find +web-push-3.0.0 in locally installed gems". Both `-f` flags are required on every +compose command. + +First time only, or if the database looks wrong: + + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml run --rm doubtfire-api \ + bash -c "bundle exec rake db:populate" + +## Check it before you present + +From `doubtfire-deploy/development`: + + bash verify-notifications.sh + +Nine sections, every one should say PASS. It checks the containers, the ports, +the web-to-api proxy, mail routing, the push api, the VAPID keys, the service +worker, and it posts a real comment and confirms a real email arrives. + +If anything fails, `RUNNING-LOCALLY.md` next to this file has the fixes. + +## Addresses + +| | | +|---|---| +| Web app | http://localhost:4200 | +| Mail inbox | http://localhost:8025 | +| API docs | http://localhost:3000/api/docs | + +Every account's password is `password`. + +| Account | Role | Use for | +|---|---|---| +| `acain` | Admin, convenor of COS10001 | the staff side | +| `student_1` | student, project 2 in COS10001 | the student side | + +Sign in as one in a normal window and the other in a private window. Most of this +feature is one person doing something and another person being told about it. + +Do not use `atutor` — it teaches COS20007, not COS10001, so it cannot see the +task this demo uses. + +--- + +## The demo + +### 1. Email on a new comment + +1. As `acain`, open COS10001, project 2, task **1.1P**, and post a comment. +2. Open http://localhost:8025. The email is there within a second. +3. Open it and point out that **the comment text is not in the email.** The email + says who commented and on what, and links back to OnTrack. Content stays in + the system. + +### 2. It respects the user's preference + +1. As `student_1`, open the profile and untick **Receive notifications for new + messages**. Save. +2. As `acain`, comment again. +3. No new mail. One preference switch gates every channel — in-app, email and + push — rather than each one having its own setting. +4. Turn it back on. + +### 3. It works in both directions + +As `student_1`, reply. `acain` gets the email. The recipient is always the other +party, never the person who commented. + +### 4. Push + +1. As `student_1`, open the profile page and **wait about six seconds**. The push + button is disabled until the service worker registers and says why. +2. Click **Turn on push notifications on this device** and accept the browser + prompt. +3. Confirm it stored: + + docker exec doubtfire-api bundle exec rails runner \ + 'puts PushSubscription.all.map { |s| "#{s.user.username} #{s.endpoint[0,50]}" }' + +4. As `acain`, post a comment. A desktop notification appears, and the email + arrives at the same time. + +The point worth making: **push needed no per-event work.** The comment event was +written before push existed. Everything fans out through one service, so the day +push was added, every existing event gained it. + +--- + +## Things that will trip you up + +**Do not run `rails test` while presenting.** The test suite and the app share +one database, so tests hold locks that make the app return 500s, and they change +the seeded data. + +**Do not post the same comment text twice.** OnTrack drops a comment identical to +the previous one on that task and answers 403. No comment, no email, and nothing +on screen explains it. Vary the text. + +**Nothing pushes to a user with no subscription.** That is a silent no-op and +looks exactly like push being broken. Step 4.2 must happen first, in the same +browser you expect the notification in. + +**macOS must allow notifications from your browser** in System Settings, or step +4.4 produces nothing with no error anywhere. + +### The push says it sent but nothing appears on screen + +This is the most likely thing to go wrong, because every layer fails silently. +Work through it in this order — the first check splits the problem in half. + +**1. Is it the browser and the operating system, or is it us?** In the dev tools +console on http://localhost:4200: + +```js +const reg = await navigator.serviceWorker.ready; +await reg.showNotification('OnTrack', {body: 'local test, no server involved'}); +``` + +- **Nothing appears** — the problem is macOS or the browser, not this feature. + Go to System Settings → Notifications → your browser, turn **Allow + notifications** on, and set the alert style to Banners or Alerts rather than + None. Turn off Do Not Disturb and any Focus mode. Then run the snippet again. +- **It appears** — the browser and the OS are fine, so the problem is between + the api and the browser. Carry on below. + +**2. Is a subscription stored, and for the person receiving the notification?** + + docker exec doubtfire-api bundle exec rails runner \ + 'puts PushSubscription.all.map { |s| "#{s.user.username} #{s.endpoint[0,50]}" }' + +Empty means the opt-in did not save. Subscribing as one account and triggering a +notification for another is the usual mistake: the push goes to whoever the +notification is *for*, so subscribe as `student_1` and comment as `acain`. + +**3. Did the api actually try to send?** + + docker logs --since 5m doubtfire-api | grep -i "push" + +`Failed to push to subscription` tells you why. **No line at all** means it never +tried, which means either no subscription for that user or no VAPID keys. + +**4. Is the service worker the one you think it is?** Dev tools → Application → +Service Workers. If it says "waiting to activate", or lists more than one, click +**Unregister**, reload, wait six seconds, and subscribe again. A stale worker +from an earlier build accepts the subscription and then does nothing useful with +it. + +**Push needs a secure context.** `http://localhost` counts. A phone pointed at +your laptop over the LAN does not, and push will silently fail. + +**Clear the inbox before you start** so the demo is not full of test mail: + + curl -s -X DELETE http://localhost:8025/api/v1/messages + +--- + +## What to say if asked + +**"Do real emails get sent?"** Not in development. Mailpit is a mail catcher: it +speaks real SMTP, accepts everything and forwards nothing. Production sends over +real SMTP through the same code path. + +**"Is that a real email address?"** The seed data uses fake addresses, and two of +the accounts were pointed at a real Deakin address during development to prove +delivery. Mailpit catches it either way. + +**"How does a user turn push on?"** The button in the profile. It asks the +browser for permission and stores the registration against that user. One row per +browser, so signing in on a second machine adds a second one. + +**"What happens when someone clears their browser data?"** The push service +starts returning 410 for that endpoint, and the api deletes the row the first +time it sees one. Dead registrations do not accumulate. + +**"What is left to do?"** Clicking a push notification does not navigate anywhere +yet — the link is in the payload, but reading it needs MN-C03. There is no +in-app notification bell yet either. The rest of the event tickets are written +and unblocked: adding one is now a service call plus two email templates, with no +changes to any shared file. diff --git a/DEPLOYING.md b/DEPLOYING.md index 930547a2..d7998161 100644 --- a/DEPLOYING.md +++ b/DEPLOYING.md @@ -41,6 +41,8 @@ The setups to configure these components include: - Add monitoring as needed to ensure ongoing operation 4. Adjust **.env.production**: - **DF_PRODUCTION_DB_*** settings - adjust database settings for adapter type, host name, database name, and password. The provided setting work with the database setup in the compose file. The password should be updates as a minimum. + - **DF_PPI_MINIMUM_COHORT_SIZE** - keep the approved minimum cohort size of `21`, or raise it to a stricter value. Do not lower it. The API withholds peer progress for cohorts below this value; an enabled unit with an eligible snapshot returns 503 when the setting is missing or invalid. + - **DF_PPI_STALE_AFTER_HOURS** - keep the approved maximum snapshot age of `48` hours unless a stricter value is required. The API withholds peer percentages from older snapshots; an enabled unit with an eligible snapshot returns 503 when the setting is missing or invalid. - **DF_SECRET_KEY_DEVISE** - contains the key used to encrypt the [Devise](https://github.com/heartcombo/devise) user data in the database. Keys can be generated with `bundle exec rake secret` run in the *apiserver* container. - **DF_SECRET_KEY_BASE** and **DF_SECRET_KEY_ATTR** - these are historic keys used to encrypt data in the database. Generate as with the Devise key. - **DF_SECRET_KEY_MOSS** - the key used to connect with the [MOSS](http://moss.stanford.edu) system for checking code similarity. @@ -99,4 +101,3 @@ The setups to configure these components include: ``` When successful you should be able to login as the admin user. - diff --git a/RUNNING-LOCALLY.md b/RUNNING-LOCALLY.md new file mode 100644 index 00000000..47763f55 --- /dev/null +++ b/RUNNING-LOCALLY.md @@ -0,0 +1,510 @@ +# Running OnTrack locally (web and api) + +How to run OnTrack on your computer with Docker. It also lists the problems we hit and how +to fix them. + +## What runs + +- doubtfire-api: the backend (Rails). Port 3000. +- doubtfire-sidekiq: the background worker for queued notification email. See below. +- doubtfire-web: the frontend (Angular). Port 4200. +- Mailpit: catches every email the app sends. Web inbox on port 8025. +- A database (MariaDB) and Redis. Docker starts these for you. + +## Before you start + +- Install Docker Desktop and start it. +- Set your git remotes. `origin` is the team org, `upstream` is thoth-tech. +- Do not use the `development` branch. It is old and frozen. +- Do not install Ruby or Node. They run inside the Docker images. The api needs Ruby 3.4. + The web needs Node 22.22.3 or newer. +- Do not run `rails`, `rubocop`, or `bundle` on your own computer. Your Mac has old Ruby + (2.6). Run those inside the container instead. + +## Clone all three repos side by side + +This is the most common reason the build fails. + +Docker builds the api and web containers from folders it expects to find next to the deploy +folder. The compose file hardcodes `../../doubtfire-api` and `../../doubtfire-web`. If your +folders have different names, or are nested, or you only cloned the deploy repo, the build +fails and the error will not tell you why. + +Make one parent folder and clone all three into it: + + mkdir ontrack && cd ontrack + git clone https://github.com/ontrack-features-t2-2026/doubtfire-deploy.git + git clone https://github.com/ontrack-features-t2-2026/doubtfire-api.git + git clone https://github.com/ontrack-features-t2-2026/doubtfire-web.git + +You should end up with exactly this: + + ontrack/ + doubtfire-deploy/ + doubtfire-api/ + doubtfire-web/ + +Do not rename the folders. Do not put doubtfire-api inside doubtfire-deploy. The deploy repo +already has empty folders with those names. They are uninitialised submodules. Your code +does not go there. + +## Which branch to check out + +- **doubtfire-api** and **doubtfire-web**: `feature/notifications`, or your own work branch + made from it. +- **doubtfire-deploy**: `11.0.x`. +- **Peer Progress Indicator work**: use `ppi/student-progress-endpoint` for + **doubtfire-api** while API PR #16 is open. After it merges, use + `feature/peer-progress-indicator`. Use `feature/peer-progress-indicator` for + **doubtfire-web**. + +If `development/docker-compose.local-paths.yml` is not in your checkout, you are on the +wrong branch, or the fix has not been merged yet. Ask the lead. + +The api and web containers run whatever is checked out in those sibling folders, including +changes you have not committed. So the branch you pick is the code you are running. + +## Steps to run + +All commands run from the deploy folder: + + cd doubtfire-deploy/development + +**Use both `-f` flags on every command.** The second file is what points the build at your +sibling folders and fixes the api proxy. Without it nothing works. + +**Do not run `run-api-web.sh`.** It sits in this folder and looks like the way to start +things. It leaves out the second `-f` flag and fails on an empty build context. + +1. Build and start everything. Use `--build` the first time, and after you switch to + `11.0.x`. + + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml up -d --build + + The first build is slow. It installs gems and node packages. + +2. Set up the database. Do this the first time, or any time the database is broken. + + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml run --rm doubtfire-api \ + bash -c "bundle exec rake db:populate" + + `db:populate` already drops, creates, migrates and seeds the database on its own. The + longer command you may see elsewhere does the slowest part of setup twice. + + If you get a database connection error, the database container is probably still + starting. Wait a few seconds and run the command again. + + It takes a while and prints a lot. That is normal. + +3. Make sure the app is up. + + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml up -d + +4. Open the app. + + - Web: http://localhost:4200 + - API docs: http://localhost:3000/api/docs + - Mail inbox: http://localhost:8025 + +5. Log in. Every test user has the password "password". + + - Student: student_1 + - Admin: aadmin + - Convenor: aconvenor + - Tutor: atutor + + You will usually want two of these signed in at once, because most notification work is + one person doing something and another person being told about it. Use a private browser + window for the second account instead of logging in and out. + + A student's dashboard is at `/projects//dashboard`. There is no top-level + `/dashboard` page. Typing that address sends you back to the home page. + +## Where the emails go + +**Open http://localhost:8025** + +That is Mailpit, a mail catcher. Every email the app sends arrives there and you can read +it in your browser, subject, recipient and all. New mail appears without reloading the page. + +The app never sends real email in development. Mailpit accepts everything and forwards +nothing, so you can safely put your own address on a test account. + +- Web inbox: http://localhost:8025 +- The api sends to it over SMTP on port 1025 inside Docker. + +The `doubtfire-sidekiq` service is there to take notification email off the api request +path. It reads the `mailers` queue in Redis and delivers what it finds to Mailpit. The +normal `up` command starts it. + +**The api still sends notification email inline.** So the queue is empty and the worker has +nothing to do yet. That changes when api PR #43 merges, which is the change that puts the +email on the queue. Until then the worker being up or down makes no difference to your +inbox. + +The worker only listens on the `mailers` queue, so it does not run the rest of the +background jobs. That is on purpose. Several of them need a LaTeX container this stack does +not have, and a worker that picked those up would fail every task submission and push the +task back to "fix". + +Check the worker and its recent job output: + + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml ps doubtfire-sidekiq + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml logs --tail 100 doubtfire-sidekiq + +Once delivery is queued, stopping the worker does not lose notification email. Starting it +again processes the pending work: + + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml stop doubtfire-sidekiq + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml start doubtfire-sidekiq + +If the inbox stays empty: + +1. Check the container is running: `docker ps | grep mailpit` +2. Check the api knows about it: + + docker exec doubtfire-api printenv DF_SMTP_ADDRESS + + You want `df-compose-mailpit`. If it is blank, your api container was started before the + mail catcher was added. Recreate it: + + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml up -d doubtfire-api + + A plain `restart` is not enough. Environment variables only change on recreate. + +**Without Docker**, or if `DF_SMTP_ADDRESS` is unset, the api falls back to writing each +email to a file under `doubtfire-deploy/data/tmp/mails/`. One file per recipient address, +with new mail appended to the end. That is the old behaviour and it still works. + +The comment in `doubtfire-api/config/environments/development.rb` used to say mail landed in +`doubtfire-api/tmp/mails`, which was wrong under Docker and sent people looking in an empty +folder in the wrong repository. That comment is now fixed. + +## How to check it is working + +- See the containers: + + docker ps + +- Check the api answers, from inside the container: + + docker exec doubtfire-api curl -s localhost:3000/api/settings + +- Check the web can reach the api through its proxy. You want 200: + + docker exec doubtfire-web curl -s -o /dev/null -w "%{http_code}\n" localhost:4200/api/settings + +- Check the mail catcher answers. You want 200: + + curl -s -o /dev/null -w "%{http_code}\n" http://localhost:8025/ + +- List what is in the mail inbox without opening a browser: + + curl -s http://localhost:8025/api/v1/messages | head -c 400 + +- Read the logs: + + docker logs doubtfire-api + docker logs doubtfire-sidekiq + docker logs doubtfire-web + +## Asking for help + +Grab these before you ask. The second one answers most questions on its own. + + docker ps -a + +On macOS or Linux: + + docker logs --tail 200 doubtfire-api > api-log.txt 2>&1 + docker logs --tail 200 doubtfire-web > web-log.txt 2>&1 + +**On Windows, wrap it in `cmd /c` or the file comes out unreadable.** + + cmd /c "docker logs --tail 200 doubtfire-api > api-log.txt 2>&1" + cmd /c "docker logs --tail 200 doubtfire-web > web-log.txt 2>&1" + +PowerShell does two things to a plain `>` redirect that ruin the file. It writes UTF-16, so +every character comes out with a null byte next to it and most tools see binary rather than +text. And it treats anything the command sends to stderr as a PowerShell error object, so the +real message gets buried under `At line:1 char:1`, `CategoryInfo` and `FullyQualifiedErrorId` +noise, with the actual error split away from its own stack trace. `cmd /c` does neither. + +Use `docker ps -a` and not `docker ps`. Plain `docker ps` hides containers that have already +exited, and a container that exited is usually the whole problem. If `doubtfire-api` is +missing from `docker ps` but says Exited in `docker ps -a`, that is your answer and its log +says why. + +**Send logs as text, not as a screenshot.** Attach the two files, or paste the output inside +a fenced code block with three backticks. A screenshot of a terminal crops the part that +matters, cannot be searched, and in a Ruby crash the line you need is usually well below the +line you can see. Text can be matched against the errors in the next section in seconds. A +screenshot cannot. + +Screenshots are still the right thing for anything visual. "The page says Temporarily +Unavailable" is a screenshot, because the rendering is the evidence. Anything with a stack +trace in it is text. + +Say which branch each repo is on as well, and whether you used both `-f` flags. A lot of the +answers below turn on those two things. From `doubtfire-deploy/development`: + + git branch --show-current + git -C ../../doubtfire-api branch --show-current + git -C ../../doubtfire-web branch --show-current + +## Problems and fixes + +1. The api will not start. Error: "Your Ruby version is 3.1.7, but your Gemfile specified + ~> 3.4.0". + Cause: the image was built with old Ruby. `11.0.x` needs Ruby 3.4. + Fix: rebuild the images. Add `--build` to the up command. + +2. The web will not start. Error: "The Angular CLI requires a minimum Node.js version of + v22". + Cause: the image was built with old Node. `11.0.x` needs Node 22. + Fix: rebuild the images. Add `--build`. + +3. `up` stops straight away. Error: "service overseer-worker-1 has neither an image nor a + build context". + Cause: an old local-paths file had overseer services with no image. + Fix: already fixed. The local-paths file now only has api and web. + +4. The web crashes. Error: "Missing script: start-compose". + Cause: `11.0.x` renamed that script to "start". + Fix: already fixed. The local-paths file runs "npm start". + +5. The api crashes while migrating. Error: "Table 'doubtfire-dev.task_prerequisites' doesn't + exist". + Cause: the database has old, half-set-up data. The api container runs `db:migrate` every + time it starts, so a half-populated database makes it crash on boot over and over, before + it ever listens on port 3000. + Fix: reset the database. Run step 2 above. + + **If step 2 fails too, throw the database away and start it again.** Step 2 asks MariaDB + to drop the database, and a server that cannot read its own files cannot drop them + either. `-v` deletes the volume the database lives in, which is the only thing that + clears it. + + ```bash + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml down -v + + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml up -d + + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml run --rm doubtfire-api bash -c "bundle exec rake db:populate" + ``` + + MariaDB sets itself up from scratch on first boot, so give it a few seconds after the `up` + before you run the populate. + + This deletes your local data. That is fine, everything in it came from `db:populate` and + the command above puts it all back. `-v` also clears the web `node_modules` volume, so the + next start is slower while npm reinstalls. Your code is untouched either way: the repos + are bind mounted, not copied. + +6. The app loads but shows "Temporarily Unavailable" and the title stays "Loading...". + **Check the api is running before you read any further.** `docker ps` hides containers + that have exited, so run `docker ps -a` and look for `doubtfire-api`. If it is missing or + says Exited, this is not a proxy problem, it is problems 5, 10 or 14, and `docker logs + doubtfire-api` says which. + Cause: the web app cannot reach the api. The proxy points at localhost:3000, which is + wrong inside the container. The api is a different container named doubtfire-api. + Fix: already fixed. The local-paths file mounts `proxy.conf.docker.json`, which points at + doubtfire-api:3000. If you still see the error, rebuild the web container and reload: + + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml up -d --build doubtfire-web + +7. The build fails straight away, or complains about an empty or missing build context. + Cause: your folders are not laid out the way the compose file expects, or you only cloned + the deploy repo. + Fix: see "Clone all three repos side by side" above. All three must sit next to each + other, with their original names. + +8. You switched branch, and now the web container fails on a package it should have. + Cause: node_modules lives in a Docker volume that survives `docker compose down`, so a + branch with different dependencies installs on top of stale packages. + Fix: clear the volume and rebuild. + + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml down -v + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml up -d --build + + **`-v` does delete your database**, because that is a volume too. Run step 2 afterwards to + put it back. Your code is not touched, the repos are bind mounted rather than copied. + +9. You trigger an email and nothing appears at http://localhost:8025. + Cause: nearly always an api container started before the mail catcher existed, so it + still has no `DF_SMTP_ADDRESS` and is writing files instead. + Fix: recreate it. `restart` does not pick up new environment variables. + + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml up -d doubtfire-api + + Confirm with `docker exec doubtfire-api printenv DF_SMTP_ADDRESS`, which should print + `df-compose-mailpit`. + +10. The api container will not start. Error: "Could not find in locally installed + gems (Bundler::GemNotFound)". + Cause: somebody added a gem to the api `Gemfile`. Gems are installed into the image when + it is built, not into a volume, so a container started from the old image does not have + it. The api then crash-loops before it ever listens on port 3000. + Fix: rebuild the image. + + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml up -d --build doubtfire-api + + Running `bundle install` with `docker exec` looks like it works and does not survive. + The gems land in the running container's writable layer and are thrown away the next + time the container is recreated. + +11. The app starts throwing 500s while you are using it, and the log says + "ActiveRecord::LockWaitTimeout: Lock wait timeout exceeded". + Cause: **the test suite and the app share one database.** The compose file sets + `DF_TEST_DB_DATABASE` and `DF_DEV_DB_DATABASE` to the same value, `doubtfire-dev`. Tests + hold long transactions, so anything you do in the browser at the same time queues behind + them until it times out. Tests also change and delete your seeded data. + Fix: do not run `rails test` while anyone is using the app, and never during a demo. Run + `rake db:populate` afterwards if your data looks wrong. + + This is not something the notification work introduced. It is how the stack has always + been configured. + +12. You post a comment, get a 403 back, and no email arrives. The error says "Comment + duplicates last comment, so ignored". + Cause: OnTrack drops a comment whose text is identical to the previous comment on that + task. No comment is created, so no notification and no email. This is existing behaviour + in `app/api/task_comments_api.rb`, not something notifications introduced. + Fix: type something different. When rehearsing a demo, vary the text each time. + +13. `git status` in doubtfire-deploy shows an untracked `doubtfire-overseer/` folder. + Cause: it is a leftover checkout from another branch. `11.0.x` does not use it. Most + people will never see it. + Fix: none needed. Leave it alone. Do not `git add` it and do not delete it. + +14. `rake db:populate` fails part way through. Error: "Error on rename of + './doubtfire@002ddev/' to './doubtfire@002ddev/#sql-backup-1-7' (errno: 194 + "Tablespace is missing for a table")". + **This is a Windows problem and it is not your data.** It happens on a completely fresh + database, so deleting things and starting again does not help. Three people tried that and + got the identical error on the identical table. + Cause: the database used to live in a bind mount, `../data/database`, a folder on your own + machine shared into the container. InnoDB cannot reliably rename a table across that share + on Windows, and it reports errno 194. Loading the schema renames tables while it adds + foreign keys, so `db:populate` trips over it on the first table in that pass every time. + It is not a Rails problem and it is not corruption. See docker-library/mariadb#331, which + reproduces it in three SQL statements. + Fix: already fixed. The database is a named Docker volume now, which lives inside Docker's + own filesystem and never touches the Windows one. Pull the latest `11.0.x`, then: + + ```bash + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml down -v + + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml up -d + + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml run --rm doubtfire-api bash -c "bundle exec rake db:populate" + ``` + + The old `doubtfire-deploy/data/database` folder is dead after that and you can delete it. + Nothing reads it any more. + You will usually see the api container die too, because it migrates on boot. Same problem, + and the same reset fixes both. + +15. `bundle exec rake db:populate` fails instantly. On Windows the error is "WSL ... ERROR: + CreateProcessCommon:800: execvpe(/bin/bash) failed: No such file or directory". On macOS + it is "bundle: command not found". + Cause: the command ran on your own machine instead of inside the api container. Ruby and + the gems are only in the container. Nothing needs to be installed on your machine. + Fix: use the whole command from step 2. The part before `bash -c` is not optional. + + ```bash + docker compose -f docker-compose.yml -f docker-compose.local-paths.yml run --rm doubtfire-api bash -c "bundle exec rake db:populate" + ``` + + This single-line form avoids the PowerShell line-continuation issue. + +## Notes + +- Docker mounts your local folders. The web and api run your branch code, including changes + you have not committed yet. +- The first time you move to `11.0.x` you must rebuild the images with `--build`. Old images + will not work. +- Only ports 3000 and 4200 are reachable from your machine. The database and Redis are + internal to Docker, so a database client on your Mac cannot connect to them. To look at the + database, go through the container: + + docker exec -it doubtfire-api bash -c "bundle exec rails console" + +- The compose files still carry image tags that say `8.0.x-dev`. If you already have an old + image cached under that name, Docker reuses it instead of building a new one. That is what + causes problems 1 and 2. It is why `--build` matters. + +## Peer Progress Indicator configuration + +The local API container receives these non-secret development defaults: + +- `DF_PPI_MINIMUM_COHORT_SIZE=21` +- `DF_PPI_STALE_AFTER_HOURS=48` + +`DF_PPI_MINIMUM_COHORT_SIZE=21` matches the API's own floor. +`PeerProgressApi::MINIMUM_SAFE_COHORT_SIZE` is 21 and `minimum_cohort_size!` +causes an enabled unit with an eligible snapshot to return 503 for anything +below it rather than publishing a smaller cohort. You can raise this value, +you cannot lower it. `positive_integer_env!` separately rejects a missing, +zero, negative or non-integer value. + +The floor is 21 because the API quantises percentages into 10-point buckets. +At 20 students each student accounts for exactly half a bucket, leaving some +rounded outputs that map to only one possible submitted count. At 21 students +each student's share is smaller than half a bucket, so every published output +maps to at least two possible counts, including the edge buckets. Changing +either number without the other breaks that guarantee, so +`MINIMUM_SAFE_COHORT_SIZE` and `PERCENTAGE_BUCKET_SIZE` are asserted together +in the API test suite. + +`DF_PPI_STALE_AFTER_HOURS=48` is the local maximum snapshot age. A snapshot +older than this is returned as stale, and the response withholds the +percentage entirely rather than returning an old one. + +The local Compose stack starts Redis and a Sidekiq worker, but the worker +only listens on the `mailers` queue, so it does not pick up +`AggregatePeerProgressJob`. Run that job by hand. To test PPI locally, first +list the active unit IDs: + +```bash +docker exec doubtfire-api bundle exec rails runner \ + 'Unit.active_units.order(:id).pluck(:id).each { |id| puts id }' +``` + +If this prints no unit IDs, complete step 2 of **Steps to run** above +(*Set up the database*) using `db:populate`, then run the command again. + +Choose a test unit ID and replace `123` in the following commands. Clear any +existing rows and record the count first, or a second run reads as a pass even +when the job raised: + +```bash +docker exec doubtfire-api bundle exec rails runner \ + 'Unit.find(123).update!(peer_progress_enabled: true)' + +docker exec doubtfire-api bundle exec rails runner \ + 'PeerProgressSnapshot.where(unit_id: 123).delete_all; \ + puts "BEFORE=#{PeerProgressSnapshot.where(unit_id: 123).count}"' + +docker exec doubtfire-api bundle exec rails runner \ + 'AggregatePeerProgressJob.new.perform(123)' + +docker exec doubtfire-api bundle exec rails runner \ + 'puts "AFTER=#{PeerProgressSnapshot.where(unit_id: 123).count}"' +``` + +`BEFORE=0` followed by an `AFTER` above zero confirms that stored +peer-progress snapshots were created by this run. An `AFTER` of zero means the +selected unit did not have suitable seeded projects, tasks, or target-grade +cohorts. + +Seeded units are small, so most target-grade cohorts will sit under the floor +of 21 and the endpoint will read as unavailable even once snapshots exist. +That is correct behaviour, not a broken setup. To see a number, seed or use a +target-grade cohort that meets both the floor of 21 and the configured +threshold; never lower the threshold below 21. + +The production Compose template supplies the same approved values through +`production/.env.production`. These values are not secrets, but both must +remain present because an enabled unit with an eligible snapshot returns 503 +when either setting is missing or invalid. diff --git a/development/api.env b/development/api.env index 72c33f9c..9ecf1975 100644 --- a/development/api.env +++ b/development/api.env @@ -5,6 +5,10 @@ RAILS_ENV=development TZ=Australia/Melbourne +# Peer Progress Indicator local development defaults +DF_PPI_MINIMUM_COHORT_SIZE=21 +DF_PPI_STALE_AFTER_HOURS=48 + # Student work location (in container) DF_STUDENT_WORK_DIR=/student-work @@ -61,4 +65,3 @@ DF_PRODUCTION_DB_PASSWORD=pwd # Mail settings DF_MAIL_DELIVERY_METHOD=test - diff --git a/development/docker-compose.full.yml b/development/docker-compose.full.yml index 69d54c40..0badc3d3 100644 --- a/development/docker-compose.full.yml +++ b/development/docker-compose.full.yml @@ -2,14 +2,18 @@ version: '3' services: dev-db: container_name: df-compose-dev-db - image: mariadb + # Kept identical to docker-compose.yml. Both share the db_data volume, so + # they must agree on the server version and on the mount. + image: mariadb:12.3 environment: MYSQL_ROOT_PASSWORD: db-root-password MYSQL_DATABASE: doubtfire-dev MYSQL_USER: dfire MYSQL_PASSWORD: pwd volumes: - - ../data/database:/var/lib/mysql + # Named volume, not ../data/database. See the comment in + # docker-compose.yml for why a bind mount breaks InnoDB on Windows. + - db_data:/var/lib/mysql doubtfire-api: container_name: doubtfire-api @@ -44,6 +48,10 @@ services: DF_AAF_AUTH_SIGNOUT_URL: https://sync-uat.deakin.edu.au/auth/logout DF_SECRET_KEY_AAF: v4~LMFLzzwRGZdju\5QBa@FiHIN9 + # Peer Progress Indicator local defaults, aligned with production policy. + DF_PPI_MINIMUM_COHORT_SIZE: '21' + DF_PPI_STALE_AFTER_HOURS: '48' + # Database settings - for development env DF_DEV_DB_ADAPTER: mysql2 DF_DEV_DB_HOST: df-compose-dev-db @@ -136,3 +144,6 @@ services: environment: RABBITMQ_DEFAULT_USER: secure_credentials RABBITMQ_DEFAULT_PASS: secure_credentials + +volumes: + db_data: diff --git a/development/docker-compose.local-paths.yml b/development/docker-compose.local-paths.yml new file mode 100644 index 00000000..616ff705 --- /dev/null +++ b/development/docker-compose.local-paths.yml @@ -0,0 +1,41 @@ +version: '3' +# Overlay for `docker-compose.yml` (api + web + db, overseer disabled). +# Repoints the api/web build contexts from the doubtfire-deploy submodule +# placeholders to the real sibling checkouts under ontrack/ (the working repos, +# incl. uncommitted changes). Overseer services are intentionally omitted — the +# base compose has none (OVERSEER_ENABLED: 0); use docker-compose.full.yml if you +# need overseer. +services: + doubtfire-api: + build: ../../doubtfire-api + volumes: + - ../../doubtfire-api/:/doubtfire + - ../data/tmp:/doubtfire/tmp + - ../data/student-work:/student-work + environment: + # api.env doesn't define these, but ActiveRecord's db tasks always + # process the "test" config alongside "development", crashing with a + # NoMethodError on the blank adapter if they're unset. + DF_TEST_DB_ADAPTER: mysql2 + DF_TEST_DB_HOST: df-compose-dev-db + DF_TEST_DB_DATABASE: doubtfire-dev + DF_TEST_DB_USERNAME: dfire + DF_TEST_DB_PASSWORD: pwd + + doubtfire-sidekiq: + build: ../../doubtfire-api + volumes: + - ../../doubtfire-api/:/doubtfire + - ../data/tmp:/doubtfire/tmp + - ../data/student-work:/student-work + + doubtfire-web: + build: ../../doubtfire-web + # (1) Base compose runs `npm run start-compose`, but 11.0.x renamed it to `start`. + # (2) The repo's proxy.conf.json targets localhost:3000 (correct only for host-based + # ng serve); inside the container the api is the `doubtfire-api` service, so we + # overlay a docker-correct proxy config over it (the host repo file is untouched). + command: /bin/bash -c 'npm install; npm start' + volumes: + - ../../doubtfire-web:/doubtfire-web + - ./proxy.conf.docker.json:/doubtfire-web/proxy.conf.json:ro diff --git a/development/docker-compose.podman.yml b/development/docker-compose.podman.yml new file mode 100644 index 00000000..aca530e0 --- /dev/null +++ b/development/docker-compose.podman.yml @@ -0,0 +1,31 @@ +services: + dev-db: + volumes: + - podman_db_data:/var/lib/mysql + + doubtfire-api: + image: localhost/ontrack-doubtfire-api:11.0-local + volumes: + - ../../doubtfire-api/:/doubtfire:z + - ../data/tmp:/doubtfire/tmp:z + - ../data/student-work:/student-work:z + + doubtfire-sidekiq: + image: localhost/ontrack-doubtfire-api:11.0-local + volumes: + - ../../doubtfire-api/:/doubtfire:z + - ../data/tmp:/doubtfire/tmp:z + - ../data/student-work:/student-work:z + + doubtfire-web: + image: localhost/ontrack-doubtfire-web:11.0-local + userns_mode: "keep-id:uid=1000,gid=1000" + security_opt: + - label=disable + command: /bin/bash -c 'npm install && npm start' + volumes: + - ../../doubtfire-web:/doubtfire-web + - ./proxy.conf.docker.json:/doubtfire-web/proxy.conf.json:ro + +volumes: + podman_db_data: diff --git a/development/docker-compose.yml b/development/docker-compose.yml index 1e5f98f0..f0cdb640 100644 --- a/development/docker-compose.yml +++ b/development/docker-compose.yml @@ -2,20 +2,54 @@ version: '3' services: dev-db: container_name: df-compose-dev-db - image: mariadb + # Pinned so everyone runs the same server. An unpinned `mariadb` tag means + # two people who set up a month apart get two different majors, and a data + # directory written by one is not readable by the other. + image: mariadb:12.3 environment: MYSQL_ROOT_PASSWORD: db-root-password MYSQL_DATABASE: doubtfire-dev MYSQL_USER: dfire MYSQL_PASSWORD: pwd volumes: - - ../data/database:/var/lib/mysql + # A named volume, NOT a bind mount to ../data/database. InnoDB cannot + # reliably rename a table on a host directory shared into the container + # on Windows, and it fails with errno 194, "Tablespace is missing for a + # table". Rails hits that on a clean `rake db:populate`, because loading + # the schema renames tables while adding foreign keys. It reads as a + # Rails or a data corruption problem and is neither: the same database + # on a named volume is fine. docker-library/mariadb#331. + # docs/ONTRACK_PODMAN_SETUP.md reached the same conclusion for Podman. + - db_data:/var/lib/mysql redis-sidekiq: container_name: df-compose-redis-sidekiq image: redis:7.0 volumes: - redis_sidekiq_data:/data + # doubtfire-sidekiq is gated on this. A plain depends_on only waits for the + # container to exist, not for Redis to accept a connection, and a worker + # that loses that race is down for the rest of the session with nothing in + # the api log to say so. + healthcheck: + test: ["CMD", "redis-cli", "ping"] + interval: 5s + timeout: 3s + retries: 10 + start_period: 5s + + # Mail catcher. Accepts every email the app sends and shows it in a web inbox + # at http://localhost:8025. Nothing is delivered to the outside world. + mailpit: + container_name: df-compose-mailpit + image: axllent/mailpit:latest + ports: + - "8025:8025" # web inbox, open this in a browser + - "1025:1025" # smtp, what the api sends to + environment: + MP_MAX_MESSAGES: 500 + MP_SMTP_AUTH_ACCEPT_ANY: 1 + MP_SMTP_AUTH_ALLOW_INSECURE: 1 doubtfire-api: container_name: doubtfire-api @@ -29,9 +63,32 @@ services: - ../data/student-work:/student-work depends_on: - dev-db - environment: + - redis-sidekiq + - mailpit + environment: &doubtfire-api-environment RAILS_ENV: 'development' + # Mail catcher. Setting DF_SMTP_ADDRESS is what switches the api from + # writing mail to a file to sending it to mailpit. Unset it and the api + # falls back to files, so running without docker still works. + DF_SMTP_ADDRESS: df-compose-mailpit + DF_SMTP_PORT: 1025 + + # Web push. Without these the push channel is a no-op and the app behaves + # exactly as it did before push existed, so it is safe to blank them out. + # + # This is a throwaway pair generated for local development only, following + # the same convention as DF_SECRET_KEY_BASE below. Production sets its own + # through real secrets and must never reuse these. + # See doubtfire-api/docs/notifications/push-setup.md to generate your own. + DOUBTFIRE_VAPID_PUBLIC_KEY: 'BOs-KbIoHK7gUIX3i2_uEuDoouj-GKxB-mY9CRmLNmd4Wn-SSl254E1g6jR1ukL3e37p8uCpaMjOvfAB0BwzvSI=' + DOUBTFIRE_VAPID_PRIVATE_KEY: '_NFIWSUTdCdLJJFh87pf4ekQLmNYqsweZ4288NpVZaY=' + DOUBTFIRE_VAPID_SUBJECT: 'mailto:noreply@doubtfire.local' + + # Peer Progress Indicator local defaults, aligned with production policy. + DF_PPI_MINIMUM_COHORT_SIZE: '21' + DF_PPI_STALE_AFTER_HOURS: '48' + DF_STUDENT_WORK_DIR: /student-work DF_INSTITUTION_HOST: http://localhost:3000 DF_INSTITUTION_PRODUCT_NAME: OnTrack @@ -83,6 +140,53 @@ services: # Redis DF_REDIS_SIDEKIQ_URL: redis://df-compose-redis-sidekiq:6379/0 + doubtfire-sidekiq: + container_name: doubtfire-sidekiq + image: lmsdoubtfire/doubtfire-api:8.0.x-dev + build: ../doubtfire-api + # Deliberately narrow. Nothing in the api sets a queue, so all thirty-odd + # perform_async calls land on `default`. Taking `default` here would run + # AcceptSubmissionJob, which is enqueued on every task submission and calls + # convert_submission_to_pdf, which raises "LATEX_CONTAINER_NAME is not set" + # because development/ has no texlive service the way production/ does. The + # task would drop to "fix" with an automated comment on it, so submissions + # would break for everyone not working on notifications. Left unread in + # Redis, the way they are today, those jobs are harmless. + # + # `default` needs a texlive service and LATEX_CONTAINER_NAME first, or the + # jobs that need one need a queue of their own. Until then anything we do + # want run here has to name `mailers`, the config/schedule.yml cron entries + # included: this worker registers them at startup and they enqueue on + # `default`, so they are scheduled but not run. + # + # `-q` is set here and not in config/sidekiq.yml because that file lives in + # doubtfire-api, which this repo cannot keep in step. The config file is + # still read for :concurrency and the command line wins over it. + command: ["bundle", "exec", "sidekiq", "-C", "config/sidekiq.yml", "-q", "mailers"] + volumes: + - ../doubtfire-api/:/doubtfire + - ../data/tmp:/doubtfire/tmp + - ../data/student-work:/student-work + # Same policy the sidekiq service in production/docker-compose.yml uses. + # A worker that died on a transient Redis or database hiccup is invisible. + # The rest of the stack still looks up and email just stops. + restart: on-failure:5 + depends_on: + dev-db: + condition: service_started + redis-sidekiq: + condition: service_healthy + mailpit: + condition: service_started + environment: + <<: *doubtfire-api-environment + DF_LOG_TO_STDOUT: '1' + # This is the process that evaluates config/schedule.yml. Three of its + # seven entries name a wall-clock time (5am, 8am, 11:55pm), so an unset + # TZ would run those on the image default of UTC and put them hours out. + # Same value as development/api.env. + TZ: Australia/Melbourne + doubtfire-web: container_name: doubtfire-web image: lmsdoubtfire/doubtfire-web:8.0.x-dev @@ -97,5 +201,6 @@ services: - web_node_modules:/doubtfire-web/node_modules volumes: + db_data: web_node_modules: redis_sidekiq_data: diff --git a/development/proxy.conf.docker.json b/development/proxy.conf.docker.json new file mode 100644 index 00000000..ff67b052 --- /dev/null +++ b/development/proxy.conf.docker.json @@ -0,0 +1,4 @@ +{ + "/api": { "target": "http://doubtfire-api:3000", "secure": false }, + "/lti/api": { "target": "http://host.docker.internal:3001", "secure": false } +} diff --git a/development/verify-notifications.sh b/development/verify-notifications.sh new file mode 100755 index 00000000..b17875ed --- /dev/null +++ b/development/verify-notifications.sh @@ -0,0 +1,104 @@ +#!/usr/bin/env bash +# Checks the notification stack end to end without opening a browser. +# +# Run from this folder: bash verify-notifications.sh +# +# Checks the stack the way DEMO.md walks through it. Read that first. +# +# Every check prints PASS or FAIL and the script exits non-zero if any failed, +# so it is safe to run before a demo or after pulling someone else's branch. +# +# Do NOT run this while `rails test` is running. The test suite and the app share +# one database (DF_TEST_DB_DATABASE and DF_DEV_DB_DATABASE are both +# doubtfire-dev), so the tests hold locks that make step 8 time out with a 500 +# that has nothing to do with the notification code. + +fails=0 +pass() { printf ' \033[32mPASS\033[0m %s\n' "$1"; } +fail() { printf ' \033[31mFAIL\033[0m %s\n' "$1"; fails=$((fails + 1)); } +check() { # check + if [ "$2" = "$3" ]; then pass "$1"; else fail "$1 (wanted '$2', got '$3')"; fi +} + +http() { curl -s -o /dev/null -w '%{http_code}' "$1"; } + +echo +echo "1. Containers" +for c in doubtfire-api doubtfire-web df-compose-dev-db df-compose-mailpit; do + state=$(docker inspect -f '{{.State.Running}}' "$c" 2>/dev/null || echo missing) + check "$c is running" "true" "$state" +done + +echo +echo "2. Ports answer" +check "api :3000/api/settings" "200" "$(http http://localhost:3000/api/settings)" +check "web :4200" "200" "$(http http://localhost:4200/)" +check "mailpit:8025" "200" "$(http http://localhost:8025/)" + +echo +echo "3. Web can reach the api through its proxy" +check "web -> api" "200" "$(docker exec doubtfire-web curl -s -o /dev/null -w '%{http_code}' localhost:4200/api/settings 2>/dev/null)" + +echo +echo "4. Mail goes to the catcher, not to a file (EN-F02)" +check "delivery_method" "smtp" "$(docker exec doubtfire-api printenv DF_SMTP_ADDRESS >/dev/null 2>&1 && echo smtp || echo file)" +check "smtp host" "df-compose-mailpit" "$(docker exec doubtfire-api printenv DF_SMTP_ADDRESS 2>/dev/null | tr -d '\r')" + +echo +echo "5. Push subscription api is mounted (MN-F01)" +paths=$(curl -s http://localhost:3000/api/swagger_doc | python3 -c "import sys,json;print('yes' if '/api/push_subscriptions' in json.load(sys.stdin).get('paths',{}) else 'no')" 2>/dev/null) +check "/api/push_subscriptions in the api docs" "yes" "$paths" +table=$(docker exec doubtfire-api bash -c "bundle exec rails runner 'puts \"R::\" + PushSubscription.table_exists?.to_s' 2>/dev/null" | grep -o 'R::.*' | cut -d: -f3) +check "push_subscriptions table exists" "true" "$table" + +echo +echo "6. Push keys are loaded (MN-F02)" +cfg=$(docker exec doubtfire-api bash -c "bundle exec rails runner 'puts \"R::\" + PushNotificationService.configured?.to_s' 2>/dev/null" | grep -o 'R::.*' | cut -d: -f3) +check "PushNotificationService.configured?" "true" "$cfg" + +echo +echo "7. Service worker is served (MN-F03)" +check "GET /ngsw-worker.js" "200" "$(http http://localhost:4200/ngsw-worker.js)" +check "GET /ngsw.json" "200" "$(http http://localhost:4200/ngsw.json)" + +echo +echo "8. A comment really does send an email" +before=$(curl -s http://localhost:8025/api/v1/messages | python3 -c "import sys,json;print(json.load(sys.stdin)['messages_count'])" 2>/dev/null) +token=$(docker exec doubtfire-api bash -c "bundle exec rails runner \"u=User.find_by(username:'acain'); puts 'R::'+u.generate_authentication_token!.authentication_token\" 2>/dev/null" | grep -o 'R::.*' | cut -d: -f3) +# The text must be different every run. OnTrack drops a comment that duplicates +# the previous one on the same task and answers 403 "Comment duplicates last +# comment, so ignored" (task_comments_api.rb). A fixed string here makes the +# script pass, then fail, then pass, depending on what ran last. No notification +# is raised for a dropped duplicate, which is correct but worth knowing when a +# demo comment produces no email. +body="verify-notifications.sh check $(date +%s) $$" +code=$(curl -s -o /tmp/verify-comment-body -w '%{http_code}' \ + -X POST "http://localhost:3000/api/projects/2/task_def_id/1/comments/" \ + -H "Username: acain" -H "Auth-Token: $token" -H "Content-Type: application/json" \ + -d "{\"comment\":\"$body\"}") +check "POST a task comment" "201" "$code" +[ "$code" = "201" ] || echo " response: $(head -c 200 /tmp/verify-comment-body)" +sleep 3 +after=$(curl -s http://localhost:8025/api/v1/messages | python3 -c "import sys,json;print(json.load(sys.stdin)['messages_count'])" 2>/dev/null) +if [ "$after" -gt "$before" ]; then + pass "mailpit received the email ($before -> $after)" +else + fail "mailpit did not receive an email ($before -> $after)" +fi + +echo +echo "9. Nothing is being swallowed" +# Email and push failures are logged and swallowed on purpose, so the log is the +# only place they show up. Scoped to the last five minutes: this is a pre-demo +# check, and an error from earlier in the day says nothing about right now. The +# comment posted in step 8 is well inside that window. +errs=$(docker logs --since 5m doubtfire-api 2>&1 | grep -c "Failed to send notification email\|Failed to push to subscription") +check "swallowed notification errors in the last 5 minutes" "0" "$errs" + +echo +if [ "$fails" -eq 0 ]; then + printf '\033[32mAll checks passed.\033[0m Read the mail at http://localhost:8025\n\n' +else + printf '\033[31m%s check(s) failed.\033[0m See doubtfire-deploy/RUNNING-LOCALLY.md\n\n' "$fails" +fi +exit "$fails" diff --git a/docs/ONTRACK_PODMAN_SETUP.md b/docs/ONTRACK_PODMAN_SETUP.md new file mode 100644 index 00000000..8c1a65e3 --- /dev/null +++ b/docs/ONTRACK_PODMAN_SETUP.md @@ -0,0 +1,690 @@ +# Running OnTrack locally with Podman on Bazzite or Fedora + +This guide records the changes that were needed to run the existing OnTrack development environment with rootless Podman on Bazzite. + +The normal Docker Compose files were kept. A separate `docker-compose.podman.yml` file was added for the Podman-specific changes. + +This should also be useful for Fedora and other Linux systems where SELinux is enabled. + +## What was different with Podman + +The main issues were not with the OnTrack code itself. They came from differences between Docker and rootless Podman: + +- SELinux blocked some bind-mounted folders. +- MariaDB could not change ownership on the host database folder. +- The frontend container could not write to `package-lock.json` or the Angular cache. +- An old Docker container was already using the Mailpit ports. +- Compose reused an older API image with the wrong Ruby version. +- Podman tried to relabel the frontend `node_modules` folder and failed. + +The final setup keeps the normal Docker files unchanged and handles these problems in a local Podman override. + +## Important notes + +- Run Podman as your normal user. Do not use `sudo podman`. +- Run the Compose commands from `doubtfire-deploy/development`. +- Use all three Compose files in every Podman command. +- Do not run the Docker and Podman OnTrack stacks on the same ports. +- Do not use `podman compose down -v` unless you want to delete the local database. +- Do not disable SELinux globally. +- Do not use `chmod 777` as a workaround. +- Keep `docker-compose.podman.yml` local unless the team decides to support it officially. + +## 1. Check the repository layout + +The three repositories must be beside each other: + +```text +Ontrack Dev/ +|-- doubtfire-api/ +|-- doubtfire-deploy/ +`-- doubtfire-web/ +``` + +Go to the development folder: + +```bash +cd "/var/home/$USER/dev/Ontrack Dev/doubtfire-deploy/development" +``` + +Your path may be under `/home` rather than `/var/home`. Both can point to the same location on Bazzite. + +Check the repository paths: + +```bash +realpath ../../doubtfire-api +realpath ../../doubtfire-web +``` + +Check the main files: + +```bash +[[ -f ../../doubtfire-api/Gemfile ]] && echo "API path is correct" || echo "API Gemfile is missing" +[[ -f ../../doubtfire-web/package.json ]] && echo "Web path is correct" || echo "Web package.json is missing" +``` + +Do not put backslashes before `&&` or `||` when running these as one-line commands. Doing that caused a `binary operator expected` error during our setup. + +## 2. Stop any old Docker version of OnTrack + +We had an old Docker Mailpit container using ports `1025` and `8025`. Podman could not start its own Mailpit container until those ports were released. + +Check the ports used by OnTrack: + +```bash +sudo ss -ltnp | grep -E ':(1025|8025|3000|4200)([[:space:]]|$)' || true +``` + +Check Docker containers using those ports: + +```bash +sudo docker ps \ + --format 'table {{.ID}}\t{{.Names}}\t{{.Ports}}' \ + | grep -E '1025|8025|3000|4200' || true +``` + +If an old Docker OnTrack stack is running, stop it from the development folder: + +```bash +sudo docker compose \ + -f docker-compose.yml \ + -f docker-compose.local-paths.yml \ + down --remove-orphans +``` + +Do not kill `docker-proxy` directly. Stop the Docker container that created it. + +Confirm the ports are free: + +```bash +sudo ss -ltnp | grep -E ':(1025|8025|3000|4200)([[:space:]]|$)' \ + || echo "Required OnTrack ports are free" +``` + +## 3. Create the Podman Compose override + +Create `docker-compose.podman.yml` inside `doubtfire-deploy/development`: + +```bash +cat > docker-compose.podman.yml <<'YAML' +services: + dev-db: + volumes: + - podman_db_data:/var/lib/mysql + + doubtfire-api: + image: localhost/ontrack-doubtfire-api:11.0-local + volumes: + - ../../doubtfire-api/:/doubtfire:z + - ../data/tmp:/doubtfire/tmp:z + - ../data/student-work:/student-work:z + + doubtfire-sidekiq: + image: localhost/ontrack-doubtfire-api:11.0-local + volumes: + - ../../doubtfire-api/:/doubtfire:z + - ../data/tmp:/doubtfire/tmp:z + - ../data/student-work:/student-work:z + + doubtfire-web: + image: localhost/ontrack-doubtfire-web:11.0-local + userns_mode: "keep-id:uid=1000,gid=1000" + security_opt: + - label=disable + command: /bin/bash -c 'npm install && npm start' + volumes: + - ../../doubtfire-web:/doubtfire-web + - ./proxy.conf.docker.json:/doubtfire-web/proxy.conf.json:ro + +volumes: + podman_db_data: +YAML +``` + +### Why these changes are needed + +`podman_db_data` is a Podman-managed volume for MariaDB. The original host bind mount failed because rootless Podman could not change ownership inside `/var/lib/mysql`. + +The local image names stop Compose from pulling or reusing an older public API image. We hit a Bundler exit code `18` because the old image had a Ruby version that did not match the current API Gemfile. + +The API mounts use `:z` so SELinux allows the source folders to be shared with the API containers. + +`doubtfire-sidekiq` is the background worker and runs the same API image, so it repeats the API block exactly. Without it the worker keeps the public `8.0.x-dev` tag from `docker-compose.yml` while the API uses the local build, and `up -d --no-build` pulls that old image instead of failing. It needs no separate build. Building `doubtfire-api` in step 8 produces the image both services use. + +The frontend uses `label=disable` because Podman failed while trying to relabel the full frontend repository, especially `node_modules`. + +The frontend also uses `keep-id` so the Node user inside the container can write to files owned by the local user. + +The command uses `npm install && npm start` so Angular does not start after a failed dependency install. + +## 4. Check the merged Compose configuration + +Run: + +```bash +podman compose \ + -f docker-compose.yml \ + -f docker-compose.local-paths.yml \ + -f docker-compose.podman.yml \ + config > /tmp/ontrack-podman-config.yml +``` + +Check the database section: + +```bash +grep -n -A20 '^ dev-db:' /tmp/ontrack-podman-config.yml +``` + +The database should use a named volume at `/var/lib/mysql`. It should not use `../data/database` as the active database mount. + +Check the frontend section: + +```bash +grep -n -A50 '^ doubtfire-web:' /tmp/ontrack-podman-config.yml +``` + +Confirm that it contains: + +```text +localhost/ontrack-doubtfire-web:11.0-local +keep-id:uid=1000,gid=1000 +label=disable +npm install && npm start +``` + +Warnings saying that the Compose `version` field is obsolete are harmless. + +The message saying Podman is executing an external Compose provider is also normal. On this system, `podman compose` used the installed Docker Compose plugin as its Compose provider. + +## 5. Prepare the API writable folders + +SELinux originally blocked the API mount. Later, Podman also failed with an `lsetxattr` error on `doubtfire-api/tmp`. + +Create the writable folders: + +```bash +sudo mkdir -p \ + ../../doubtfire-api/tmp \ + ../data/tmp \ + ../data/student-work +``` + +Return ownership to the current user: + +```bash +sudo chown -R "$(id -u):$(id -g)" \ + ../../doubtfire-api/tmp \ + ../data/tmp \ + ../data/student-work +``` + +Give the owner write access: + +```bash +sudo chmod -R u+rwX \ + ../../doubtfire-api/tmp \ + ../data/tmp \ + ../data/student-work +``` + +Apply the SELinux container label: + +```bash +sudo chcon -R system_u:object_r:container_file_t:s0 \ + ../../doubtfire-api/tmp \ + ../data/tmp \ + ../data/student-work +``` + +Check the labels: + +```bash +ls -ldZ \ + ../../doubtfire-api/tmp \ + ../data/tmp \ + ../data/student-work +``` + +Each path should show `container_file_t`. + +## 6. Prepare the frontend writable files + +The frontend initially failed with permission errors for: + +```text +/doubtfire-web/package-lock.json +/doubtfire-web/.angular/cache +``` + +Fix `package-lock.json` if it exists: + +```bash +if [ -f ../../doubtfire-web/package-lock.json ]; then + sudo chown "$(id -u):$(id -g)" ../../doubtfire-web/package-lock.json + sudo chmod u+rw ../../doubtfire-web/package-lock.json +fi +``` + +Recreate the Angular cache as the current user: + +```bash +sudo rm -rf ../../doubtfire-web/.angular +mkdir -p ../../doubtfire-web/.angular +chmod 700 ../../doubtfire-web/.angular +``` + +Check the ownership: + +```bash +ls -ldn \ + ../../doubtfire-web \ + ../../doubtfire-web/.angular \ + ../../doubtfire-web/package-lock.json +``` + +The owner should match the result of: + +```bash +id -u +``` + +## 7. Clean up failed containers and the old dependency volume + +Stop the Podman stack without deleting volumes: + +```bash +podman compose \ + -f docker-compose.yml \ + -f docker-compose.local-paths.yml \ + -f docker-compose.podman.yml \ + down --remove-orphans +``` + +Remove failed temporary containers if they exist: + +```bash +podman rm -f ontrack-db-populate 2>/dev/null || true +podman rm -f doubtfire-web 2>/dev/null || true +``` + +Find the frontend dependency volume: + +```bash +podman volume ls --format '{{.Name}}' | grep web_node_modules || true +``` + +In our setup, the volume was called: + +```text +development_web_node_modules +``` + +Remove only that dependency volume: + +```bash +podman volume rm development_web_node_modules 2>/dev/null || true +``` + +The project prefix may be different on another computer. Remove the volume ending in `web_node_modules`. + +Do not remove the volume ending in `podman_db_data`. + +## 8. Build the current API and frontend images + +Check the API Dockerfile and current branches: + +```bash +grep -n '^FROM ruby:' ../../doubtfire-api/Dockerfile +git -C ../../doubtfire-api branch --show-current +git -C ../../doubtfire-web branch --show-current +``` + +Build the API from scratch: + +```bash +podman compose \ + -f docker-compose.yml \ + -f docker-compose.local-paths.yml \ + -f docker-compose.podman.yml \ + build --pull --no-cache doubtfire-api +``` + +Build the frontend: + +```bash +podman compose \ + -f docker-compose.yml \ + -f docker-compose.local-paths.yml \ + -f docker-compose.podman.yml \ + build --pull doubtfire-web +``` + +Confirm that the local images exist: + +```bash +podman images | grep -E 'ontrack-doubtfire-(api|web)' +``` + +Check the API Ruby and Bundler versions: + +```bash +podman compose \ + -f docker-compose.yml \ + -f docker-compose.local-paths.yml \ + -f docker-compose.podman.yml \ + run --rm -T \ + --no-deps \ + --entrypoint bash \ + doubtfire-api \ + -lc 'ruby -v; bundle -v' +``` + +The Ruby version must match the requirement in the current API Gemfile. + +Check the frontend image user: + +```bash +podman run --rm \ + --entrypoint id \ + localhost/ontrack-doubtfire-web:11.0-local +``` + +The image used during this setup reported UID and GID `1000`. If a future image uses another UID or GID, update the values in `userns_mode`. + +## 9. Start MariaDB, Redis, and Mailpit + +Start the supporting services first: + +```bash +podman compose \ + -f docker-compose.yml \ + -f docker-compose.local-paths.yml \ + -f docker-compose.podman.yml \ + up -d dev-db redis-sidekiq mailpit +``` + +Wait for MariaDB to initialise: + +```bash +sleep 20 +``` + +Check the containers: + +```bash +podman compose \ + -f docker-compose.yml \ + -f docker-compose.local-paths.yml \ + -f docker-compose.podman.yml \ + ps -a +``` + +Confirm MariaDB is ready: + +```bash +podman exec df-compose-dev-db \ + mariadb-admin ping \ + -h 127.0.0.1 \ + -uroot \ + -pdb-root-password +``` + +Expected output: + +```text +mysqld is alive +``` + +If the database exits, check its logs: + +```bash +podman logs --tail 200 df-compose-dev-db +``` + +## 10. Populate the database + +Use a named detached container so the logs remain available: + +```bash +podman rm -f ontrack-db-populate 2>/dev/null || true +``` + +Start the population task: + +```bash +podman compose \ + -f docker-compose.yml \ + -f docker-compose.local-paths.yml \ + -f docker-compose.podman.yml \ + run -d \ + --no-deps \ + --name ontrack-db-populate \ + --entrypoint bash \ + doubtfire-api \ + -lc 'bundle exec rake --trace db:populate' +``` + +Follow the logs: + +```bash +podman logs -f --tail 100 ontrack-db-populate +``` + +Check the status: + +```bash +podman inspect ontrack-db-populate \ + --format 'status={{.State.Status}} exit={{.State.ExitCode}} error={{.State.Error}}' +``` + +If the status is still `running`, do not try to remove it. Wait for it to finish: + +```bash +podman wait ontrack-db-populate +``` + +A successful task returns: + +```text +0 +``` + +The final inspect result should be: + +```text +status=exited exit=0 error= +``` + +After a successful run, remove the temporary container: + +```bash +podman rm ontrack-db-populate +``` + +If it exits with a non-zero code, keep the container until you have checked the logs: + +```bash +podman logs --tail 300 ontrack-db-populate +``` + +## 11. Start the complete OnTrack environment + +Start all services using the images that were already built: + +```bash +podman compose \ + -f docker-compose.yml \ + -f docker-compose.local-paths.yml \ + -f docker-compose.podman.yml \ + up -d --no-build +``` + +Check everything: + +```bash +podman compose \ + -f docker-compose.yml \ + -f docker-compose.local-paths.yml \ + -f docker-compose.podman.yml \ + ps -a +``` + +The main containers should be running: + +```text +df-compose-dev-db +df-compose-mailpit +df-compose-redis-sidekiq +doubtfire-api +doubtfire-sidekiq +doubtfire-web +``` + +Check the application logs: + +```bash +podman logs --tail 100 doubtfire-api +podman logs --tail 150 doubtfire-web +``` + +NPM deprecation warnings are not a startup failure. The important errors to look for are `EACCES`, `permission denied`, or `lsetxattr`. + +## 12. Check frontend write access + +Run: + +```bash +podman exec doubtfire-web bash -lc ' + echo "Container identity:" + id + + test -w /doubtfire-web/package-lock.json && + echo "package-lock.json is writable" || + echo "package-lock.json is not writable" + + mkdir -p /doubtfire-web/.angular/cache/podman-write-test && + rmdir /doubtfire-web/.angular/cache/podman-write-test && + echo "Angular cache is writable" +' +``` + +Both write checks should succeed. + +## 13. Open the local services + +```text +OnTrack web: http://localhost:4200 +API documentation: http://localhost:3000/api/docs +Mailpit: http://localhost:8025 +``` + +Common local test accounts use the password `password`: + +```text +student_1 +atutor +aconvenor +aadmin +``` + +## Normal commands after the first setup + +Start the environment: + +```bash +podman compose \ + -f docker-compose.yml \ + -f docker-compose.local-paths.yml \ + -f docker-compose.podman.yml \ + up -d --no-build +``` + +Stop the environment: + +```bash +podman compose \ + -f docker-compose.yml \ + -f docker-compose.local-paths.yml \ + -f docker-compose.podman.yml \ + down +``` + +Check status: + +```bash +podman compose \ + -f docker-compose.yml \ + -f docker-compose.local-paths.yml \ + -f docker-compose.podman.yml \ + ps -a +``` + +Follow API logs: + +```bash +podman logs -f doubtfire-api +``` + +Follow frontend logs: + +```bash +podman logs -f doubtfire-web +``` + +## Errors we hit + +| Error or message | Cause | Fix | +|---|---|---| +| `binary operator expected` | A Bash test command was pasted with incorrect backslashes | Run the test as one normal line | +| `/doubtfire: Permission denied` | SELinux blocked the API bind mount | Use `:z` on the API mounts | +| `lsetxattr ... doubtfire-api/tmp ... operation not permitted` | API writable folders had unsuitable ownership or SELinux labels | Use `chown`, `chmod`, and `chcon` on the writable folders | +| `bind: address already in use` on port 1025 | An old Docker Mailpit container was still running | Stop the old Docker stack | +| `/var/lib/mysql: Permission denied` | Rootless Podman could not change ownership on the database bind mount | Use the `podman_db_data` named volume | +| Database population exited with code 18 | Compose used an older API image with the wrong Ruby version | Use unique local image names and rebuild the API | +| `ontrack-db-populate` could not be removed | The population job was still running | Follow its logs and wait for it to exit | +| `lsetxattr ... doubtfire-web/node_modules` | Podman tried to relabel the full frontend repository | Use `security_opt: label=disable` for the frontend | +| `EACCES` for `package-lock.json` | The frontend user could not write to the host file | Use `keep-id` and repair the file ownership | +| `EACCES` for `.angular/cache` | The Angular cache had the wrong owner | Delete and recreate `.angular` as the local user | +| Angular started after `npm install` failed | The original command used `;` | Use `npm install && npm start` | +| `version is obsolete` | The Compose files contain an older `version` field | Harmless warning | +| `Executing external compose provider` | `podman compose` is using an installed Compose provider | Normal behaviour | + +## Final Podman override + +The final working `docker-compose.podman.yml` was: + +```yaml +services: + dev-db: + volumes: + - podman_db_data:/var/lib/mysql + + doubtfire-api: + image: localhost/ontrack-doubtfire-api:11.0-local + volumes: + - ../../doubtfire-api/:/doubtfire:z + - ../data/tmp:/doubtfire/tmp:z + - ../data/student-work:/student-work:z + + doubtfire-sidekiq: + image: localhost/ontrack-doubtfire-api:11.0-local + volumes: + - ../../doubtfire-api/:/doubtfire:z + - ../data/tmp:/doubtfire/tmp:z + - ../data/student-work:/student-work:z + + doubtfire-web: + image: localhost/ontrack-doubtfire-web:11.0-local + userns_mode: "keep-id:uid=1000,gid=1000" + security_opt: + - label=disable + command: /bin/bash -c 'npm install && npm start' + volumes: + - ../../doubtfire-web:/doubtfire-web + - ./proxy.conf.docker.json:/doubtfire-web/proxy.conf.json:ro + +volumes: + podman_db_data: +``` + +The existing OnTrack Docker setup did not need to be rewritten. The working solution was a small local override for SELinux, rootless file ownership, MariaDB storage, and the local API and frontend images. diff --git a/docs/pull_request_template.md b/docs/pull_request_template.md new file mode 100644 index 00000000..92d5dc97 --- /dev/null +++ b/docs/pull_request_template.md @@ -0,0 +1,41 @@ +## Jira ticket + +Ticket number or link: + +## Summary + +Briefly explain what you changed and why. + +## Target branch + +Which shared branch should this be merged into? + +Example: `feature/email-notifications` + +## Testing + +Explain how you tested the change. + +Include any useful commands, screenshots, logs, or test results. + +## Security and privacy + +Does this change affect authentication, permissions, notifications, student data, +secrets, personal information, or privacy? + +If there is no known impact, write: `No known security or privacy impact.` + +## Evidence + +Add any screenshots, test output, diagrams, or other evidence that will help the reviewer. + +## Checklist + +- [ ] I selected the correct base branch. +- [ ] My changes match the assigned Jira ticket. +- [ ] I kept the change within the agreed scope. +- [ ] I tested my changes. +- [ ] I did not include passwords, tokens, API keys, secrets, or real student data. +- [ ] I updated relevant documentation, or no documentation change was needed. +- [ ] I reviewed my own changes before requesting review. +- [ ] This pull request is ready for review. diff --git a/production/.env.production b/production/.env.production index 9ce785fb..9e8fd37b 100644 --- a/production/.env.production +++ b/production/.env.production @@ -32,6 +32,10 @@ LATEX_BUILD_PATH=/texlive/shell/latex_build.sh # Redis for sidekiq DF_REDIS_SIDEKIQ_URL=redis://redis-sidekiq:6379/0 +# Peer Progress Indicator approved production values +DF_PPI_MINIMUM_COHORT_SIZE=21 +DF_PPI_STALE_AFTER_HOURS=48 + # # Institution settings #