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
30 changes: 30 additions & 0 deletions migrations/000_combined.sql
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ COMMENT ON TABLE remote_agent_workflow_runs IS
CREATE TABLE IF NOT EXISTS remote_agent_workflow_events (
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
workflow_run_id UUID NOT NULL REFERENCES remote_agent_workflow_runs(id) ON DELETE CASCADE,
event_order BIGINT,
event_type VARCHAR(50) NOT NULL,
step_index INTEGER,
step_name VARCHAR(255),
Expand All @@ -280,6 +281,9 @@ CREATE INDEX IF NOT EXISTS idx_workflow_events_type
-- (WHERE created_at >= $1 ORDER BY created_at ASC).
CREATE INDEX IF NOT EXISTS idx_workflow_events_created_at
ON remote_agent_workflow_events(created_at);
CREATE UNIQUE INDEX IF NOT EXISTS idx_workflow_events_run_order
ON remote_agent_workflow_events(workflow_run_id, event_order)
WHERE event_order IS NOT NULL;

COMMENT ON TABLE remote_agent_workflow_events IS
'Lean UI-relevant workflow events for observability (step transitions, artifacts, errors)';
Expand Down Expand Up @@ -517,6 +521,32 @@ ALTER TABLE remote_agent_user_ai_prefs
ALTER TABLE remote_agent_users
ADD COLUMN IF NOT EXISTS role VARCHAR(16) NOT NULL DEFAULT 'admin';

-- Lifecycle ordering (#2359 follow-up): timestamps can tie, especially on
-- SQLite (one-second precision), so a database-assigned order breaks the tie and
-- preserves event chronology. `id` cannot serve this role — it is a random UUID,
-- not monotonic.
--
-- Deliberately a plain column plus a sequence DEFAULT, NOT `GENERATED ... AS
-- IDENTITY`. Adding an identity column REWRITES the whole table under ACCESS
-- EXCLUSIVE (verified on postgres:18: relfilenode changes), and this is the
-- largest table in the schema while the schema auto-applies on startup — that is
-- a boot-time stall proportional to event history. ADD COLUMN with no default is
-- metadata-only, and SET DEFAULT afterwards applies to future inserts only.
--
-- It also keeps both databases honest: existing rows stay NULL on Postgres AND
-- SQLite, so the COALESCE(event_order, 0) fallback in read queries behaves
-- identically. An identity column would have back-filled Postgres rows (1, 2,
-- 3...) while SQLite left them NULL.
ALTER TABLE remote_agent_workflow_events
ADD COLUMN IF NOT EXISTS event_order BIGINT;
CREATE SEQUENCE IF NOT EXISTS remote_agent_workflow_events_event_order_seq
OWNED BY remote_agent_workflow_events.event_order;
ALTER TABLE remote_agent_workflow_events
ALTER COLUMN event_order SET DEFAULT nextval('remote_agent_workflow_events_event_order_seq');
CREATE UNIQUE INDEX IF NOT EXISTS idx_workflow_events_run_order
ON remote_agent_workflow_events(workflow_run_id, event_order)
WHERE event_order IS NOT NULL;

Comment on lines +524 to +532

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does PostgreSQL ALTER TABLE ADD COLUMN GENERATED BY DEFAULT AS IDENTITY require an ACCESS EXCLUSIVE lock and rewrite the table on versions 11+?

💡 Result:

Yes, the ALTER TABLE ... ADD COLUMN ... GENERATED BY DEFAULT AS IDENTITY command requires an ACCESS EXCLUSIVE lock in PostgreSQL versions 11 and later [1][2]. While many ADD COLUMN operations in modern PostgreSQL (version 11+) are "instant" because they do not require a full table rewrite [2][3], the addition of an identity column is an exception [4]. Specifically, adding an identity column triggers a table rewrite [4]. This occurs because the operation must initialize the underlying implicit sequence and ensure the new column is correctly populated and constrained across all existing rows [5][4]. Because it performs a full table rewrite, this operation is generally not instant on large tables and will block all access (including reads) to the table for the duration of the process due to the ACCESS EXCLUSIVE lock [1][2].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== migrations files =="
git ls-files 'migrations/**/*.sql' 'migrations/*.sql' | sed -n '1,80p'

echo
echo "== relevant migration sections =="
if [ -f migrations/000_combined.sql ]; then
  sed -n '240,310p' migrations/000_combined.sql
  echo "---- lines 505-540 ----"
  sed -n '505,540p' migrations/000_combined.sql
fi

echo
echo "== db adapter references to event_order / SQLite identity-like handling =="
rg -n "event_order|GENERATED BY DEFAULT AS IDENTITY|remote_agent_workflow_events|workflow_events" . --glob '!node_modules' --glob '!dist' --glob '!build' | sed -n '1,200p'

echo
echo "== lock/rewrite docs from local installed postgres docs if available =="
if command -v psql >/dev/null 2>&1; then
  psql -U postgres -h localhost -c "SELECT version();" 2>/dev/null || true
fi
if command -v pg_config >/dev/null 2>&1; then
  pg_config --docdir || true
fi
python3 - <<'PY'
from pathlib import Path
for root in [Path('/usr/share/doc/postgresql*')]:
    pass
PY

Repository: coleam00/Archon

Length of output: 50371


🌐 Web query:

PostgreSQL 18 identity columns table rewrite when alter table add column GENERATED BY DEFAULT AS IDENTITY existing rows NULL documentation

💡 Result:

In PostgreSQL 18, adding a column defined with GENERATED BY DEFAULT AS IDENTITY to an existing table will cause the entire table and all its indexes to be rewritten [1][2]. According to the official PostgreSQL 18 documentation, adding a column with a volatile default (such as clock_timestamp), a stored generated column, an identity column, or a column with a domain data type that has constraints requires a complete table rewrite [1][2]. This is distinct from adding a column with a simple, non-volatile constant default value, which is a metadata-only operation that does not require a rewrite [1][2]. Because this operation involves a full table rewrite, it can be resource-intensive and time-consuming on large tables, as it requires the creation of a new table file and the copying of all existing data. Even though the identity column will effectively be NULL for existing rows until they are updated or rewritten, the structural change associated with identity columns currently forces this physical rewrite of the table [1][2].

Citations:


Use a nullable order column or batched backfill instead of adding an identity column.

PostgreSQL identity columns are non-null, and they can only be added with an ACCESS EXCLUSIVE rewrite. Update the migration comment at migrations/000_combined.sql:526 because existing rows do not stay NULL under PostgreSQL. Migrate both dialects to a nullable event_order with non-blocking assignment/backfill instead of GENERATED BY DEFAULT AS IDENTITY so existing tables do not block reads/writes during startup.

🧰 Tools
🪛 Squawk (2.61.0)

[warning] 528-528: Adding a generated column requires a table rewrite with an ACCESS EXCLUSIVE lock. In Postgres versions 11+, non-VOLATILE DEFAULTs can be added without a rewrite. Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.

(adding-field-with-default)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@migrations/000_combined.sql` around lines 524 - 532, Replace the
identity-based event_order addition in the remote_agent_workflow_events
migration with a nullable INTEGER column for both database dialects. Add a
non-blocking, batched backfill that assigns unique per-workflow event_order
values to existing rows, then retain the partial unique index and update the
migration comment to reflect that existing rows are populated rather than left
NULL.

Source: Linters/SAST tools

-- ============================================================================
-- Schema vintage (#2316)
-- ============================================================================
Expand Down
13 changes: 10 additions & 3 deletions packages/cli/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ Options:
--quiet, -q Reduce log verbosity to warnings and errors only
--verbose, -v Show debug-level output
--json Output machine-readable JSON (list/status/get/runs/approve/reject/abandon/resume)
--events For verbose JSON status/get: output raw event rows instead of node summaries
--detach Run 'workflow run' in a detached background child (returns immediately)
--all For 'workflow runs': list across all projects (ignore cwd scope)
--status <status> For 'workflow runs': filter to one status (running, completed, failed, ...)
Expand Down Expand Up @@ -286,6 +287,7 @@ async function main(): Promise<number> {
quiet: { type: 'boolean', short: 'q' },
verbose: { type: 'boolean', short: 'v' },
json: { type: 'boolean' },
events: { type: 'boolean' },
'run-id': { type: 'string' },
type: { type: 'string' },
data: { type: 'string' },
Expand Down Expand Up @@ -585,13 +587,17 @@ async function main(): Promise<number> {
}

case 'status':
await workflowStatusCommand(jsonFlag, values.verbose as boolean | undefined);
await workflowStatusCommand(
jsonFlag,
values.verbose as boolean | undefined,
values.events as boolean | undefined
);
break;

case 'get': {
const getRunId = positionals[2];
if (!getRunId) {
console.error('Usage: archon workflow get <run-id> [--json] [--verbose]');
console.error('Usage: archon workflow get <run-id> [--json] [--verbose] [--events]');
return 1;
}
// Propagate the command's exit code so `get <id> && ...` and CI
Expand All @@ -600,7 +606,8 @@ async function main(): Promise<number> {
getRunId,
jsonFlag,
values.verbose as boolean | undefined,
effectiveCwd
effectiveCwd,
values.events as boolean | undefined
);
}

Expand Down
Loading
Loading