Skip to content

Commit 4b9e7fe

Browse files
committed
Merge branch 'claude/postgres-quality-framework-FOIwg' into 'main'
Add PostgreSQL quality engineering framework Closes #158 See merge request postgres-ai/postgresai!231
2 parents 4bd4318 + b969b3f commit 4b9e7fe

File tree

19 files changed

+4016
-130
lines changed

19 files changed

+4016
-130
lines changed

.claude/CLAUDE.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ git submodule update --remote .cursor
1717

1818
- **README.md** — Project overview, features, and quick start
1919
- **CONTRIBUTING.md** — Local development workflow, Docker setup, debugging
20+
- **quality/QUALITY_ENGINEERING_GUIDE.md** — Quality standards, processes, automated gates
21+
- **quality/pr-review-prompt.md** — AI PR review system prompt (PostgreSQL-specific)
22+
- **quality/failure-modes.md** — Critical failure modes with required test coverage
23+
- **quality/checklists/** — PR review and release checklists
2024

2125
### Commands
2226

@@ -44,3 +48,15 @@ postgresai mon local-install --demo
4448
| H004 | Redundant indexes |
4549
| F004 | Table bloat |
4650
| K003 | Top queries |
51+
52+
## Quality
53+
54+
```bash
55+
# Run release readiness check
56+
./quality/scripts/release-readiness.sh
57+
58+
# Full check (includes integration tests)
59+
./quality/scripts/release-readiness.sh --full
60+
```
61+
62+
See `quality/QUALITY_ENGINEERING_GUIDE.md` for the full quality framework.

.gitlab-ci.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
include:
22
- local: 'components/index_pilot/.gitlab-ci.yml'
3+
- local: 'quality/gitlab-ci-quality.yml'
34
- template: Security/SAST.gitlab-ci.yml
45
- project: 'postgres-ai/infra'
56
file: '/ci/templates/approval-check.yml'

.pre-commit-config.yaml

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,42 @@ repos:
33
rev: v8.30.0
44
hooks:
55
- id: gitleaks
6+
7+
- repo: https://github.com/pre-commit/pre-commit-hooks
8+
rev: v5.0.0
9+
hooks:
10+
# Prevent large files from being committed
11+
- id: check-added-large-files
12+
args: ['--maxkb=500']
13+
# Ensure JSON files are valid
14+
- id: check-json
15+
# Ensure YAML files are valid
16+
- id: check-yaml
17+
args: ['--allow-multiple-documents']
18+
# Prevent committing to main directly
19+
- id: no-commit-to-branch
20+
args: ['--branch', 'main']
21+
# Ensure files end with newline
22+
- id: end-of-file-fixer
23+
# Remove trailing whitespace
24+
- id: trailing-whitespace
25+
args: ['--markdown-linebreak-ext=md']
26+
27+
- repo: local
28+
hooks:
29+
# Catch potential SQL injection patterns in TypeScript
30+
- id: sql-injection-check
31+
name: SQL injection check (TypeScript)
32+
entry: bash -c 'grep -rn --include="*.ts" -E "(\`SELECT|\`INSERT|\`UPDATE|\`DELETE|\`DROP|\`ALTER|\`CREATE).*\$\{" "$@" && echo "ERROR: Possible SQL injection — use parameterized queries" && exit 1 || exit 0' --
33+
language: system
34+
files: '\.(ts|js)$'
35+
exclude: '(test|spec|__tests__)'
36+
pass_filenames: false
37+
38+
# Verify JSON schemas are valid
39+
- id: validate-schemas
40+
name: Validate JSON schemas
41+
entry: python3 -c "import json, sys; [json.load(open(f)) for f in sys.argv[1:]]"
42+
language: system
43+
files: '\.schema\.json$'
44+
types: [json]

cli/bin/postgres-ai.ts

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2452,16 +2452,20 @@ mon
24522452

24532453
// Test connection
24542454
console.log("Testing connection to the added instance...");
2455-
try {
2456-
const client = new Client({ connectionString: connStr });
2457-
await client.connect();
2458-
const result = await client.query("select version();");
2459-
console.log("✓ Connection successful");
2460-
console.log(`${result.rows[0].version}\n`);
2461-
await client.end();
2462-
} catch (error) {
2463-
const message = error instanceof Error ? error.message : String(error);
2464-
console.error(`✗ Connection failed: ${message}\n`);
2455+
{
2456+
let testClient: InstanceType<typeof Client> | null = null;
2457+
try {
2458+
testClient = new Client({ connectionString: connStr, connectionTimeoutMillis: 10000 });
2459+
await testClient.connect();
2460+
const result = await testClient.query("select version();");
2461+
console.log("✓ Connection successful");
2462+
console.log(`${result.rows[0].version}\n`);
2463+
} catch (error) {
2464+
const message = error instanceof Error ? error.message : String(error);
2465+
console.error(`✗ Connection failed: ${message}\n`);
2466+
} finally {
2467+
if (testClient) await testClient.end();
2468+
}
24652469
}
24662470
} else if (opts.yes) {
24672471
// Auto-yes mode without database URL - skip database setup
@@ -2496,16 +2500,20 @@ mon
24962500

24972501
// Test connection
24982502
console.log("Testing connection to the added instance...");
2499-
try {
2500-
const client = new Client({ connectionString: connStr });
2501-
await client.connect();
2502-
const result = await client.query("select version();");
2503-
console.log("✓ Connection successful");
2504-
console.log(`${result.rows[0].version}\n`);
2505-
await client.end();
2506-
} catch (error) {
2507-
const message = error instanceof Error ? error.message : String(error);
2508-
console.error(`✗ Connection failed: ${message}\n`);
2503+
{
2504+
let testClient: InstanceType<typeof Client> | null = null;
2505+
try {
2506+
testClient = new Client({ connectionString: connStr, connectionTimeoutMillis: 10000 });
2507+
await testClient.connect();
2508+
const result = await testClient.query("select version();");
2509+
console.log("✓ Connection successful");
2510+
console.log(`${result.rows[0].version}\n`);
2511+
} catch (error) {
2512+
const message = error instanceof Error ? error.message : String(error);
2513+
console.error(`✗ Connection failed: ${message}\n`);
2514+
} finally {
2515+
if (testClient) await testClient.end();
2516+
}
25092517
}
25102518
}
25112519
} else {
@@ -3292,7 +3300,7 @@ targets
32923300
console.log(`Testing connection to monitoring target '${name}'...`);
32933301

32943302
// Use native pg client instead of requiring psql to be installed
3295-
const client = new Client({ connectionString: instance.conn_str });
3303+
const client = new Client({ connectionString: instance.conn_str, connectionTimeoutMillis: 10000 });
32963304

32973305
try {
32983306
await client.connect();

cli/lib/init.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,10 @@ export async function connectWithSslFallback(
127127
verbose?: boolean
128128
): Promise<{ client: PgClient; usedSsl: boolean }> {
129129
const tryConnect = async (config: PgClientConfig): Promise<PgClient> => {
130-
const client = new ClientClass(config);
130+
const client = new ClientClass({ ...config, connectionTimeoutMillis: 10_000 } as any);
131131
await client.connect();
132+
// Set a default statement timeout to prevent runaway queries
133+
await client.query("SET statement_timeout = '30s'");
132134
return client;
133135
};
134136

@@ -454,8 +456,18 @@ export function resolveAdminConnection(opts: {
454456
return { clientConfig: cfg, display: describePgConfig(cfg), sslFallbackEnabled: true };
455457
}
456458

459+
/**
460+
* Generate a cryptographically secure random password for the monitoring role.
461+
*
462+
* Encoding note — bytes vs output length:
463+
* - hex: N bytes → 2N characters (24 bytes → 48 hex chars)
464+
* - base64: N bytes → ⌈4N/3⌉ chars (24 bytes → 32 base64url chars, no padding)
465+
*
466+
* We use base64url (RFC 4648 §5) because it is shorter than hex and safe in URLs,
467+
* connection strings, and shell variables without quoting.
468+
*/
457469
function generateMonitoringPassword(): string {
458-
// URL-safe and easy to copy/paste; 24 bytes => 32 base64url chars (no padding).
470+
// 24 random bytes 32 base64url characters (no padding).
459471
// Note: randomBytes() throws on failure; we add a tiny sanity check for unexpected output.
460472
const password = randomBytes(24).toString("base64url");
461473
if (password.length < 30) {

cli/test/init.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1314,3 +1314,56 @@ describe.skipIf(!dockerAvailable)("imageTag priority behavior", () => {
13141314
expect(envContent).toMatch(/GF_SECURITY_ADMIN_PASSWORD=secret123/);
13151315
}, 60000);
13161316
});
1317+
1318+
// ---------------------------------------------------------------------------
1319+
// connectWithSslFallback — connectionTimeoutMillis and statement_timeout
1320+
// Issues 9 & 10
1321+
// ---------------------------------------------------------------------------
1322+
describe("connectWithSslFallback", () => {
1323+
// Issue 9: Verify that connectionTimeoutMillis is forwarded to the pg Client
1324+
// constructor so slow-responding servers don't hang the CLI indefinitely.
1325+
// Direct integration testing against a real TCP timeout would be flaky in CI,
1326+
// so we use a mock ClientClass and assert the config passed to its constructor.
1327+
test("passes connectionTimeoutMillis: 10_000 to the pg Client constructor", async () => {
1328+
const receivedConfigs: any[] = [];
1329+
1330+
class MockClient {
1331+
constructor(config: any) {
1332+
receivedConfigs.push(config);
1333+
}
1334+
async connect() {}
1335+
async query() { return {}; }
1336+
}
1337+
1338+
const adminConn = init.resolveAdminConnection({ conn: "postgresql://u:p@localhost:5432/d" });
1339+
// Disable SSL fallback so we exercise the simple (non-retry) path.
1340+
(adminConn as any).sslFallbackEnabled = false;
1341+
1342+
await init.connectWithSslFallback(MockClient as any, adminConn);
1343+
1344+
expect(receivedConfigs.length).toBeGreaterThanOrEqual(1);
1345+
expect(receivedConfigs[0].connectionTimeoutMillis).toBe(10_000);
1346+
});
1347+
1348+
// Issue 10: Verify that SET statement_timeout is issued after every successful
1349+
// connection to prevent runaway queries from blocking the CLI.
1350+
test("issues SET statement_timeout = '30s' after connecting", async () => {
1351+
const queriesSent: string[] = [];
1352+
1353+
class MockClient {
1354+
constructor(_config: any) {}
1355+
async connect() {}
1356+
async query(sql: string) {
1357+
queriesSent.push(sql);
1358+
return {};
1359+
}
1360+
}
1361+
1362+
const adminConn = init.resolveAdminConnection({ conn: "postgresql://u:p@localhost:5432/d" });
1363+
(adminConn as any).sslFallbackEnabled = false;
1364+
1365+
await init.connectWithSslFallback(MockClient as any, adminConn);
1366+
1367+
expect(queriesSent.some((q) => /SET\s+statement_timeout/i.test(q))).toBe(true);
1368+
});
1369+
});

0 commit comments

Comments
 (0)