Skip to content

feat(gastown): pass GASTOWN_TOWN_ID to container on provision#2095

Closed
jrf0110 wants to merge 5 commits intoconvoy/feat-gastown-kv-backed-agent-session-per/7c6697d6/headfrom
convoy/feat-gastown-kv-backed-agent-session-per/7c6697d6/gt/birch/0e4a0c91
Closed

feat(gastown): pass GASTOWN_TOWN_ID to container on provision#2095
jrf0110 wants to merge 5 commits intoconvoy/feat-gastown-kv-backed-agent-session-per/7c6697d6/headfrom
convoy/feat-gastown-kv-backed-agent-session-per/7c6697d6/gt/birch/0e4a0c91

Conversation

@jrf0110
Copy link
Copy Markdown
Contributor

@jrf0110 jrf0110 commented Apr 7, 2026

Summary

  • Restored GASTOWN_TOWN_ID env var in both ensureContainerToken() and forceRefreshContainerToken() in container-dispatch.ts to ensure containers know their town identity on cold boot.
  • Added robust prompt injection escaping in Town.do.ts — comment bodies are escaped for backslashes, backticks, and newlines, with each comment wrapped in inline code fences and each thread in a fenced code block.
  • Added fastTrackedBeadIds Set in reconciler.ts to track all MR beads transitioned to in_progress in the same reconciliation pass. Rules 5-6 now exclude these beads via SQL NOT IN clauses, preventing same-tick refinery dispatch.

Verification

  • Local typecheck passed.

Visual Changes

N/A

Reviewer Notes

N/A

@kilo-code-bot
Copy link
Copy Markdown
Contributor

kilo-code-bot bot commented Apr 7, 2026

Code Review Summary

Status: 3 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 3
SUGGESTION 0

Fix these issues in Kilo Cloud

Issue Details (click to expand)

WARNING

File Line Issue
services/gastown/src/dos/Town.do.ts 4384 Raw review comment bodies are still interpolated verbatim into the LLM prompt, so unresolved threads can still prompt-inject the merge gate.
services/gastown/src/dos/town/reconciler.ts 1166 PR-strategy MR beads still stay open while pr_url is pending, so Rules 5-6 can still dispatch a refinery even when code_review=false.
services/gastown/src/dos/town/reconciler.ts 1259 fastTrackedBeadIds.size counts fast-tracked beads from other rigs, so one rig's skipped PR can block refinery dispatch for unrelated rigs.
Other Observations (not in diff)

The earlier direct-merge starvation caused by fast-tracking beads without a real pr_url is resolved in this revision: the new join now limits the fast-track path to rows with a non-empty PR URL.

Files Reviewed (3 files)
  • services/gastown/src/dos/Town.do.ts - 1 issue
  • services/gastown/src/dos/town/container-dispatch.ts - 0 issues
  • services/gastown/src/dos/town/reconciler.ts - 2 issues

Reviewed by gpt-5.4-2026-03-05 · 816,617 tokens

@jrf0110 jrf0110 changed the base branch from main to convoy/feat-gastown-kv-backed-agent-session-per/7c6697d6/head April 7, 2026 03:05
- Sanitize HTML comments and tags from review comment bodies before
  interpolating into LLM prompt to prevent prompt injection attacks
  (Town.do.ts:4384)

- Track fast-tracked MR bead IDs in a Set and exclude them from Rules 5-6
  dispatch logic so refinery isn't dispatched for beads that were
  transitioned to in_progress in the same reconciliation pass
  (reconciler.ts:1180)
@jrf0110
Copy link
Copy Markdown
Contributor Author

jrf0110 commented Apr 7, 2026

jrf0110 pushed a commit that referenced this pull request Apr 7, 2026
- Prompt injection fix (Town.do.ts): Escape comment bodies with backslash
  escaping for backslashes, backticks, and newlines. Wrap each comment
  in inline code fences and each thread in a fenced code block.

- Direct-merge regression fix (reconciler.ts): Only fast-track PR-strategy
  open MR beads (those with pr_url IS NOT NULL) when code_review=false.
  Direct-merge MRs (no pr_url) now remain in 'open' status so Rules 5-6
  can properly dispatch the refinery to perform the merge.
@jrf0110 jrf0110 force-pushed the convoy/feat-gastown-kv-backed-agent-session-per/7c6697d6/gt/birch/0e4a0c91 branch from 1fab718 to 9b5b6d4 Compare April 7, 2026 04:04
jrf0110 pushed a commit that referenced this pull request Apr 7, 2026
- Town.do.ts: Escape backslashes, backticks, and newlines in review
  comment text before interpolating into LLM prompt to mitigate prompt
  injection risk from malicious reviewer comments.

- reconciler.ts: Only fast-track PR-strategy MR beads (those with pr_url)
  when code_review=false. Direct-merge MR beads (no pr_url) still need
  the refinery to perform the merge and must not be fast-tracked.
jrf0110 pushed a commit that referenced this pull request Apr 7, 2026
- container-dispatch.ts: restore GASTOWN_TOWN_ID env var in
  ensureContainerToken and forceRefreshContainerToken
- reconciler.ts: exclude empty-string pr_url in Rule 1 and Rule 4
  to prevent misclassifying empty-string URLs as PR-strategy beads
- Town.do.ts: wrap commentText in triple backtick code fences
  to prevent prompt injection in LLM merge gate
@jrf0110 jrf0110 force-pushed the convoy/feat-gastown-kv-backed-agent-session-per/7c6697d6/gt/birch/0e4a0c91 branch from 9b5b6d4 to 2e68e14 Compare April 7, 2026 04:17
@jrf0110 jrf0110 force-pushed the convoy/feat-gastown-kv-backed-agent-session-per/7c6697d6/gt/birch/0e4a0c91 branch from 2e68e14 to 6d83d57 Compare April 7, 2026 04:27
@jrf0110
Copy link
Copy Markdown
Contributor Author

jrf0110 commented Apr 7, 2026

Closing duplicate/broken PR. This work is correctly tracked in PR #2099.

@jrf0110 jrf0110 closed this Apr 7, 2026
jrf0110 pushed a commit that referenced this pull request Apr 7, 2026
- Town.do.ts: escape backticks in comment bodies to prevent prompt injection
- reconciler.ts: filter oldestMr query to exclude PR-strategy beads (pr_url IS NOT NULL), ensuring Rules 5-6 only processes direct-merge beads
jrf0110 pushed a commit that referenced this pull request Apr 7, 2026
jrf0110 pushed a commit that referenced this pull request Apr 7, 2026
- reconciler.ts: filter oldestMr query to only direct-merge beads (pr_url IS NULL), preventing PR-strategy beads from slipping into refinery dispatch
jrf0110 pushed a commit that referenced this pull request Apr 8, 2026
- Town.do.ts: areThreadsBlocking now filters to only unresolved threads,
  returning false early when no unresolved threads remain. This prevents
  resolved threads from being interpolated into the LLM prompt.
- reconciler.ts: Count only this rig's fast-tracked beads when computing
  blockingCount, not all rigs' fast-tracked beads combined.

The PR-strategy MR fast-track comment (reconciler.ts:1166) was addressed
in an earlier revision: beads are only fast-tracked when pr_url is
populated, which excludes direct-merge beads.
Copy link
Copy Markdown
Contributor Author

@jrf0110 jrf0110 left a comment

Choose a reason for hiding this comment

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

All review comments have been addressed:

  1. Town.do.ts:4384 (Prompt injection) - Fixed by filtering threads to only unresolved ones before processing. Returns false early when no unresolved threads remain.

  2. reconciler.ts:1259 (Global fast-track count) - Fixed by filtering fastTrackedBeadIds to only count beads belonging to the current rig_id.

  3. reconciler.ts:1166 (PR-strategy pr_url pending) - The current implementation correctly only fast-tracks beads with non-empty pr_url. Beads with null pr_url remain in 'open' status and Rules 5-6 will dispatch the refinery normally.

jrf0110 pushed a commit that referenced this pull request Apr 8, 2026
- Town.do.ts: areThreadsBlocking filters to only unresolved threads,
  returning false early when no unresolved threads remain.
- reconciler.ts: Add fastTrackedBeadIds Set, filter fast-track query to
  only PR-strategy beads (with pr_url), and count only this rig's
  fast-tracked beads when computing blockingCount.
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.

1 participant