Skip to content

Phase5 - #6

Merged
MachineLearning-Nerd merged 13 commits into
mainfrom
phase5
Aug 26, 2026
Merged

MachineLearning-Nerd merged 13 commits into
mainfrom
phase5

Conversation

@RKOQWER

@RKOQWER RKOQWER commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

I761449 added 2 commits August 25, 2026 23:33
…I cards

Wire the Need-to-Know release officer as a TrueForge agent:

- src/agent/manifest.ts: buildAgentManifest() produces the POST /api/v1/agents
  payload with require_approval_for_tools: ["release_result"], dynamic subagents
  and Ask User Questions enabled, generative UI enabled, sandbox disabled.

- src/agent/prompt.ts: root agent system prompt embedding the fixed authorized
  purpose/audience literals, the five-step tool flow, three parallel subagent
  roles, hard rules (no raw rows, no canary, release_result only after approved),
  and the three openui card templates (clearance/denial/receipt).

- src/agent/subagents.ts: self-contained prompt constants for the planner,
  privacy-reviewer, and evidence-reviewer subagents. Each returns a typed JSON
  object and is explicitly forbidden from calling vault tools or approving releases.

- src/agent/openui.ts: clearanceCard() and receiptCard() helpers that produce
  the canonical fenced openui blocks the model is prompted to emit. Purpose/audience
  tuple and both hashes are in the non-scrolling header region (EIP-7730 scroll-out
  mitigation); double quotes in values are escaped.

- src/agent/agent.test.ts: 43 tests covering prompt invariants (authorized mission
  literals, approval gate, subagent names, canary absence, ask_user_question
  instruction), subagent prompt invariants (role field, column lists, tool
  prohibition), OpenUI card shape (fenced block, header labels, hash fields,
  quote escaping), and manifest shape (approval gate, enable_tools, feature flags,
  model params).
scripts/start-vault.ts:
  Opens the in-memory vault database, creates the store, and starts the
  Vault MCP server. Accepts --port or $VAULT_PORT (default 8788). Prints
  the listening URL and the TrueForge registration command on stdout.
  Handles SIGTERM/SIGINT with a clean shutdown (server close + db close).
  Uses the same Node 24 --experimental-strip-types loader as verify-receipt.

package.json:
  Add "start-vault" script: node --experimental-strip-types
  --loader=./scripts/ts-loader.mjs scripts/start-vault.ts

README.md:
  Replace the placeholder "instructions landing soon" with a concrete
  three-step getting started section: start the vault, register it and the
  agent in TrueForge, open the UI. Add verify-receipt usage.
  Update Status section to reflect all five phases complete.
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Wire TrueForge agent flow and deterministic receipt verification

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Wire TrueForge agent with parallel safety reviews and human-gated releases.
• Add fail-closed receipt verification for policy, hashes, canaries, and event ordering.
• Provide tested CLIs, OpenUI cards, and end-to-end operating instructions.
Diagram

sequenceDiagram
  actor User
  participant TF as TrueForge
  participant Agent as Release Agent
  participant Reviewers as Safety Reviewers
  participant Vault as Vault MCP
  actor Human
  participant Verifier as Receipt Verifier
  User->>TF: Request aggregate
  TF->>Agent: Run manifest
  Agent->>Vault: Prepare candidate
  par Parallel review
    Agent->>Reviewers: Review plan
  and Contract validation
    Agent->>Vault: Validate release
  end
  Vault-->>Agent: Verdict and hashes
  Agent-->>TF: Clearance card
  TF->>Human: Request approval
  Human-->>TF: Approve release
  TF->>Vault: Release result
  Vault-->>User: Receipt bundle
  User->>Verifier: Verify receipt
  Verifier-->>User: Typed outcome
Loading
High-Level Assessment

The layered approach is appropriate: deterministic vault policy remains authoritative, subagents are advisory, TrueForge supplies the human execution gate, and verification independently reuses the contract library. Agent-only enforcement or a separate verification policy implementation would create weaker controls or policy drift.

Files changed (13) +1418 / -3

Enhancement (7) +656 / -0
manifest.tsDefine the TrueForge release-agent manifest +79/-0

Define the TrueForge release-agent manifest

• Builds a deterministic model configuration with dynamic subagents, user questions, generative UI, and a preloaded vault connection. Critically, release_result is configured as human approval-gated.

src/agent/manifest.ts

openui.tsGenerate deterministic clearance and receipt cards +85/-0

Generate deterministic clearance and receipt cards

• Adds typed builders for approved, denied, needs-review, and released OpenUI presentations. Cards escape interpolated values and expose authorization, findings, hashes, suppression, and receipt metadata.

src/agent/openui.ts

prompt.tsEncode the authorized release workflow in the root prompt +95/-0

Encode the authorized release workflow in the root prompt

• Defines strict mission literals, ordered vault calls, parallel reviewer responsibilities, human-gated release behavior, data-boundary rules, and canonical OpenUI output formats.

src/agent/prompt.ts

subagents.tsAdd advisory planner, privacy, and evidence reviewers +55/-0

Add advisory planner, privacy, and evidence reviewers

• Defines structured prompts for three tool-free subagents that assess mission fit, sensitive-column absence, group-size enforcement, and suppression disclosure without authorizing releases.

src/agent/subagents.ts

cli.tsAdd the verify-receipt command-line interface +72/-0

Add the verify-receipt command-line interface

• Reads a verifiable receipt from a file or stdin, invokes deterministic verification, prints concise PASS or FAIL output, and distinguishes verification failures from input I/O errors.

src/verify/cli.ts

receipt.tsDefine verifiable receipt inputs and typed outcomes +57/-0

Define verifiable receipt inputs and typed outcomes

• Introduces the receipt bundle schema, exact-key constants, relevant TrueForge event names, and exhaustive pass or failure outcome types used by the verifier.

src/verify/receipt.ts

verify.tsImplement fail-closed receipt and event verification +213/-0

Implement fail-closed receipt and event verification

• Strictly parses receipt bundles, delegates candidate and hash checks to the release contract, scans candidates and events for canaries, and validates approval-before-response ordering. Every unexpected condition maps to a typed result instead of escaping as an exception.

src/verify/verify.ts

Tests (2) +629 / -0
agent.test.tsTest agent safety, manifest, and OpenUI invariants +257/-0

Test agent safety, manifest, and OpenUI invariants

• Covers authorized mission literals, tool ordering, subagent restrictions, deterministic card shapes, escaping, human approval configuration, and TrueForge feature flags.

src/agent/agent.test.ts

verify.test.tsExercise positive, adversarial, and event-order verification cases +372/-0

Exercise positive, adversarial, and event-order verification cases

• Builds real receipts through vault handlers and tests valid bundles, malformed structures, hash tampering, policy denial, canary leakage, missing or duplicate approvals, release ordering, and non-throwing hostile inputs.

src/verify/verify.test.ts

Documentation (1) +45 / -2
README.mdDocument the complete agent and receipt-verification workflow +45/-2

Document the complete agent and receipt-verification workflow

• Adds setup instructions for starting and registering the Vault MCP server and TrueForge agent. Documents receipt verification and updates project status to reflect all five completed phases.

README.md

Other (3) +88 / -1
package.jsonExpose vault startup and receipt verification commands +3/-1

Expose vault startup and receipt verification commands

• Adds npm scripts that run the Vault MCP server and verify-receipt CLI directly from TypeScript sources through the custom Node loader.

package.json

start-vault.tsAdd a runnable Vault MCP server entry point +67/-0

Add a runnable Vault MCP server entry point

• Creates an in-memory synthetic vault, wires store and handlers, starts the loopback MCP server, validates port input, and closes resources on termination signals.

scripts/start-vault.ts

ts-loader.mjsResolve TypeScript sources for buildless Node execution +18/-0

Resolve TypeScript sources for buildless Node execution

• Adds a Node ESM loader that remaps relative .js imports to existing .ts files, enabling the TypeScript CLIs to run without a build step.

scripts/ts-loader.mjs

@qodo-code-review

qodo-code-review Bot commented Aug 26, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Turn history order reversed ✗ Dismissed 🐞 Bug ≡ Correctness ⭐ New
Description
listSessionTurnIds expects the first listed turn to have no predecessor and each later turn to
point backward to the prior array element, but TrueForge lists session turns newest first. Every
normal multi-turn Gate A session is therefore rejected as events_partial, preventing live receipt
verification and Gate B from succeeding.
Code

src/verify/events.ts[R147-150]

+        if (
+          !isSafeTrueForgeId(row.id) ||
+          state !== "done" ||
+          row.previous_turn_id !== previousTurnId
Relevance

●●● Strong

Recent verifier reviews accepted clear correctness fixes; reversing this API-order invariant is a
deterministic fail-closed bug.

PR-#5

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The implementation initializes previousTurnId to null for the first response row and then requires
each row's predecessor to equal the previously appended ID, which is an oldest-first invariant.
TrueForge's turn-list API explicitly documents that turns are returned newest first and pagination
walks backward through the ancestor chain; Gate A creates multiple turns and depends on this loader
for persisted evidence.

src/verify/events.ts[135-172]
scripts/gate-a.ts[86-116]
scripts/gate-a.ts[179-203]
🌐 The List turns API states that session turns are returned newest first and pagination walks the ancestor chain from the latest turn.

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
TrueForge returns session turns newest first, but `listSessionTurnIds` validates them as an oldest-first chain. This rejects valid multi-turn sessions and breaks the live verification gates.

## Issue Context
For newest-first rows, each row's `previous_turn_id` should identify the next row in the returned list; only the oldest/final row has a null predecessor. After validating all pages and cross-page links in that direction, return the IDs oldest first so event concatenation preserves chronological order.

## Fix Focus Areas
- src/verify/events.ts[144-172]
- src/verify/events.test.ts[112-170]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Session turns can be omitted ✓ Resolved 🐞 Bug ⛨ Security
Description
fetchSessionEvents fetches only the bundle-supplied turnIds and never compares them with the
session's actual turn list, so a crafted receipt can omit persisted turns containing a denial,
duplicate gate, or canary leak while still passing live verification. This contradicts the
verifier's claim to validate the persisted session evidence completely.
Code

src/verify/events.ts[R80-83]

+  for (const turnId of turnIds) {
+    const result = await fetchTurnEvents(baseUrl, sessionId, turnId);
+    if (!result.ok) return result;
+    events.push(...result.events);
Relevance

●●● Strong

Team consistently accepted fail-closed evidence/session-binding fixes in verify.ts history (PR #5).

PR-#5

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The CLI passes the untrusted evidence list directly into the fetcher, and the fetcher merely
iterates that list. Gate A separately calls the session turns endpoint to discover turn IDs, proving
the authoritative list exists, while the verifier never performs that discovery or checks that no
turns were omitted.

src/verify/cli.ts[68-84]
src/verify/events.ts[73-85]
scripts/gate-a.ts[96-103]
src/verify/verify.ts[106-109]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Live receipt verification trusts the receipt bundle's caller-supplied `turnIds`, allowing persisted turns to be omitted from approval and canary checks.

## Issue Context
Fetch the authoritative turn list from TrueForge, paginate it fail-closed, and either use that full list or require it to exactly match the evidence list before fetching events.

## Fix Focus Areas
- src/verify/events.ts[73-85]
- src/verify/cli.ts[65-88]
- src/verify/events.test.ts[84-94]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Charting precedes required release ✓ Resolved 🐞 Bug ≡ Correctness
Description
The root prompt requires render_safe_chart before validation and release_result, but the MCP
handler rejects charting until a release receipt exists. An agent following the mandated order stops
with not_released and never reaches the approval/release steps.
Code

src/agent/prompt.ts[R36-39]

+4. **render_safe_chart** — once subagents return and findings are clean, call with the queryId from prepare_analysis.
+5. **validate_release** — call with the queryId from prepare_analysis. Check the returned verdict.
+6. Emit a clearance or denial **openui block** (see card format below) with the verdict, findings, purpose, audience, queryId, contractHash, outputHash, and suppressed-cell count.
+7. **release_result** — call ONLY when validate_release returns status "approved". This call is approval-gated: the turn will pause for a human to inspect the authorization tuple and hashes before the vault executes the release. After the human approves, the vault writes the release receipt.
Evidence
The prompt places charting at step 4 and release at step 7, while the actual handler explicitly
requires store.getReceipt(queryId) and returns not_released without it; the MCP description
states the same contract.

src/agent/prompt.ts[36-40]
src/server/handlers.ts[253-273]
src/server/mcp.ts[109-117]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The prescribed tool sequence calls `render_safe_chart` before the receipt required by that tool exists.

## Issue Context
The chart handler returns `not_released` until `release_result` succeeds.

## Fix Focus Areas
- src/agent/prompt.ts[36-40]
- src/server/handlers.ts[253-273]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View high (4)
4. Event indices become misaligned ✓ Resolved 🐞 Bug ≡ Correctness
Description
The verifier filters missing event types into a shorter sequence array, then indexes that array
using positions from the original events array. For example, `[{}, tool.response,
tool.approval_required, user.tool_approval]` hides the response and can pass even though execution
precedes approval.
Code

src/verify/verify.ts[R77-80]

+  // Extract event types in sequence order.
+  const sequence: string[] = events
+    .map((event) => (typeof event.type === "string" ? event.type : ""))
+    .filter((type) => type.length > 0);
Evidence
parseEvents accepts records without a string type; sequence removes them, but the later loops
range over events.length and read sequence[index], so later response positions are shifted or
omitted.

src/verify/verify.ts[53-63]
src/verify/verify.ts[77-80]
src/verify/verify.ts[114-139]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Filtering event types destroys positional correspondence with the original event stream.

## Issue Context
Determine event types directly from each original event at its real index, or preserve placeholders without filtering.

## Fix Focus Areas
- src/verify/verify.ts[77-80]
- src/verify/verify.ts[114-139]
- src/verify/verify.test.ts[302-337]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Approval events are uncorrelated ✓ Resolved 🐞 Bug ⛨ Security
Description
The ordering check accepts any earlier approval event for any later tool response and never matches
the response's tool_call_id to an approved call. Thus approval for tc-1 followed by a release
response for tc-2 is certified as valid approval evidence.
Code

src/verify/verify.ts[R130-133]

+  // Every tool.response must be preceded by at least one approval event.
+  for (const responsePos of responsePositions) {
+    const priorApproval = approvalPositions.some((approvalPos) => approvalPos < responsePos);
+    if (!priorApproval) {
Evidence
Although approval tool-call IDs are collected, response IDs and user-approval IDs are never compared
against that set; the final condition only asks whether any approval position is earlier than a
response position.

src/verify/verify.ts[82-107]
src/verify/verify.ts[114-139]
src/verify/verify.test.ts[78-109]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Approval and execution events are not correlated by tool-call identity.

## Issue Context
Track approval-required, user-allow, and response events per tool call and ensure the checked call is `release_result`.

## Fix Focus Areas
- src/verify/verify.ts[82-139]
- src/verify/verify.test.ts[302-337]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Human approval is never checked ✓ Resolved 🐞 Bug ⛨ Security
Description
checkEvents collects user.tool_approval positions but never uses them or verifies an allow
decision. An event stream containing only tool.approval_required followed by tool.response
therefore passes despite containing no evidence that a human approved execution.
Code

src/verify/verify.ts[R114-120]

+  // Find the position of any user.tool_approval (resume) event.
+  const resumePositions: number[] = [];
+  for (let index = 0; index < events.length; index++) {
+    if (sequence[index] === TF_EVENT_USER_TOOL_APPROVAL) {
+      resumePositions.push(index);
+    }
+  }
Evidence
The code computes resumePositions but never checks it; the only execution-order condition requires
a prior approval-required notification, not a human approval. The test fixture shows the intended
human event shape includes approval.status: "allow".

src/verify/verify.ts[109-141]
src/verify/verify.test.ts[95-109]
README.md[15-18]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The verifier does not require a human `user.tool_approval` allow event before execution.

## Issue Context
Validate the approval payload and ordering for the same release tool call, and reject missing, denied, or late decisions.

## Fix Focus Areas
- src/verify/verify.ts[114-141]
- src/verify/verify.test.ts[302-337]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. Receipt metadata remains unbound ✓ Resolved 🐞 Bug ≡ Correctness
Description
verifyReceipt validates only the candidate against the receipt's hashes and never compares the
receipt's queryId, datasetVersion, or policyVersion with their candidate counterparts.
Changing any of those receipt fields to another non-empty string still returns pass, so the CLI
can certify and print a falsely relabeled receipt.
Code

src/verify/verify.ts[R163-165]

+    // Validate the candidate through the release contract and recompute hashes.
+    // verifyRelease calls validateRelease internally, then checks both hashes.
+    const verdict = verifyRelease(candidate, receipt.contractHash, receipt.outputHash);
Evidence
Receipt parsing requires only non-empty strings, and verifyRelease receives only the candidate and
two hashes. Candidates already carry provenance query ID and dataset/policy versions, while server
receipt generation copies those exact values, establishing the required equality.

src/verify/verify.ts[16-26]
src/verify/verify.ts[163-203]
src/contract/validate.ts[10-20]
src/server/handlers.ts[129-135]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Receipt identity and version metadata can be changed independently of the hash-verified candidate.

## Issue Context
Compare receipt metadata with the parsed candidate before returning `pass`, and add tampering tests for every duplicated field.

## Fix Focus Areas
- src/verify/verify.ts[163-203]
- src/verify/verify.test.ts[194-244]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

8. Conflicts preserve stale configuration ✗ Dismissed 🐞 Bug ☼ Reliability
Description
tolerateConflict treats every 409 as successful setup without checking or updating the existing
provider or MCP server. Rerunning with a rotated API key, changed model endpoint, or changed vault
URL therefore exits successfully while TrueForge continues using the stale configuration.
Code

scripts/setup-trueforge.ts[R49-52]

+  } catch (error) {
+    if ((error as { status?: number }).status === 409) {
+      process.stdout.write(`setup-trueforge: ${label} already registered\n`);
+      return;
Relevance

●●● Strong

Stale credentials/endpoints on rerun is a real reliability bug; reconciliation fits agent branch
pattern.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The environment controls provider credentials, model endpoint/model ID, and vault URL, but those
values are only sent in POST requests. A 409 returns immediately without reconciliation, unlike the
agent branch which explicitly updates the existing resource by ID.

scripts/setup-trueforge.ts[18-21]
scripts/setup-trueforge.ts[44-55]
scripts/setup-trueforge.ts[72-101]
scripts/setup-trueforge.ts[103-116]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Setup silently accepts conflicting provider and MCP registrations, leaving existing credentials and endpoints unchanged while reporting success.

## Issue Context
On conflict, look up the existing registration and update it by ID, or compare its effective manifest with the requested configuration and fail clearly when they differ.

## Fix Focus Areas
- scripts/setup-trueforge.ts[44-56]
- scripts/setup-trueforge.ts[72-101]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


9. Registration command is invalid ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The documented agent-registration curl posts {...}, which is not valid JSON and is rejected
before any manifest can be registered. Because buildAgentManifest is only a TypeScript function
and no command prints its result, the advertised full-agent setup flow is not directly runnable as
written.
Code

README.md[R52-56]

+# Register the agent (run once; see src/agent/manifest.ts for the full schema)
+# Replace <provider> and <model-id> with your TrueForge model provider and model.
+curl -s -X POST http://localhost:8890/api/v1/agents \
+  -H 'content-type: application/json' \
+  -d '{"name":"need-to-know","manifest":{...}}'
Evidence
The code block presented as the registration step literally sends an ellipsis inside JSON. The
referenced manifest builder returns an object but this PR adds no package script or CLI that invokes
it for the user.

README.md[44-59]
src/agent/manifest.ts[48-79]
package.json[15-22]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The full-flow setup includes an invalid JSON registration request.

## Issue Context
Add a script that emits/posts `buildAgentManifest`, or replace the placeholder with a complete valid payload and concrete provider/model substitution instructions.

## Fix Focus Areas
- README.md[52-59]
- src/agent/manifest.ts[48-79]
- package.json[15-22]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


10. Stdin path breaks Windows ✓ Resolved 🐞 Bug ☼ Reliability
Description
The CLI reads piped input through the Unix-only /dev/stdin path. On Windows, invoking
verify-receipt without a file or with - fails with an I/O error and exit code 2 instead of
verifying the input.
Code

src/verify/cli.ts[R35-36]

+  // Read from stdin synchronously.
+  return readFileSync("/dev/stdin", "utf8");
Evidence
Both omitted-path and - modes hard-code /dev/stdin, and the surrounding catch converts the
resulting filesystem failure to exit code 2. The package declares Node >=24 but no Unix-only
platform restriction.

src/verify/cli.ts[30-47]
package.json[5-8]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Piped receipt verification is not portable to Windows.

## Issue Context
Use `readFileSync(0, "utf8")` or `process.stdin` rather than `/dev/stdin`, and add a stdin execution test.

## Fix Focus Areas
- src/verify/cli.ts[30-37]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View medium (1)
11. OpenUI values allow injection ✓ Resolved 🐞 Bug ⛨ Security
Description
escapeValue escapes quotes but not backslashes or control characters before embedding
caller-controlled findings and card fields into OpenUI source. A value containing a backslash before
a quote plus a newline can terminate the argument and inject or corrupt subsequent OpenUI
components.
Code

src/agent/openui.ts[R29-31]

+function escapeValue(value: string | number): string {
+  return String(value).replace(/"/g, '\\"');
+}
Evidence
The encoder performs only quote replacement, while purpose/audience/query fields and finding details
are inserted directly into executable-looking OpenUI expressions. Finding details can include
clipped caller-controlled contract values and are not otherwise escaped.

src/agent/openui.ts[29-31]
src/agent/openui.ts[43-62]
src/contract/findings.ts[3-10]
src/contract/queryPlan.ts[32-55]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Dynamic OpenUI values are not encoded as safe string literals.

## Issue Context
Use a complete serializer such as `JSON.stringify` for every interpolated string and test backslashes, newlines, tabs, and quote combinations.

## Fix Focus Areas
- src/agent/openui.ts[29-31]
- src/agent/openui.ts[43-62]
- src/agent/agent.test.ts[160-164]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Web pages:
  +9 more
Review mode: ⚖️ Balanced: Comparison failure: full PR diff.

Grey Divider

Tip of the day
💡 Did you know, you can start a comment with 'qodo' or '@qodo' to chat about any finding

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit 8f91d7e

Results up to commit fd6c8b9 ⚖️ Balanced


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Action required
1. Human approval is never checked ✓ Resolved 🐞 Bug ⛨ Security
Description
checkEvents collects user.tool_approval positions but never uses them or verifies an allow
decision. An event stream containing only tool.approval_required followed by tool.response
therefore passes despite containing no evidence that a human approved execution.
Code

src/verify/verify.ts[R114-120]

+  // Find the position of any user.tool_approval (resume) event.
+  const resumePositions: number[] = [];
+  for (let index = 0; index < events.length; index++) {
+    if (sequence[index] === TF_EVENT_USER_TOOL_APPROVAL) {
+      resumePositions.push(index);
+    }
+  }
Evidence
The code computes resumePositions but never checks it; the only execution-order condition requires
a prior approval-required notification, not a human approval. The test fixture shows the intended
human event shape includes approval.status: "allow".

src/verify/verify.ts[109-141]
src/verify/verify.test.ts[95-109]
README.md[15-18]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The verifier does not require a human `user.tool_approval` allow event before execution.

## Issue Context
Validate the approval payload and ordering for the same release tool call, and reject missing, denied, or late decisions.

## Fix Focus Areas
- src/verify/verify.ts[114-141]
- src/verify/verify.test.ts[302-337]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Receipt metadata remains unbound ✓ Resolved 🐞 Bug ≡ Correctness
Description
verifyReceipt validates only the candidate against the receipt's hashes and never compares the
receipt's queryId, datasetVersion, or policyVersion with their candidate counterparts.
Changing any of those receipt fields to another non-empty string still returns pass, so the CLI
can certify and print a falsely relabeled receipt.
Code

src/verify/verify.ts[R163-165]

+    // Validate the candidate through the release contract and recompute hashes.
+    // verifyRelease calls validateRelease internally, then checks both hashes.
+    const verdict = verifyRelease(candidate, receipt.contractHash, receipt.outputHash);
Evidence
Receipt parsing requires only non-empty strings, and verifyRelease receives only the candidate and
two hashes. Candidates already carry provenance query ID and dataset/policy versions, while server
receipt generation copies those exact values, establishing the required equality.

src/verify/verify.ts[16-26]
src/verify/verify.ts[163-203]
src/contract/validate.ts[10-20]
src/server/handlers.ts[129-135]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Receipt identity and version metadata can be changed independently of the hash-verified candidate.

## Issue Context
Compare receipt metadata with the parsed candidate before returning `pass`, and add tampering tests for every duplicated field.

## Fix Focus Areas
- src/verify/verify.ts[163-203]
- src/verify/verify.test.ts[194-244]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Approval events are uncorrelated ✓ Resolved 🐞 Bug ⛨ Security
Description
The ordering check accepts any earlier approval event for any later tool response and never matches
the response's tool_call_id to an approved call. Thus approval for tc-1 followed by a release
response for tc-2 is certified as valid approval evidence.
Code

src/verify/verify.ts[R130-133]

+  // Every tool.response must be preceded by at least one approval event.
+  for (const responsePos of responsePositions) {
+    const priorApproval = approvalPositions.some((approvalPos) => approvalPos < responsePos);
+    if (!priorApproval) {
Evidence
Although approval tool-call IDs are collected, response IDs and user-approval IDs are never compared
against that set; the final condition only asks whether any approval position is earlier than a
response position.

src/verify/verify.ts[82-107]
src/verify/verify.ts[114-139]
src/verify/verify.test.ts[78-109]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Approval and execution events are not correlated by tool-call identity.

## Issue Context
Track approval-required, user-allow, and response events per tool call and ensure the checked call is `release_result`.

## Fix Focus Areas
- src/verify/verify.ts[82-139]
- src/verify/verify.test.ts[302-337]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View high (2)
4. Charting precedes required release ✓ Resolved 🐞 Bug ≡ Correctness
Description
The root prompt requires render_safe_chart before validation and release_result, but the MCP
handler rejects charting until a release receipt exists. An agent following the mandated order stops
with not_released and never reaches the approval/release steps.
Code

src/agent/prompt.ts[R36-39]

+4. **render_safe_chart** — once subagents return and findings are clean, call with the queryId from prepare_analysis.
+5. **validate_release** — call with the queryId from prepare_analysis. Check the returned verdict.
+6. Emit a clearance or denial **openui block** (see card format below) with the verdict, findings, purpose, audience, queryId, contractHash, outputHash, and suppressed-cell count.
+7. **release_result** — call ONLY when validate_release returns status "approved". This call is approval-gated: the turn will pause for a human to inspect the authorization tuple and hashes before the vault executes the release. After the human approves, the vault writes the release receipt.
Evidence
The prompt places charting at step 4 and release at step 7, while the actual handler explicitly
requires store.getReceipt(queryId) and returns not_released without it; the MCP description
states the same contract.

src/agent/prompt.ts[36-40]
src/server/handlers.ts[253-273]
src/server/mcp.ts[109-117]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The prescribed tool sequence calls `render_safe_chart` before the receipt required by that tool exists.

## Issue Context
The chart handler returns `not_released` until `release_result` succeeds.

## Fix Focus Areas
- src/agent/prompt.ts[36-40]
- src/server/handlers.ts[253-273]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Event indices become misaligned ✓ Resolved 🐞 Bug ≡ Correctness
Description
The verifier filters missing event types into a shorter sequence array, then indexes that array
using positions from the original events array. For example, `[{}, tool.response,
tool.approval_required, user.tool_approval]` hides the response and can pass even though execution
precedes approval.
Code

src/verify/verify.ts[R77-80]

+  // Extract event types in sequence order.
+  const sequence: string[] = events
+    .map((event) => (typeof event.type === "string" ? event.type : ""))
+    .filter((type) => type.length > 0);
Evidence
parseEvents accepts records without a string type; sequence removes them, but the later loops
range over events.length and read sequence[index], so later response positions are shifted or
omitted.

src/verify/verify.ts[53-63]
src/verify/verify.ts[77-80]
src/verify/verify.ts[114-139]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Filtering event types destroys positional correspondence with the original event stream.

## Issue Context
Determine event types directly from each original event at its real index, or preserve placeholders without filtering.

## Fix Focus Areas
- src/verify/verify.ts[77-80]
- src/verify/verify.ts[114-139]
- src/verify/verify.test.ts[302-337]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended
6. Registration command is invalid ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The documented agent-registration curl posts {...}, which is not valid JSON and is rejected
before any manifest can be registered. Because buildAgentManifest is only a TypeScript function
and no command prints its result, the advertised full-agent setup flow is not directly runnable as
written.
Code

README.md[R52-56]

+# Register the agent (run once; see src/agent/manifest.ts for the full schema)
+# Replace <provider> and <model-id> with your TrueForge model provider and model.
+curl -s -X POST http://localhost:8890/api/v1/agents \
+  -H 'content-type: application/json' \
+  -d '{"name":"need-to-know","manifest":{...}}'
Evidence
The code block presented as the registration step literally sends an ellipsis inside JSON. The
referenced manifest builder returns an object but this PR adds no package script or CLI that invokes
it for the user.

README.md[44-59]
src/agent/manifest.ts[48-79]
package.json[15-22]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The full-flow setup includes an invalid JSON registration request.

## Issue Context
Add a script that emits/posts `buildAgentManifest`, or replace the placeholder with a complete valid payload and concrete provider/model substitution instructions.

## Fix Focus Areas
- README.md[52-59]
- src/agent/manifest.ts[48-79]
- package.json[15-22]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. Stdin path breaks Windows ✓ Resolved 🐞 Bug ☼ Reliability
Description
The CLI reads piped input through the Unix-only /dev/stdin path. On Windows, invoking
verify-receipt without a file or with - fails with an I/O error and exit code 2 instead of
verifying the input.
Code

src/verify/cli.ts[R35-36]

+  // Read from stdin synchronously.
+  return readFileSync("/dev/stdin", "utf8");
Evidence
Both omitted-path and - modes hard-code /dev/stdin, and the surrounding catch converts the
resulting filesystem failure to exit code 2. The package declares Node >=24 but no Unix-only
platform restriction.

src/verify/cli.ts[30-47]
package.json[5-8]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Piped receipt verification is not portable to Windows.

## Issue Context
Use `readFileSync(0, "utf8")` or `process.stdin` rather than `/dev/stdin`, and add a stdin execution test.

## Fix Focus Areas
- src/verify/cli.ts[30-37]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


8. OpenUI values allow injection ✓ Resolved 🐞 Bug ⛨ Security
Description
escapeValue escapes quotes but not backslashes or control characters before embedding
caller-controlled findings and card fields into OpenUI source. A value containing a backslash before
a quote plus a newline can terminate the argument and inject or corrupt subsequent OpenUI
components.
Code

src/agent/openui.ts[R29-31]

+function escapeValue(value: string | number): string {
+  return String(value).replace(/"/g, '\\"');
+}
Evidence
The encoder performs only quote replacement, while purpose/audience/query fields and finding details
are inserted directly into executable-looking OpenUI expressions. Finding details can include
clipped caller-controlled contract values and are not otherwise escaped.

src/agent/openui.ts[29-31]
src/agent/openui.ts[43-62]
src/contract/findings.ts[3-10]
src/contract/queryPlan.ts[32-55]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Dynamic OpenUI values are not encoded as safe string literals.

## Issue Context
Use a complete serializer such as `JSON.stringify` for every interpolated string and test backslashes, newlines, tabs, and quote combinations.

## Fix Focus Areas
- src/agent/openui.ts[29-31]
- src/agent/openui.ts[43-62]
- src/agent/agent.test.ts[160-164]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit 1ad24b5 ⚖️ Balanced


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Action required
1. Session turns can be omitted ✓ Resolved 🐞 Bug ⛨ Security
Description
fetchSessionEvents fetches only the bundle-supplied turnIds and never compares them with the
session's actual turn list, so a crafted receipt can omit persisted turns containing a denial,
duplicate gate, or canary leak while still passing live verification. This contradicts the
verifier's claim to validate the persisted session evidence completely.
Code

src/verify/events.ts[R80-83]

+  for (const turnId of turnIds) {
+    const result = await fetchTurnEvents(baseUrl, sessionId, turnId);
+    if (!result.ok) return result;
+    events.push(...result.events);
Relevance

●●● Strong

Team consistently accepted fail-closed evidence/session-binding fixes in verify.ts history (PR #5).

PR-#5

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The CLI passes the untrusted evidence list directly into the fetcher, and the fetcher merely
iterates that list. Gate A separately calls the session turns endpoint to discover turn IDs, proving
the authoritative list exists, while the verifier never performs that discovery or checks that no
turns were omitted.

src/verify/cli.ts[68-84]
src/verify/events.ts[73-85]
scripts/gate-a.ts[96-103]
src/verify/verify.ts[106-109]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Live receipt verification trusts the receipt bundle's caller-supplied `turnIds`, allowing persisted turns to be omitted from approval and canary checks.

## Issue Context
Fetch the authoritative turn list from TrueForge, paginate it fail-closed, and either use that full list or require it to exactly match the evidence list before fetching events.

## Fix Focus Areas
- src/verify/events.ts[73-85]
- src/verify/cli.ts[65-88]
- src/verify/events.test.ts[84-94]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended
2. Conflicts preserve stale configuration ✗ Dismissed 🐞 Bug ☼ Reliability
Description
tolerateConflict treats every 409 as successful setup without checking or updating the existing
provider or MCP server. Rerunning with a rotated API key, changed model endpoint, or changed vault
URL therefore exits successfully while TrueForge continues using the stale configuration.
Code

scripts/setup-trueforge.ts[R49-52]

+  } catch (error) {
+    if ((error as { status?: number }).status === 409) {
+      process.stdout.write(`setup-trueforge: ${label} already registered\n`);
+      return;
Relevance

●●● Strong

Stale credentials/endpoints on rerun is a real reliability bug; reconciliation fits agent branch
pattern.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The environment controls provider credentials, model endpoint/model ID, and vault URL, but those
values are only sent in POST requests. A 409 returns immediately without reconciliation, unlike the
agent branch which explicitly updates the existing resource by ID.

scripts/setup-trueforge.ts[18-21]
scripts/setup-trueforge.ts[44-55]
scripts/setup-trueforge.ts[72-101]
scripts/setup-trueforge.ts[103-116]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Setup silently accepts conflicting provider and MCP registrations, leaving existing credentials and endpoints unchanged while reporting success.

## Issue Context
On conflict, look up the existing registration and update it by ID, or compare its effective manifest with the requested configuration and fail clearly when they differ.

## Fix Focus Areas
- scripts/setup-trueforge.ts[44-56]
- scripts/setup-trueforge.ts[72-101]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread src/agent/prompt.ts Outdated
Comment thread src/verify/verify.ts
Comment thread src/verify/verify.ts Outdated
Comment thread src/verify/verify.ts Outdated
Comment thread src/verify/verify.ts Outdated
Comment thread src/agent/openui.ts Outdated
Comment thread src/verify/cli.ts Outdated
Comment thread README.md Outdated
Gate A caught this live: the prompt told the model to call
render_safe_chart before release_result, but the vault only serves
charts for released aggregates, so every run hit a guaranteed
not_released error — and the hard rules order the model to stop on any
error. One run pushed through by disobeying its own stop rule; the
next halted and asked the user, ending the turn without ever reaching
the approval gate. The chart step now follows the release, with the
vault's actual constraint stated beside it.

AI-assisted (Claude Code); reviewed and understood by the author.
A VerifiableReceipt can now carry an evidence section naming the
session and turns that produced it. Naming a session raises the bar:
events become required, any event carrying session or turn ids must
agree with the evidence, and some gated post-approval tool.response
must witness the exact receipt — id and both hashes — because
trueforge 0.1.4 responds to a denied gated call too, so mere
execution-after-pause proves nothing. A denial leaves its error marker
on the response and fails; a stream from another run cannot contain
this receipt and fails as receipt_unwitnessed.

The CLI fetches persisted events live when TRUEFORGE_BASE_URL is set:
paginated at the server's 100-event cap, failing closed as
events_partial when pagination cannot be proven honoured and
events_unavailable on any fetch error. Events without a string type
now fail closed as events_malformed instead of being skipped —
skipped records would let evidence hide inside the stream unexamined.

user.tool_approval events are not persisted by trueforge 0.1.4, so
the explicit-grant rule applies when a log carries them and the
structural rules above carry the proof when it does not.

AI-assisted (Claude Code); reviewed and understood by the author.
Registers the ZAI model provider and the vault MCP server tolerating
409 as already-registered, then upserts the agent by id: list agents,
PUT /agents/{id} with {manifest} when the name exists, POST /agents
when absent. The agents API on trueforge 0.1.4 is id-routed —
name-based routes silently no-op. Base URLs use localhost throughout;
0.1.4 listens on IPv6 only and refuses 127.0.0.1.

AI-assisted (Claude Code); reviewed and understood by the author.
Gate A runs the full flow twice against a live TrueForge with the
vault started in-process as ground truth. The deny path proves the
pause happened and the vault recorded zero release transitions and
zero receipts; the allow path proves exactly one receipt. Both
sessions' persisted events are then refetched and the allow stream
must witness the exact receipt — persisted approval events carry
tool_calls ids only, no tool names, so gated calls are matched by id
to their responses. The deny stream must carry the user-denial marker
and no receipt. Writes the allow run as a VerifiableReceipt bundle
with its session evidence.

Gate B refetches the bundle's persisted events and checks the
boundary: canary email and free text absent from every event byte,
no sensitive column keys, customer ids, email addresses, or phone
patterns in any vault JSON tool response (TrueForge's own openui
syntax documentation is the one non-JSON tool response and carries a
placeholder address that is not vault data), and verifyReceipt passes
on the refetched events through the same path the CLI uses.

Gate artifacts (gate-*.json) are run evidence, not source, and are
excluded from Biome.

AI-assisted (Claude Code); reviewed and understood by the author.
@MachineLearning-Nerd

MachineLearning-Nerd commented Aug 26, 2026 •

Copy link
Copy Markdown
Owner

Phase 4 + Phase 5 completion

This PR now carries both phases end to end. Branch synced with main (all 14 PR #5 review fixes included via merge e871757).

Phase 4 — TrueForge wiring: exit check MET, live

  • T4.1 scripts/setup-trueforge.ts — ZAI provider registration (409-tolerant), vault MCP server registration, id-routed agent upsert (list → PUT /agents/{id} {manifest} on name match, POST /agents when absent). Both create and update paths exercised live.
  • T4.2 was already here (manifest/prompts). Gate A caught one defect in it live: the prompt ordered render_safe_chart before release, but the vault only charts after release — the guaranteed not_released error plus the prompt's own stop-on-error rule made runs non-deterministically halt before the approval gate. Fixed in 55f2bad.
  • T4.3 scripts/gate-a.ts — PASS, 13/13 live assertions on pinned trueforge 0.1.4 at http://localhost:8891: deny path pauses and leaves zero release transitions, zero receipts (vault store in-process as ground truth); allow path yields exactly one receipt; persisted events refetched for both sessions; the allow stream must witness the exact receipt (id + both hashes) in the gated post-approval response; the deny stream must carry the user-denial marker and no receipt.
  • T4.4 scripts/gate-b.ts — PASS, 9/9: canary email + free text absent from every persisted event byte; no sensitive column keys, customer ids, emails, or phone patterns in any vault JSON tool response; verifyReceipt passes on the refetched events.

Phase 5 — evidence: exit check MET

  • T5.2 VerifiableReceipt gains an evidence section (sessionId, turnIds) binding the receipt to the run that produced it; Gate A writes it into every bundle.
  • T5.3 the CLI fetches persisted events live (TRUEFORGE_BASE_URL), paginated at the server's 100-event cap, failing closed as events_partial when pagination cannot be proven honoured and events_unavailable on any fetch error. Verified in both modes against a real release: embedded events → PASS, live fetch (33 events) → PASS.
  • T5.4 negative enumeration now includes: missing events on a session-bound bundle (events_missing), partial/truncated fetch, unavailable fetch, mismatched session/turn ids (session_mismatch), a stream that never witnesses the receipt (receipt_unwitnessed), a recorded user denial, type-less event records (events_malformed — previously skipped silently), plus the existing malformed/hash/ordering/duplicate/canary cases. 54 verify-suite tests, 217 total.

Empirical findings worth knowing

  • trueforge 0.1.4 does not persist user.tool_approval events, and a denied gated call still produces a tool.response (with a User denied tool call error). So execution-after-pause proves nothing by itself — the verifier instead requires the gated response to carry the exact receipt (only a granted release can), and treats the denial marker as an explicit fail. When a log does carry user approval events, the strict grant-between-request-and-response rule applies.
  • Persisted approval events carry tool_calls[].id only — no tool names — so gated calls are matched by id.

Deliberate scope decisions (declines with reasons)

  • T5.1 separate ledger schema: not built as a distinct module — the vault store's append-only audit log (findings, outcomes) + one-receipt-per-queryId + the bundle's session evidence together carry every field T5.1 enumerates except "approver identity", which trueforge 0.1.4 does not expose in any persisted event. Building a parallel ledger would duplicate the store.
  • Unknown string-typed events tolerated: real streams carry many types (model.message, mcp.initialize, thread.*); enumerating them would break on every runtime addition. Records without a string type fail closed instead.
  • Assertion-count publication belongs to the README pass (T9.3).

Gate artifacts (gate-a-bundle.json, gate-*-result.json) are run evidence, excluded from Biome, uncommitted — banking them is Phase 7/9 scope.

AI-assisted (Claude Code); reviewed and understood by the author.
@MachineLearning-Nerd
MachineLearning-Nerd requested a balanced review from Copilot August 26, 2026 06:55
@MachineLearning-Nerd

Copy link
Copy Markdown
Owner

/review

Comment thread src/verify/events.ts
Comment thread scripts/setup-trueforge.ts
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 1ad24b5

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The evidence/witness verification and live-fetch security logic is safety-critical and non-trivial, and one latent boundary-scan issue plus a docs/port inconsistency warrant human review before approval.

Pull request overview

This PR (Phase 5) wires the Need-to-Know release officer into a TrueForge agent and completes the persisted-evidence verification path. It builds on the existing verify-receipt CLI by adding session/turn "evidence" binding: a bundle that names its TrueForge session must carry (or live-fetch) the persisted events that witness the receipt, and the verifier fails closed on partial fetches, mismatched sessions, or an unwitnessed receipt. It also adds the agent manifest/prompt/subagents/OpenUI card layer plus operational scripts (start-vault, setup-trueforge, gate-b).

Changes:

  • Added evidence (sessionId + turnIds) to VerifiableReceipt, with new outcomes (events_missing, session_mismatch, receipt_unwitnessed) and structural approval evidence for persisted streams that don't record the user's decision.
  • New paginated, fail-closed event fetcher (events.ts) used by the CLI and gate scripts to verify against server-persisted events.
  • New agent wiring: root prompt, three advisory subagents, OpenUI card builders, agent manifest with require_approval_for_tools: ["release_result"], plus setup/start/gate scripts and README docs.
File summaries
File Description
src/verify/verify.ts Parses/validates evidence; enforces type-per-event, session match, and receipt witnessing; structural denial fallback
src/verify/receipt.ts Adds evidence type, new outcomes, and VERIFIABLE_OPTIONAL_KEYS
src/verify/events.ts Paginated fail-closed persisted-event fetcher
src/verify/events.test.ts Tests pagination, offset-ignored, and unavailable paths
src/verify/verify.test.ts Tests session-bound evidence, denial marker, unrecognised types
src/verify/cli.ts Async main; live-fetches events when evidence + base URL present
src/agent/prompt.ts, subagents.ts, openui.ts, manifest.ts Root/subagent prompts, card builders, and TrueForge manifest
src/agent/agent.test.ts Invariant tests for prompts, cards, manifest
scripts/start-vault.ts, setup-trueforge.ts, gate-b.ts Vault launcher, TrueForge registration, Gate B boundary checks
README.md, package.json, package-lock.json, biome.json Run docs, scripts, SDK dep, ignore gate result files
Review details
  • Files reviewed: 17/19 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/gate-b.ts Outdated
Comment thread README.md Outdated
escapeValue escaped quotes only, so a value ending in a backslash
neutralised its own closing quote and a newline started a fresh OpenUI
statement — injection, not formatting. Backslashes are escaped first,
then quotes, and control characters flatten to spaces since statements
are single-line. Regression test verified failing against the old
escape.

The CLI's live mode fetched only the turns the bundle claimed, so a
crafted bundle could omit the turn holding a denial or a canary leak
and still verify. The session's actual turn list is now fetched
(paginated at the 25-turn cap, same fail-closed rules) and must equal
the claimed list exactly; events are then fetched from the server's
list, never the bundle's. Verified live: the genuine Gate A bundle
passes, the same bundle with one turn omitted fails session_mismatch.

setup-trueforge's 409 path now says what it does: existing provider
and MCP configuration is kept, not updated — 0.1.4 settings routes
have no validated update path and name-based routes can silently
no-op, so rotation requires deleting the old entry first. The README's
unrunnable curl with a {...} placeholder is replaced by the
setup-trueforge command.

Found by Qodo review on PR #6.

AI-assisted (Claude Code); reviewed and understood by the author.
Verifier: a receipt now verifies only against the full vault chain — one
root-thread prepare/validate/release call trio from the vault MCP server,
arguments and responses cross-checked against the candidate, one approval
gate resolving to the release call, one user allow between gate and
response, and the released payload witnessed exactly. Evidence names the
session, its inline agent, and a duplicate-free turn list; ids are
format-validated and URL-encoded before any fetch, and event/turn
pagination follows the server's page tokens, failing closed on overlap,
missing metadata, or a turn list that changes under the fetch.

Live mode authenticates the named session: it must exist, be owned by an
inline (frozen) agent, and match the pinned manifest on the fields that
matter — instructions, model, no seeded messages, the single gated vault
server, subagents and sandbox disabled. Server-added defaults are ignored
so a runtime default change cannot false-fail every bundle. Offline mode
is an explicit CLI choice announced on stderr, never a silent downgrade.

Agent: canary literals no longer appear in the prompt; release_result now
carries the full approval tuple (purpose, audience, columns, suppressed
cells, both hashes) which the vault revalidates at execution time
(approval_tuple_mismatch); OpenUI cards use the real 0.1.4 grammar;
dynamic subagents are removed and disabled — 0.1.4 children inherit the
root's vault tools and would bypass the root-only release rule. Receipt
ids are random UUIDs. setup-trueforge validates existing non-secret
settings on 409 instead of silently keeping them.

Proven live: Gate A and Gate B pass against the running TrueForge;
verify-receipt passes in live and offline modes on the Gate A bundle, and
a bundle with one turn omitted fails session_mismatch. 253 unit tests,
tsc and biome clean. Gate artifacts are run evidence and stay untracked.

AI-assisted (Claude Code); reviewed and understood by the author.
Phase 5's exit check asks for the enumerated negative-test count to be
published: 89 verifier tests across verify, event-fetch, live-session,
and boundary suites.

AI-assisted (Claude Code); reviewed and understood by the author.
Seeds the Phase 9 limitations doc with the boundaries found during the
completeness review: approval is structurally evidenced but not
identity-bound (standalone 0.1.4 has no auth and no reason field), denied
attempts are vault-audited but session-linked only through gate run
artifacts until the Phase 9 published runs, and offline verification is
announced as weaker than live.

AI-assisted (Claude Code); reviewed and understood by the author.
@MachineLearning-Nerd

Copy link
Copy Markdown
Owner

/review

Comment thread src/verify/events.ts
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 5789a1e

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The PR is large and touches security-critical fail-closed release verification whose correctness depends on subtle event-ordering and evidence-binding invariants that warrant final human review.

Review details
  • Files reviewed: 34/36 changed files
  • Comments generated: 3
  • Review effort level: Balanced

Comment thread scripts/gate-a-actions.ts Outdated
Comment thread src/verify/verify.ts Outdated
Comment thread README.md Outdated
Move gate-a-actions imports to the top of the file, give the malformed-
candidate verdict a descriptive detail instead of echoing its outcome
name, and correct the README: the frozen inline agent spec is checked
once, while the turn list is the thing compared before and after the
event refetch.

AI-assisted (Claude Code); reviewed and understood by the author.
@MachineLearning-Nerd
MachineLearning-Nerd merged commit cd3434f into main Aug 26, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants