docs: add session handover for 2026-05-31#89
Closed
B67687 wants to merge 6 commits into
Closed
Conversation
…routing, ADR patterns, and CI gate recommendations
…ection to AGENTS.md
…max reasoning, update architecture.md
…fallback chain, pre-flight check script
Reviewer's GuideDocuments the current agent-seed deployment on the MiniPC, introduces an architect/worker two-tier agent architecture and ADR system for agentic-workflows, and adds a dated session handover plus cost guard tooling for Pro models. Sequence diagram for Pro model cost guard before architect delegationsequenceDiagram
participant Orchestrator
participant ProCostGuard as pro_cost_guard_sh
participant Opencode as opencode_stats
participant Architect
participant Pro as deepseek_v4_pro
participant Flash as deepseek_v4_flash
Orchestrator->>ProCostGuard: pro-cost-guard.sh
ProCostGuard->>Opencode: opencode stats --days 1
Opencode-->>ProCostGuard: current spend
ProCostGuard-->>Orchestrator: {status, monthMax, message}
alt status == PASS or WARN
Orchestrator->>Architect: delegate @architect
Architect->>Pro: architecture_request
Pro-->>Architect: architecture_response
else status == BLOCK
Orchestrator->>Architect: delegate @architect (fallback)
Architect->>Flash: architecture_request (reasoning_effort max)
Flash-->>Architect: architecture_response
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR establishes an ADR-based governance system for recording architecture decisions, implements two-tier model routing (strategic vs. tactical agents on different model tiers), defines a four-layer cost defense to control expensive model usage, documents the complete agent architecture with safety layers and daemon loop phases, and provides deployment guidance and session continuity procedures. ChangesArchitecture Governance, Model Routing, and Cost Controls
Agent Architecture, Safety Layers, and Operational Workflows
Deployment, Session Continuity, and Operational Setup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The
pro-cost-guard.shscript only ever emitsstatusvalues ofpass/warnand never actually inspects current spend or enforces a ceiling, which doesn’t match the ADR’s description of PASS/WARN/BLOCK behavior—consider wiring it intoopencode statsand emitting a true BLOCK state when projected or actual spend exceedsPRO_MONTHLY_MAX. - agent-seed architecture docs and the new session handover describe different daemon implementations (640-line safety-layered daemon vs a 40-line minimal loop); consider explicitly distinguishing “target architecture” vs “current deployment” in one place to avoid confusion about which design is authoritative.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `pro-cost-guard.sh` script only ever emits `status` values of `pass`/`warn` and never actually inspects current spend or enforces a ceiling, which doesn’t match the ADR’s description of PASS/WARN/BLOCK behavior—consider wiring it into `opencode stats` and emitting a true BLOCK state when projected or actual spend exceeds `PRO_MONTHLY_MAX`.
- agent-seed architecture docs and the new session handover describe different daemon implementations (640-line safety-layered daemon vs a 40-line minimal loop); consider explicitly distinguishing “target architecture” vs “current deployment” in one place to avoid confusion about which design is authoritative.
## Individual Comments
### Comment 1
<location path="scripts/tools/pro-cost-guard.sh" line_range="55-56" />
<code_context>
+ MESSAGE="${MESSAGE} Cannot verify current spend (opencode stats unavailable)."
+fi
+
+cat <<EOF
+{"status":"${MODE}","monthMax":${CEILING},"message":"${MESSAGE}"}
+EOF
+
</code_context>
<issue_to_address>
**issue (bug_risk):** MESSAGE is interpolated into JSON without escaping, which can produce invalid JSON if it ever contains double quotes or newlines.
This is currently safe only because MESSAGE happens to avoid double quotes/newlines. Any future content change (quotes, backslashes, newlines) will break the JSON. Please construct the JSON via a tool that handles escaping (e.g., jq or a small python -c json.dumps helper), or add a shell escape helper to safely encode MESSAGE before interpolation.
</issue_to_address>
### Comment 2
<location path="scripts/tools/pro-cost-guard.sh" line_range="21-26" />
<code_context>
+
+COST_GUARD="${HOME}/.config/opencode/cost-guard.config.json"
+# Read ceiling from env or cost-guard config
+if [ -n "${PRO_MONTHLY_MAX:-}" ]; then
+ CEILING="$PRO_MONTHLY_MAX"
+elif [ -f "$COST_GUARD" ]; then
+ CEILING=$(python3 -c "import json; print(json.load(open('$COST_GUARD')).get('maxCostUsd', 30))" 2>/dev/null || echo "30")
+else
+ CEILING="30"
+fi
+
</code_context>
<issue_to_address>
**issue (bug_risk):** monthMax is emitted as a bare number but CEILING is never validated, so non-numeric input will lead to invalid JSON.
Because CEILING is never validated and monthMax is emitted unquoted, any non-numeric value (e.g. `PRO_MONTHLY_MAX=30usd` or a bad `maxCostUsd` in the config) will produce invalid JSON and can break consumers. Please either validate CEILING as numeric (and fall back to a safe default) before emitting it bare, or emit it as a quoted JSON string so arbitrary values remain syntactically valid JSON.
</issue_to_address>
### Comment 3
<location path="docs/architecture.md" line_range="28-29" />
<code_context>
+├─────────────────────────────────────────────────────────────┤
+│ Subagent Delegation Layer │
+├──────────┬──────────┬──────────┬──────────┬──────────┬──────┤
+│ Architect│ Worker │ Review │ Plan │ Explore │ Scout│
+│ v4-flash │ free │ free │ free │ free │ free │
+│ reasn:h │ none │ none │ none │ none │ none │
</code_context>
<issue_to_address>
**issue (typo):** Typo in architecture diagram: `reasn:h` should likely be `reason:h` or similar.
Please correct the `reasn:h` label in the diagram (e.g., to `reason:h` or `reason: h`) to keep the architecture snapshot clear and consistent.
```suggestion
│ v4-flash │ free │ free │ free │ free │ free │
│ reason:h │ none │ none │ none │ none │ none │
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+55
to
+56
| cat <<EOF | ||
| {"status":"${MODE}","monthMax":${CEILING},"message":"${MESSAGE}"} |
There was a problem hiding this comment.
issue (bug_risk): MESSAGE is interpolated into JSON without escaping, which can produce invalid JSON if it ever contains double quotes or newlines.
This is currently safe only because MESSAGE happens to avoid double quotes/newlines. Any future content change (quotes, backslashes, newlines) will break the JSON. Please construct the JSON via a tool that handles escaping (e.g., jq or a small python -c json.dumps helper), or add a shell escape helper to safely encode MESSAGE before interpolation.
Comment on lines
+21
to
+26
| if [ -n "${PRO_MONTHLY_MAX:-}" ]; then | ||
| CEILING="$PRO_MONTHLY_MAX" | ||
| elif [ -f "$COST_GUARD" ]; then | ||
| CEILING=$(python3 -c "import json; print(json.load(open('$COST_GUARD')).get('maxCostUsd', 30))" 2>/dev/null || echo "30") | ||
| else | ||
| CEILING="30" |
There was a problem hiding this comment.
issue (bug_risk): monthMax is emitted as a bare number but CEILING is never validated, so non-numeric input will lead to invalid JSON.
Because CEILING is never validated and monthMax is emitted unquoted, any non-numeric value (e.g. PRO_MONTHLY_MAX=30usd or a bad maxCostUsd in the config) will produce invalid JSON and can break consumers. Please either validate CEILING as numeric (and fall back to a safe default) before emitting it bare, or emit it as a quoted JSON string so arbitrary values remain syntactically valid JSON.
Comment on lines
+28
to
+29
| │ v4-flash │ free │ free │ free │ free │ free │ | ||
| │ reasn:h │ none │ none │ none │ none │ none │ |
There was a problem hiding this comment.
issue (typo): Typo in architecture diagram: reasn:h should likely be reason:h or similar.
Please correct the reasn:h label in the diagram (e.g., to reason:h or reason: h) to keep the architecture snapshot clear and consistent.
Suggested change
| │ v4-flash │ free │ free │ free │ free │ free │ | |
| │ reasn:h │ none │ none │ none │ none │ none │ | |
| │ v4-flash │ free │ free │ free │ free │ free │ | |
| │ reason:h │ none │ none │ none │ none │ none │ |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request establishes a comprehensive architecture documentation framework, introducing a living architecture snapshot, research on a two-tier agent architecture, and several Architecture Decision Records (ADRs) covering the tier system, model routing, and budget policies. It also adds a pre-flight cost guard script (pro-cost-guard.sh) to prevent budget overruns when delegating to premium models. Feedback on the changes highlights critical issues in the cost guard script, including case mismatches in status outputs, missing block logic for budget checks, and unsafe shell expansion in Python commands, along with a robust code suggestion to address these flaws.
Comment on lines
+20
to
+59
| # Read ceiling from env or cost-guard config | ||
| if [ -n "${PRO_MONTHLY_MAX:-}" ]; then | ||
| CEILING="$PRO_MONTHLY_MAX" | ||
| elif [ -f "$COST_GUARD" ]; then | ||
| CEILING=$(python3 -c "import json; print(json.load(open('$COST_GUARD')).get('maxCostUsd', 30))" 2>/dev/null || echo "30") | ||
| else | ||
| CEILING="30" | ||
| fi | ||
|
|
||
| MODE="pass" | ||
| MESSAGE="" | ||
|
|
||
| # Check cost-guard mode | ||
| if [ -f "$COST_GUARD" ]; then | ||
| GUARD_MODE=$(python3 -c "import json; print(json.load(open('$COST_GUARD')).get('mode','warn'))" 2>/dev/null || echo "unknown") | ||
| GUARD_MAX=$(python3 -c "import json; print(json.load(open('$COST_GUARD')).get('maxCostUsd',0))" 2>/dev/null || echo "0") | ||
|
|
||
| if [ "$GUARD_MODE" = "block" ]; then | ||
| MODE="pass" | ||
| MESSAGE="Cost guard active at \$${GUARD_MAX} block mode. Pro approved if within ceiling." | ||
| else | ||
| MODE="warn" | ||
| MESSAGE="Cost guard mode is '${GUARD_MODE}' --- set to 'block' for full protection." | ||
| fi | ||
| else | ||
| MODE="warn" | ||
| MESSAGE="No cost-guard.config.json found --- Pro spend has no hard ceiling." | ||
| fi | ||
|
|
||
| # Check if we can reach opencode stats (sanity check) | ||
| if ! opencode stats --days 1 >/dev/null 2>&1; then | ||
| MODE="warn" | ||
| MESSAGE="${MESSAGE} Cannot verify current spend (opencode stats unavailable)." | ||
| fi | ||
|
|
||
| cat <<EOF | ||
| {"status":"${MODE}","monthMax":${CEILING},"message":"${MESSAGE}"} | ||
| EOF | ||
|
|
||
| exit 0 |
There was a problem hiding this comment.
There are several critical issues in the current implementation of pro-cost-guard.sh:
- Case Mismatch in Status Values: The script outputs lowercase status values (
"pass"and"warn"), but the documentation and ADR-0004 explicitly expect uppercase values ("PASS","WARN","BLOCK"). This could break integrations or bypass the fallback mechanism. - Missing
BLOCKLogic: The script never actually performs the budget check or spend comparison, meaning it will never return"BLOCK". This directly violates the confirmation criteria in ADR-0004 (e.g.,PRO_MONTHLY_MAX=0must returnBLOCK). - Unsafe Shell Expansion in Python Command: Passing
$COST_GUARDdirectly inside the single-quoted Python command string can cause syntax errors or shell injection if the path (which depends on$HOME) contains spaces or single quotes. Passing it viasys.argvis much safer.
We can resolve these issues by parsing the spend from opencode stats and performing a robust comparison in Python.
# Read ceiling from env or cost-guard config
if [ -n "${PRO_MONTHLY_MAX:-}" ]; then
CEILING="$PRO_MONTHLY_MAX"
elif [ -f "$COST_GUARD" ]; then
CEILING=$(python3 -c "import sys, json; print(json.load(open(sys.argv[1])).get('maxCostUsd', 30))" "$COST_GUARD" 2>/dev/null || echo "30")
else
CEILING="30"
fi
MODE="PASS"
MESSAGE=""
# Check cost-guard mode
if [ -f "$COST_GUARD" ]; then
GUARD_MODE=$(python3 -c "import sys, json; print(json.load(open(sys.argv[1])).get('mode','warn'))" "$COST_GUARD" 2>/dev/null || echo "unknown")
GUARD_MAX=$(python3 -c "import sys, json; print(json.load(open(sys.argv[1])).get('maxCostUsd',0))" "$COST_GUARD" 2>/dev/null || echo "0")
if [ "$GUARD_MODE" = "block" ]; then
MODE="PASS"
MESSAGE="Cost guard active at \$${GUARD_MAX} block mode. Pro approved if within ceiling."
else
MODE="WARN"
MESSAGE="Cost guard mode is '${GUARD_MODE}' --- set to 'block' for full protection."
fi
else
MODE="WARN"
MESSAGE="No cost-guard.config.json found --- Pro spend has no hard ceiling."
fi
# Query spend from opencode stats
SPEND="0.0"
if opencode stats --days 1 >/dev/null 2>&1; then
SPEND=$(opencode stats --days 1 2>/dev/null | python3 -c "import sys, re; match = re.search(sys.argv[1], sys.stdin.read()); print(match.group(1) if match else sys.argv[2])" "([0-9]+\.[0-9]+)" "0.0")
else
MODE="WARN"
MESSAGE="${MESSAGE} Cannot verify current spend (opencode stats unavailable)."
fi
# Perform budget check
if ! python3 -c "import sys; spend, ceiling = float(sys.argv[1]), float(sys.argv[2]); sys.exit(0 if (ceiling > 0 and spend <= ceiling) else 1)" "$SPEND" "$CEILING" 2>/dev/null; then
MODE="BLOCK"
MESSAGE="Pro spend limit exceeded or blocked (\$${SPEND} spend, \$${CEILING} limit)."
fi
cat <<EOF
{"status":"${MODE}","monthMax":${CEILING},"message":"${MESSAGE}"}
EOF
exit 0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Comprehensive handover: daemon running on MiniPC, 4 AI commits, GitHub push setup instructions, cloud LLM consideration, next steps.
Summary by Sourcery
Document the current agent-seed and agentic-workflows architecture and decisions, introduce ADR-based architecture records and budget policies, and add a detailed session handover for 2026-05-31 covering the MiniPC daemon setup and next steps.
Enhancements:
Documentation:
Chores:
Summary by CodeRabbit
New Features
Documentation