Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
165 changes: 147 additions & 18 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ concurrency:
env:
BUN_VERSION: 1.3.13
FOUNDRY_VERSION: v1.5.1
ZOLTAR_CI_TEST_SHARDS: 4

jobs:
required:
Expand Down Expand Up @@ -60,19 +61,13 @@ jobs:
- name: Generate and verify generated artifacts
run: bun run check:generated-clean

- name: TypeScript type checking
run: bun run tsc

- name: Tests
run: bun run test

- name: Format
run: bun run format

- name: Verify formatter produced no tracked or untracked diff
- name: Verify pre-task worktree cleanliness
run: |
if ! git diff --exit-code -- .; then
echo 'Formatter changed tracked files. Run bun run format and commit the result.'
echo 'Generated files or formatting changed tracked files. Run the relevant command locally and commit the result.'
exit 1
fi

Expand All @@ -82,17 +77,151 @@ jobs:
exit 1
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Architectural concern: This single step embeds approximately 80 lines of bash process management code (start_task, remove_pid, terminate_remaining, wait_for_tasks) inline in the YAML workflow. The codebase has an established pattern of placing CI-related logic in scripts/ as TypeScript files (e.g., check-generated-artifacts.mts, check-mainnet-deployment.mts) that are linted by Biome, type-checked by tsc:scripts, and testable. The inline bash orchestrator cannot be linted, type-checked, or tested by any existing project tooling.

Additionally, this loses GitHub Actions step-level visibility: the old workflow had individually named steps (Tests, Biome and Solidity checks, Dead code analysis, Dependency audit) whose pass/fail status was visible directly in the UI. With the combined step, all sub-task output is interleaved and developers must parse logs to identify which sub-task failed.

Recommendation: extract the orchestration logic into a script under scripts/ to restore testability, lintability, and reusability while preserving the parallelism benefit.

- name: Biome and Solidity checks
run: bun run check
git diff --check

- name: Dead code analysis
run: bun run knip
- name: TypeScript type checking
run: bun run tsc

- name: Dependency audit
- name: Production UI build and mainnet deployment validation
run: bun run ui:build:prod:optimized

- name: Tests, checks, dead code analysis, and audits
shell: bash
env:
ZOLTAR_USE_EXISTING_PRODUCTION_BUILD: "1"
run: |
bun audit
(cd ui && bun audit)
(cd solidity && bun audit)
set -Eeuo pipefail

require_positive_integer() {
local name="$1"
local value="$2"
if [[ ! "$value" =~ ^[1-9][0-9]*$ ]]; then
echo "::error::${name} must be a positive integer."
exit 1
fi
}

test_shards="${ZOLTAR_CI_TEST_SHARDS:-4}"
require_positive_integer ZOLTAR_CI_TEST_SHARDS "$test_shards"

if [[ -n "${ZOLTAR_CI_TEST_PARALLEL:-}" ]]; then
test_parallel="$ZOLTAR_CI_TEST_PARALLEL"
require_positive_integer ZOLTAR_CI_TEST_PARALLEL "$test_parallel"
elif (( "$(nproc)" >= 8 )); then
test_parallel=2
else
test_parallel=1
fi

- name: Production UI build and mainnet deployment validation
run: bun run ui:build:prod
echo "Using ${test_shards} test shard(s) with --parallel=${test_parallel}."

declare -a task_pids=()
declare -A task_names=()

start_task() {
local name="$1"
local command="$2"
echo
echo "==> Starting ${name}"
echo "$ ${command}"
setsid bash -c "$command" &
local pid="$!"
task_pids+=("$pid")
task_names["$pid"]="$name"
}

remove_pid() {
local completed_pid="$1"
local -a remaining_pids=()
local pid
for pid in "${task_pids[@]}"; do
if [[ "$pid" != "$completed_pid" ]]; then
remaining_pids+=("$pid")
fi
done
task_pids=("${remaining_pids[@]}")
}

terminate_remaining() {
local failed_name="$1"
local pid
for pid in "${task_pids[@]}"; do
if kill -0 "$pid" 2>/dev/null; then
echo "Aborting ${task_names[$pid]} because ${failed_name} failed."
kill -TERM "-$pid" 2>/dev/null || kill -TERM "$pid" 2>/dev/null || true
fi
done

sleep 5

for pid in "${task_pids[@]}"; do
if kill -0 "$pid" 2>/dev/null; then
echo "Force-aborting ${task_names[$pid]}."
kill -KILL "-$pid" 2>/dev/null || kill -KILL "$pid" 2>/dev/null || true
fi
done

for pid in "${task_pids[@]}"; do
wait "$pid" 2>/dev/null || true
done
}

wait_for_tasks() {
local completed_pid
local failed_name
local status

while ((${#task_pids[@]} > 0)); do
completed_pid=''
if wait -n -p completed_pid "${task_pids[@]}"; then
status=0
else
status="$?"
fi

if [[ -z "$completed_pid" ]]; then
break
fi

failed_name="${task_names[$completed_pid]}"
remove_pid "$completed_pid"

if ((status != 0)); then
echo "::error::${failed_name} failed with exit code ${status}."
terminate_remaining "$failed_name"
exit "$status"
fi

echo "PASS ${failed_name}"
done
}

if ((test_shards == 1)); then
start_task "Tests" "bun run test:run:shard -- --bail=1 --parallel=${test_parallel}"
else
for shard in $(seq 1 "$test_shards"); do
start_task "Tests shard ${shard}/${test_shards}" "bun run test:run:shard -- --bail=1 --parallel=${test_parallel} --shard=${shard}/${test_shards}"
done
fi

start_task "Biome and Solidity checks" "bun run check"
start_task "Dead code analysis" "bun run knip"
start_task "Dependency audit" "bun audit && (cd ui && bun audit) && (cd solidity && bun audit)"

wait_for_tasks

- name: Verify final worktree cleanliness
if: ${{ always() }}
run: |
if ! git diff --exit-code -- .; then
echo 'CI left tracked file changes. Commit intended changes or fix generated output churn.'
exit 1
fi

if [[ -n "$(git status --porcelain --untracked-files=normal)" ]]; then
git status --short
echo 'CI generated untracked files. Commit them or update .gitignore as appropriate.'
exit 1
fi

git diff --check
23 changes: 17 additions & 6 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,14 @@ Use the staged and unstaged diff, including untracked files intended for the PR,
- For narrow changes, prefer the smallest meaningful targeted test command first. Run the full suite when changes touch contracts, shared behavior, cross-module contracts, package/dependency wiring, or broad app behavior.
- For docs-only, instruction-only, formatting-only, `.codex/agents`-only, or other non-executable changes, do not run tests unless the change affects a test runner, generated output, or executable tooling. State why tests were skipped.
- When the task is itself about tests, test fixtures, or test cleanup, do not require additional tests for the tests. Instead verify that the changed tests are meaningful and run the relevant test command.
- Full test command:
```bash
bun run test
```
- Full test command when TypeScript was not selected in the same validation cycle:
```bash
bun run test
```
- When `bun run tsc` has already passed in the same validation cycle, run the test runner directly so TypeScript is not checked twice:
```bash
bun run ensure-contract-artifacts && bun run check:shared-dependencies && bun run test:run -- --bail=1
```
- If tests require Anvil and the `anvil` executable is missing, install it with:
```bash
bun run install:anvil
Expand Down Expand Up @@ -81,7 +85,14 @@ Repeat the relevant part of the validation cycle after each fix. If a fix expand

## Final Review Gate

For file-changing tasks that are being prepared for a PR, after the selected checks above pass, merge the latest `main` into the branch and resolve any conflicts if they exist. Rerun the selected checks after merging `main`, and again after any conflict resolution or follow-up edits. If the merge changes the touched area, update the validation scope before rerunning checks. Skip this step for read-only analysis, exploratory answers, or when the user explicitly asks not to merge; state the reason in the final response.
For file-changing tasks that are being prepared for a PR, after the selected checks above pass, fetch `origin/main` and check whether the branch is behind:

```bash
git fetch origin main:refs/remotes/origin/main
git rev-list --count HEAD..origin/main
```

If the count is `0`, record that the branch is current and do not merge or rerun checks solely for this gate. If the count is nonzero, merge the latest `main` into the branch and resolve any conflicts if they exist. Rerun the selected checks after merging `main`, and again after any conflict resolution or follow-up edits. If the merge changes the touched area, update the validation scope before rerunning checks. Skip this step for read-only analysis, exploratory answers, or when the user explicitly asks not to merge; state the reason in the final response.

As the final quality gate for any task that changes code, tests, configuration, or repo instructions, the main agent must spawn the project-scoped `reviewer` custom agent defined in `.codex/agents/reviewer.toml` and wait for it to complete before responding to the user. Start the reviewer from a clear task summary instead of relying on inherited conversation context.

Expand Down Expand Up @@ -150,7 +161,7 @@ In the final response to the user, summarize the reviewer feedback received, rep

- Generated build outputs and protocol artifacts are intentionally untracked. Keep `/ui/js`, `/shared/js`, `/ui/vendor`, `/solidity/artifacts`, `/ui/ts/contractArtifact.ts`, and `/solidity/ts/types/contractArtifact.ts` out of source review.
- If a deployment workflow ever requires committing generated artifacts, update this policy in the same PR and add a freshness check that regenerates the artifacts and fails on a dirty tracked diff.
- Use `bun run check:generated-clean` when validating artifact freshness for CI or release work.
- The generated artifact freshness rule in Validation Selection is the source of truth for when to run artifact freshness checks.

# Code Style Guidelines

Expand Down
10 changes: 8 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,15 @@
"generate:ui-vendor": "bun run ui:vendor",
"generate:contracts": "bun run compile-contracts",
"generate:ui": "bun run generate:shared-js && bun run generate:ui-vendor",
"check:generated-clean": "bun run generate && git diff --exit-code -- shared/js solidity/artifacts solidity/ts/types/contractArtifact.ts solidity/types/contractArtifact.ts ui/js ui/ts/abis.ts ui/ts/contractArtifact.ts ui/ts/deploymentArtifacts.ts ui/ts/deploymentsArtifacts.ts ui/vendor",
"check:generated-artifacts": "bun run generate && bun ./scripts/check-generated-artifacts.mts",
"check:generated-clean": "bun run check:generated-artifacts",
"setup": "bun install --frozen-lockfile && cd ui && bun install --frozen-lockfile && cd .. && bun run setup-contracts && bun run shared:build && bun run ui:vendor && cd ui && bun run build && bun run build:tests",
"setup-contracts": "cd solidity && bun run setup",
"compile-contracts": "cd solidity && bun run compile-contracts",
"generate": "bun run generate:contracts && bun run generate:ui",
"check:mainnet-deployment": "bun run generate && bun run refresh:shared-dependencies && cd ui && bun x tsc --project tsconfig.json && cd .. && bun ./scripts/check-mainnet-deployment.mts",
"check:mainnet-deployment:generated": "bun run generate && bun run refresh:shared-dependencies && bun ./scripts/check-mainnet-deployment.mts",
"check:mainnet-deployment:current": "bun ./scripts/check-mainnet-deployment.mts",
"tsc:app": "bun x tsc --noEmit",
"tsc:scripts": "bun x tsc --project tsconfig.scripts.json --noEmit",
"tsc:solidity": "cd solidity && bun ../scripts/ensure-shared-package-fresh.mjs --refresh && bun x tsc --noEmit",
Expand All @@ -52,14 +55,17 @@
"app:build": "bun run generate && cd ui && bun run build && bun run build:tests",
"ui:build": "bun run app:build",
"ui:build:prod": "bun run check:mainnet-deployment && cd ui && bun run build:prod",
"ui:build:prod:optimized": "bun run check:mainnet-deployment:current && cd ui && bun run build:prod",
"ui:workers": "cd ui && bun ./build/workers.mts",
"app:watch": "bun run app:build && bun ./ui/build/watch.mts",
"ui:watch": "bun run app:watch",
"app:serve": "bun run app:build && bun ./ui/dev-server.ts",
"ui:serve": "bun run app:serve",
"ui:vendor": "cd ui && bun ./build/vendor.mts",
"gas-costs": "cd solidity && bun run gas-costs",
"test": "bun run ensure-contract-artifacts && bun run check:shared-dependencies && bun run tsc && bun test --preload ./bun-test-setup-ui.ts --parallel=4 --timeout 300000",
"test": "bun run ensure-contract-artifacts && bun run check:shared-dependencies && bun run tsc && bun run test:run",
"test:run": "bun test --preload ./bun-test-setup-ui.ts --parallel=4 --timeout 300000",
"test:run:shard": "bun test --preload ./bun-test-setup-ui.ts --reporter=dots --timeout 300000",
"test:peripherals": "bun run ensure-contract-artifacts && bun run check:shared-dependencies && bun run tsc:solidity && bun test --preload ./bun-test-setup-solidity.ts --timeout 300000 solidity/ts/tests/peripherals",
"test:security-pool-workflow": "bun run ensure-shared-build && bun run check:shared-dependencies && bun run tsc:app && bun test --preload ./bun-test-setup-ui.ts --timeout 300000 ui/ts/tests/securityPoolWorkflowSection",
"test:integration:mainnet": "bun run ensure-contract-artifacts && bun run check:shared-dependencies && RUN_MAINNET_INTEGRATION_TESTS=1 bun test --preload ./bun-test-setup-ui.ts --timeout 300000 ui/ts/tests/uniswapQuoter.integration.test.ts",
Expand Down
127 changes: 127 additions & 0 deletions scripts/check-generated-artifacts.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import { spawnSync } from 'node:child_process'
import { promises as fs } from 'node:fs'
import * as path from 'node:path'
import * as url from 'node:url'

const scriptDirectory = path.dirname(url.fileURLToPath(import.meta.url))
const repositoryRoot = path.join(scriptDirectory, '..')

const explicitlyRequiredGeneratedOutputs = ['shared/js/.freshness-hash', 'solidity/artifacts/Contracts.json', 'solidity/artifacts/.freshness-hash', 'solidity/.contract-hash.json', 'solidity/ts/types/contractArtifact.ts', 'solidity/types/contractArtifact.ts', 'ui/ts/abis.ts', 'ui/ts/contractArtifact.ts']

const generatedReviewPaths = ['shared/js', 'solidity/artifacts', 'solidity/.contract-hash.json', 'solidity/ts/types/contractArtifact.ts', 'solidity/types', 'ui/js', 'ui/ts/abis.ts', 'ui/ts/contractArtifact.ts', 'ui/ts/deploymentArtifacts.ts', 'ui/ts/deploymentsArtifacts.ts', 'ui/vendor']

function isRecord(value: unknown): value is Record<string, unknown> {
return typeof value === 'object' && value !== null && !Array.isArray(value)
}

async function assertExists(relativePath: string) {
try {
await fs.stat(path.join(repositoryRoot, relativePath))
} catch (error) {
if (error instanceof Error && 'code' in error && error.code === 'ENOENT') {
throw new Error(`Generated artifact is missing after generation: ${relativePath}`)
}
throw error
}
}

async function readJsonObject(relativePath: string) {
const parsed = JSON.parse(await fs.readFile(path.join(repositoryRoot, relativePath), 'utf8'))
if (!isRecord(parsed)) throw new Error(`${relativePath} must contain a JSON object`)
return parsed
}

async function assertContractsJsonReadable() {
const contractsJsonPath = path.join(repositoryRoot, 'solidity/artifacts/Contracts.json')
try {
JSON.parse(await fs.readFile(contractsJsonPath, 'utf8'))
} catch (error) {
if (error instanceof SyntaxError) throw new Error('Generated solidity/artifacts/Contracts.json is not valid JSON')
throw error
}
}

function normalizeRepositoryRelativePath(baseDirectory: string, relativePath: string) {
if (!relativePath.startsWith('./') && !relativePath.startsWith('../')) {
throw new Error(`Expected a local generated path, received: ${relativePath}`)
}
return path.posix.normalize(path.posix.join(baseDirectory, relativePath))
}

async function getSharedPackageGeneratedOutputs() {
const packageJson = await readJsonObject('shared/package.json')
const exportsValue = packageJson['exports']
if (!isRecord(exportsValue)) throw new Error('shared/package.json exports must be an object')

const outputs: string[] = []
for (const [exportName, exportValue] of Object.entries(exportsValue)) {
if (!isRecord(exportValue)) throw new Error(`shared/package.json export ${exportName} must be an object`)
const defaultPath = exportValue['default']
if (typeof defaultPath !== 'string') throw new Error(`shared/package.json export ${exportName} must define a default path`)
const generatedJavaScriptPath = normalizeRepositoryRelativePath('shared', defaultPath)
outputs.push(generatedJavaScriptPath)
if (generatedJavaScriptPath.endsWith('.js')) {
outputs.push(generatedJavaScriptPath.replace(/\.js$/, '.d.ts'))
}
}
return outputs
}

async function getUiImportMapGeneratedOutputs() {
const indexHtml = await fs.readFile(path.join(repositoryRoot, 'ui/index.html'), 'utf8')
const importMapMatch = indexHtml.match(/<script\b[^>]*\btype\s*=\s*['"]importmap['"][^>]*>([\s\S]*?)<\/script>/i)
if (importMapMatch === null) throw new Error('ui/index.html is missing an import map')

const importMapText = importMapMatch[1]
if (importMapText === undefined) throw new Error('ui/index.html import map is empty')
const importMap = JSON.parse(importMapText)
if (!isRecord(importMap)) throw new Error('ui/index.html import map must be a JSON object')
const imports = importMap['imports']
if (!isRecord(imports)) throw new Error('ui/index.html import map imports must be an object')

const outputs: string[] = []
for (const [specifier, targetPath] of Object.entries(imports)) {
if (typeof targetPath !== 'string') throw new Error(`ui/index.html import map target for ${specifier} must be a string`)
if (!targetPath.startsWith('./') && !targetPath.startsWith('../')) continue
outputs.push(normalizeRepositoryRelativePath('ui', targetPath))
}
return outputs
}

function runGit(args: readonly string[]) {
const result = spawnSync('git', args, {
cwd: repositoryRoot,
encoding: 'utf8',
})
if (result.error !== undefined) throw result.error
return result
}

function getTrackedGeneratedPaths() {
const result = runGit(['ls-files', '--', ...generatedReviewPaths])
if (result.status !== 0) {
throw new Error(`Unable to list tracked generated paths.\n${result.stdout}${result.stderr}`)
}
return result.stdout
.split('\n')
.map(line => line.trim())
.filter(line => line !== '')
}

function assertNoTrackedGeneratedPaths(trackedGeneratedPaths: readonly string[]) {
if (trackedGeneratedPaths.length === 0) return

throw new Error(`Generated artifacts must remain untracked. Remove these paths from Git and keep them covered by .gitignore and the generated artifact policy:\n${trackedGeneratedPaths.join('\n')}`)
}

const requiredGeneratedOutputs = new Set([...explicitlyRequiredGeneratedOutputs, ...(await getSharedPackageGeneratedOutputs()), ...(await getUiImportMapGeneratedOutputs())])

for (const relativePath of requiredGeneratedOutputs) {
await assertExists(relativePath)
}
await assertContractsJsonReadable()

const trackedGeneratedPaths = getTrackedGeneratedPaths()
assertNoTrackedGeneratedPaths(trackedGeneratedPaths)

console.log('Generated artifacts verified. Generated outputs are intentionally untracked, so freshness is validated by successful generation and required-output checks.')
Loading
Loading