mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 08:23:59 +01:00
test(review-army): assert the Red Team branch the run took in the N+1 case
Under the blocking floor the Performance specialist rates a 12-line N+1 INFORMATIONAL, so the conditional Red Team does not activate; the case encoded the old severity (N+1 is CRITICAL, a Red Team always follows). It now asserts the branch the run took: a Red Team that ran must follow the Performance specialist; no Red Team is correct only when the report's own count says 0 critical and its Red Team line names that reason. The N+1 detection assertion is unchanged. The free contract test gains the informational-not-activated and informational-claimed controls and the budget test's synthetic report carries the count line. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
b288a1b705
commit
8eae4db8d3
4 files changed
+27
-11
No files matched your search
@@ -31,6 +31,8 @@
|
||||
- Pass-1 review fixes raised the same two caps again, each with its measurement: ship `maxSizeRatio` 1.7010 → 1.7150 (measured 321,630 / 187,706 = 1.7135) for the lead-judgment stage, the `cause` anchor, the glossary, the stop rules and pass 2's core read; review `maxSizeRatio` 1.2960 → 1.3100 (measured 141,949 / 108,523 = 1.3080) for the shared lead-judgment stage and `cause` anchor. The gstack-review pins on the five hosts that inline the review-army section (cursor, factory, kiro, opencode, slate) go up by 1,396 characters each to their exact rendered sizes; agy, codex and copilot carve the section and are unchanged. The owner's standing decision for these raises is the one that opened the lane (GStack Minder, 2026-10-26: the two wording contributions are inside the "too overzealous" ruling, and a cap raise with a stated reason is the lane's to make). `test/review-workflow-clarity.test.ts`'s merge-stage marker list follows the new stage numbering (eight stages; the Red Team reruns 1–8); no assertion was weakened. `test/review-scope.test.ts`'s `defer` expectation moved from exit 3 to exit 0 because the contract moved (the prose always said `unfiled` keeps the finding deferred), and its `summary` zero-pass expectation from a zero record to a refusal, both by decision, not to pass.
|
||||
- Pass 4 (a fourth review pass, approved by the owner's delegate for five findings at the blocking floor that the Step 11 outside voice and native adversarial pass raised after pass 3, recorded here as the policy requires): a path the policy names under `stamp_files`, `stamp_paths` or `release_outputs` is excluded from the carry's product patch only while its change is a stamp, the rule `lib/tree-receipt.ts` already applies to the same paths; `parseHunks` keeps a deleted file as a whole changed location, so a `cause` anchored in a removed export stays in the diff; `defer` files issues through `lib/gstack-post`'s publication scan, so reviewer text that quotes a credential stays `unfiled (publication scan refused: …)`; `classify` pins `--src-prefix=a/ --dst-prefix=b/ --no-textconv` so `diff.mnemonicPrefix` cannot send every finding outside; `summary` dates a pass from the logger's `review_binding.started_at` before the writer's `timestamp`. `test/review-scope.test.ts`'s deleted-file expectation moved from "dropped" to "whole" because the behaviour changed by decision; the e2e stamp-paths case now writes a real stamp (the old text with the version replaced) because a dropped line was never a stamp.
|
||||
|
||||
- The `review-army-perf-n-plus-one` paid case (rule, gate) encoded the old severity: an N+1 was CRITICAL, so a foreground Red Team always followed the Performance specialist. Under the blocking floor the specialist rates a 12-line N+1 INFORMATIONAL (two focused runs at this head, both `0 critical`, no Red Team: `Red Team: not activated — ... no specialist produced a CRITICAL finding`), so the case now asserts the branch the run took: a Red Team that ran must follow the Performance specialist; no Red Team is correct only when the report's own count says `0 critical` and its Red Team line names that reason. The N+1 detection assertion is unchanged; `test/review-n-plus-one-contract.test.ts` gains the `informational-not-activated` (passes) and `informational-claimed` (fails: a claimed Red Team with no dispatch) controls, and the budget test's synthetic report carries the count line. No threshold moved; the case's kind and trial count are unchanged.
|
||||
|
||||
## [1.91.248.0] - 2026-10-26
|
||||
|
||||
**`$B` in direct mode no longer lets Chromium pick a proxy from the environment (#3440 item 8).** `BROWSE_PROXY_URL=direct` (and an unparseable `HTTPS_PROXY`) only stopped gstack from configuring a proxy. The daemon inherits the whole environment and the launch passed no proxy switch, so Chromium read `HTTPS_PROXY`, `HTTP_PROXY` or `ALL_PROXY` itself and every `goto` still went through that proxy while `$B` reported a direct connection. That made the BROWSER.md precedence list false and any certificate check on a proxied machine unreliable. Reproduced on Linux with Playwright 1.62.1: a logging CONNECT proxy in `HTTPS_PROXY` received `CONNECT <host>:443` with `BROWSE_PROXY_URL=direct` set.
|
||||
|
||||
@@ -8,7 +8,7 @@ const ROOT = path.resolve(import.meta.dir, '..');
|
||||
|
||||
test.each([
|
||||
['consensus', 'review-army-consensus', 'SQL injection'],
|
||||
['N+1', 'review-army-perf-n-plus-one', 'N+1 queries in posts_controller.rb'],
|
||||
['N+1', 'review-army-perf-n-plus-one', 'SPECIALIST REVIEW: 1 findings (1 critical, 0 informational) from 1 specialist\nN+1 queries in posts_controller.rb'],
|
||||
])('%s drains its timed-out capture before Bun retries or removes the fixture', (_label, caseId, report) => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'review-army-finalization-'));
|
||||
try {
|
||||
|
||||
@@ -4,7 +4,7 @@ import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import {spawnSync} from 'node:child_process';
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
test.each(['complete-control', 'captured-omission', 'claimed-only', 'background', 'missing-report', 'unrelated-report', 'captured-timeout', 'performance-omitted', 'performance-background'])
|
||||
test.each(['complete-control', 'captured-omission', 'claimed-only', 'background', 'missing-report', 'unrelated-report', 'captured-timeout', 'performance-omitted', 'performance-background', 'informational-not-activated', 'informational-claimed'])
|
||||
('N+1 registered completion contract: %s', scenario => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'n1-contract-'));
|
||||
const facts = path.join(dir, 'facts.json');
|
||||
@@ -20,6 +20,7 @@ const data=structuredClone(['captured-omission','claimed-only'].includes(scenari
|
||||
if(scenario==='background')data.events[1].message.content[0].input.run_in_background=true;
|
||||
if(scenario==='performance-background')data.events[0].message.content[0].input.run_in_background=true;
|
||||
if(scenario==='performance-omitted')data.events.shift();
|
||||
if(['informational-not-activated','informational-claimed'].includes(scenario))data.events.splice(1,1);
|
||||
const parsed=actual.parseNDJSON(data.events.map(e=>JSON.stringify(e)));
|
||||
let prompt='';
|
||||
mock.module(path.join(root,'test/helpers/e2e-helpers.ts'),()=>({
|
||||
@@ -35,7 +36,10 @@ mock.module(path.join(root,'test/helpers/session-runner.ts'),()=>({...actual,run
|
||||
expect(options.timeout).toBe(300000);expect(options.maxTurns).toBe(20);
|
||||
prompt=options.prompt;
|
||||
if(scenario!=='missing-report')fs.writeFileSync(path.join(options.workingDirectory,'review-output.md'),
|
||||
scenario==='unrelated-report'?'No relevant evidence':'N+1 queries at posts_controller.rb:7 and :9.'+
|
||||
scenario==='unrelated-report'?'No relevant evidence':
|
||||
scenario==='informational-not-activated'?'SPECIALIST REVIEW: 2 findings (0 critical, 2 informational) from 1 specialist\\nN+1 queries at posts_controller.rb:7 and :9.\\nRed Team: not activated — no specialist produced a CRITICAL finding.':
|
||||
scenario==='informational-claimed'?'SPECIALIST REVIEW: 2 findings (0 critical, 2 informational) from 1 specialist\\nN+1 queries at posts_controller.rb:7 and :9.\\nRed Team completed.':
|
||||
'SPECIALIST REVIEW: 1 findings (1 critical, 0 informational) from 1 specialist\\nN+1 queries at posts_controller.rb:7 and :9.'+
|
||||
(scenario==='claimed-only'?' Red Team completed.':''));
|
||||
return {exitReason:scenario==='captured-timeout'?'timeout':'success',browseErrors:[],toolCalls:parsed.toolCalls,
|
||||
transcript:parsed.transcript,output:data.publicAcknowledgement??'Synthetic successful completion control.'};
|
||||
@@ -47,11 +51,12 @@ await import(path.join(root,'test/skill-e2e-review-army.test.ts'));
|
||||
cwd: dir, encoding: 'utf8', timeout: 10_000,
|
||||
env: {PATH: process.env.PATH ?? '', HOME: dir, TMPDIR: dir, NO_COLOR: '1'},
|
||||
});
|
||||
expect(child.status, child.stdout + child.stderr).toBe(scenario === 'complete-control' ? 0 : 1);
|
||||
const passes = scenario === 'complete-control' || scenario === 'informational-not-activated';
|
||||
expect(child.status, child.stdout + child.stderr).toBe(passes ? 0 : 1);
|
||||
expect(child.stderr).not.toContain('Unhandled error between tests');
|
||||
expect(fs.existsSync(facts), child.stdout + child.stderr).toBe(true);
|
||||
const observed = JSON.parse(fs.readFileSync(facts, 'utf8'));
|
||||
expect(observed.passed).toBe(scenario === 'complete-control');
|
||||
expect(observed.passed).toBe(passes);
|
||||
expect(observed.exitReason).toBe(scenario === 'captured-timeout' ? 'timeout' : 'success');
|
||||
expect(observed.prompt).toContain('conditional Red Team dispatch');
|
||||
expect(observed.prompt).toContain('separate foreground Red Team subagent');
|
||||
|
||||
@@ -254,22 +254,31 @@ finish with a brief acknowledgement rather than repeating the findings in the fi
|
||||
let passed = false;
|
||||
try {
|
||||
expect(result.exitReason).toBe('success');
|
||||
expect(result.toolCalls.some(call =>
|
||||
['Agent', 'Task'].includes(call.tool)
|
||||
&& /\bred[ -]team\b/i.test(call.input.description ?? call.input.subagent_type ?? '')
|
||||
&& call.input.run_in_background === false,
|
||||
)).toBe(true);
|
||||
const foreground = result.toolCalls.filter(call =>
|
||||
['Agent', 'Task'].includes(call.tool) && call.input.run_in_background === false);
|
||||
const label = (call: { input: any }) => call.input.description ?? call.input.subagent_type ?? '';
|
||||
const isRedTeam = (call: { input: any }) => /\bred[ -]team\b/i.test(label(call));
|
||||
const performance = foreground.findIndex(call => /\bperformance\b/i.test(label(call)) && !isRedTeam(call));
|
||||
expect(performance).toBeGreaterThanOrEqual(0);
|
||||
expect(foreground.findIndex(isRedTeam)).toBeGreaterThan(performance);
|
||||
|
||||
const outputPath = path.join(dir, 'review-output.md');
|
||||
expect(fs.existsSync(outputPath)).toBe(true);
|
||||
const content = fs.readFileSync(outputPath, 'utf-8').toLowerCase();
|
||||
// The Red Team activates only on a CRITICAL specialist finding (a test that would
|
||||
// fail, a production break, or a security or data-loss defect). An N+1 on a
|
||||
// 12-line diff is usually INFORMATIONAL under that definition, so the case asserts
|
||||
// the branch the run took: a foreground Red Team subagent that ran must follow the
|
||||
// Performance specialist; no Red Team is correct only when the report's own count
|
||||
// says 0 critical and its Red Team line names that reason.
|
||||
const criticalCount = Number(/specialist review:.*?\((\d+) critical/.exec(content)?.[1] ?? NaN);
|
||||
expect(Number.isFinite(criticalCount)).toBe(true);
|
||||
const redTeamIndex = foreground.findIndex(isRedTeam);
|
||||
if (redTeamIndex >= 0) {
|
||||
expect(redTeamIndex).toBeGreaterThan(performance);
|
||||
} else {
|
||||
expect(criticalCount).toBe(0);
|
||||
expect(content).toMatch(/red team[^\n]*(not activated|not dispatched|skipped)[^\n]*critical/);
|
||||
}
|
||||
const hasN1Finding =
|
||||
content.includes('n+1') ||
|
||||
content.includes('n + 1') ||
|
||||
|
||||
Reference in new issue
Block a user