feat(traverse): one walk() for any AST; walkSql(text) in plpgsql-parser

[pyramation]

Summary

There were two functions named walk (SQL AST in @pgsql/traverse, PL/pgSQL AST in plpgsql-parser) and no way to walk a SQL string. So the parse+hydrate+iterate harness got hand-rolled 7 times inside @pgsql/transform alone:

// before, ×7
transformSync(sql, (ctx) => {
  for (const stmt of ctx.sql.stmts) walkSql(stmt.stmt, v);          // walkSql = @pgsql/traverse's walk, aliased
  for (const fn of ctx.functions) walkPlpgsql(fn.plpgsql.hydrated, pv,
    { walkSqlExpressions: true, sqlVisitor: v });
}, { hydrate: true });

Now there is one walker per axis:

Function	Package	Walks
walk(ast, visitors, opts?)	@pgsql/traverse	any AST — parsed script, ParseResult, SQL node, PL/pgSQL node, array
walkSqlAst(ast, visitor)	@pgsql/traverse	SQL AST only (the primitive)
walkPlpgsqlAst(ast, visitor, opts?)	@pgsql/traverse	PL/pgSQL AST only (the primitive)
traverse(ast, mutableVisitor)	@pgsql/traverse	SQL AST, mutation-capable
walkSql(text, visitors, opts?)	plpgsql-parser	SQL text — parse + hydrate + walk

The PL/pgSQL walker moved down into @pgsql/traverse (src/plpgsql.ts). That's the enabling move: it only ever needed PL/pgSQL types, which live in plpgsql-deparser (no dep on @pgsql/traverse), not the parser — so no cycle and no WASM in the leaf package. Only text-in traversal stays up in plpgsql-parser, since only it can parse.

walk adds the three things every caller was reimplementing:

const { aborted, reason } = walkSql(sql, [blockedSchemas, readOnlySchemas], {
  RangeVar: (path, ctx) => {
    if (ctx.isWrite && path.node.schemaname === 'audit') ctx.abort('audit is read-only');
    //     ^ statement context: stmtTag, stmtIndex, isWrite, isRead, insideFunction, functionName
  },
  PLpgSQL_stmt_dynexecute: (_p, ctx) => ctx.abort('no dynamic EXECUTE'),
  //  ^ SQL and PL/pgSQL tags in one visitor object
  statement: (path, ctx) => classify(path.tag, ctx.stmtIndex),
  //  ^ reserved key: fires once per top-level statement, before its children
});

return false (skip children) and ctx.abort() (end the walk) are deliberately different mechanisms; visitor arrays let N independent policies share one parse.

One implementation note worth flagging, because the obvious version is wrong: walk does not iterate parseResult.stmts and call walkSqlAst per statement. Doing that skips bare RawStmt entries and every untagged typed field (CreatePolicyStmt.table is a raw RangeVar) that only the runtime schema knows how to reach. Instead it makes a single walkSqlAst pass over the whole parse result and derives statement context on the fly — traversal is depth-first and statements are sequential, so every node after a RawStmt belongs to it.

Removed, no shims: visit() (superseded by walk — no NodePath, no skip-children, and its Object.keys recursion misses untagged typed fields; its tests were converted to walk) and walkParsedScript() (absorbed by walk).

@pgsql/transform is unchanged in behavior: the walk as walkSql aliases became walkSqlAst so walkSql has exactly one meaning ecosystem-wide, and the uniform-visitor harnesses collapsed onto walk. Passes that keep genuine per-statement state (CTE names in qualifyUnqualified, per-statement facts in classifyStatements) still drive the primitives directly. All 252 transform tests and 4 snapshots pass untouched — that is the regression gate.

Docs: @pgsql/traverse, plpgsql-parser and @pgsql/transform READMEs, root README, AGENTS.md, and a new .agents/skills/ast-traversal/SKILL.md covering which function to pick, visitor composition, skip-vs-abort, and the gotchas (hydration, untagged typed fields, loadModule()). The transform README now states the boundary explicitly: transform owns routing/classification/qualification/round-trip policy, @pgsql/traverse owns reaching the nodes.

Also adds @pgsql/traverse to the CI test matrix — its 36 tests weren't running there.

Versions are minor: @pgsql/traverse 18.5.0, plpgsql-parser 18.3.0, @pgsql/transform 18.12.0 (major stays reserved for PostgreSQL versions).

Plan: constructive-io/constructive-planning#1336. After publish, constructive-db deletes parsing/sql-ast (a fork of this that also under-translates schema names in the wire proxy) and points proxy/sql-filter at walkSql.

Testing

pnpm build clean; every package's suite passes (@pgsql/traverse 36, plpgsql-parser 322 incl. 19 new walkSql tests ported from the downstream fork's coverage, @pgsql/transform 252 + 4 snapshots, pgsql-deparser 715, @pgsql/transform-ast 1437, …). pnpm lint is broken repo-wide on main — ESLint 9 is installed but the repo only has .eslintrc.json and no flat config; CI does not lint. Left alone here.

Link to Devin session: https://app.devin.ai/sessions/2aa454e560904773a0a6955712e0aeda
Requested by: @pyramation

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring