Improve agentic workflow — CI rules, branch strategy, deprecation rules, and new agents #28

Merged

TapTap merged 4 commits from improve-agentic-workflow into main 2026-07-20 14:44:36 +02:00

Conversation 2 Commits 4 Files Changed 17

+1361 -6

TapTap commented 2026-07-20 14:35:07 +02:00

Owner

Copy Link

Copy Source

Summary

Improves the agentic workflow configuration with consistent rules across all agents.

Changes

1. AGENTS.md — Added CI Workflow (wait for results with timeout) and Branch Strategy (never push to main, always PR) sections
2. CI vs Host deps clarified — CI must use Docker image; host can use nix-shell
3. All 12 existing agents — Added 3 standard sections: CI & Task Execution, Branch Strategy, Dependency Installation
4. integrator.md — Fixed CI example that incorrectly used sudo apt-get install (now uses Docker container)
5. 4 new agents created:
   • issue-creator (mode: subagent) — orchestrator that dispatches sub-agents and creates GitHub issues
   • feature-scout — scans for feature opportunities (TODOs, hardcoded values, rsync gaps)
   • security-screener — scans for vulnerabilities (buffer overflows, TLS, path traversal)
   • code-quality-guardian — scans for quality issues (god functions, duplication, complexity)

## Summary Improves the agentic workflow configuration with consistent rules across all agents. ### Changes 1. **AGENTS.md** — Added CI Workflow (wait for results with timeout) and Branch Strategy (never push to main, always PR) sections 2. **CI vs Host deps clarified** — CI must use Docker image; host can use nix-shell 3. **All 12 existing agents** — Added 3 standard sections: CI & Task Execution, Branch Strategy, Dependency Installation 4. **integrator.md** — Fixed CI example that incorrectly used sudo apt-get install (now uses Docker container) 5. **4 new agents created:** - `issue-creator` (mode: subagent) — orchestrator that dispatches sub-agents and creates GitHub issues - `feature-scout` — scans for feature opportunities (TODOs, hardcoded values, rsync gaps) - `security-screener` — scans for vulnerabilities (buffer overflows, TLS, path traversal) - `code-quality-guardian` — scans for quality issues (god functions, duplication, complexity)

TapTap added 3 commits 2026-07-20 14:35:08 +02:00

doc: improve agentic workflow — CI wait, branch strategy, dependency rules for all agents d3b0cb9357

doc: clarify CI vs host deps (Docker image for CI, nix-shell for host) b10da86f87

fix: set issue-creator mode to subagent, move into .opencode/agents/ 6501b15574

TapTap commented 2026-07-20 14:40:54 +02:00

Author

Owner

Copy Link

Copy Source

Review

CI is green, new agents are well-structured, integrator fix correct. One stale section needs attention.

---

🔴 cmake-expert.md — stale "When Adding Sanitizer Support" section contradicts correct pattern

The document has two contradictory sanitizer instructions:

Lines 148–163 (correct, "Sanitizer Configurations"):

cmake -B build -S . -DSANITIZER=address   # correct
cmake -B build -S . -DSANITIZER=thread    # correct

Lines 165–191 (wrong, "When Adding Sanitizer Support to CMakeLists.txt"):

option(ENABLE_ASAN "Enable AddressSanitizer" OFF)
option(ENABLE_TSAN "Enable ThreadSanitizer" OFF)
option(ENABLE_UBSAN "Enable UndefinedBehaviorSanitizer" OFF)
# ... then build with: cmake -B build -S . -DENABLE_ASAN=ON

This is the old stale pattern that was already flagged in the PR #26 review. The real CMakeLists.txt uses a single SANITIZER cache variable with elseif branches — not three separate option() booleans. An agent that reads the "When Adding Sanitizer Support" section will produce wrong CMake code.

Fix: Replace lines 165–191 with instructions showing how to add a new value to the existing SANITIZER cache variable (e.g., add an elseif(SANITIZER STREQUAL "undefined") block). Or remove the section entirely — the correct pattern is already shown above and in AGENTS.md.

---

🟡 Dockerfile — lcov/valgrind addition conflicts with PR #27's AGENTS.md rule

PR #27's AGENTS.md says "never add apt-get install to CI workflows — use the custom Docker image". PR #25 adds lcov/valgrind to the Dockerfile. This PR also adds them. Both PRs are correct individually, but whoever merges second should confirm the Dockerfile is clean.

No action needed from this PR — just flagging the overlap. The lcov/valgrind addition in the Dockerfile is the right approach per AGENTS.md.

---

New agents — well-structured

• issue-creator: Good orchestrator pattern — dispatches sub-agents, collects findings, creates issues with gh issue create. Uses mode: subagent correctly.
• feature-scout: Clear scan categories (TODOs, hardcoded values, protocol gaps), structured output format with severity/location/suggestion.
• security-screener: Good attack-surface focus (buffer overflows, TLS, path traversal, crypto), separate from security-auditor (which does targeted TLS verification).
• code-quality-guardian: Clear quality categories (god functions, duplication, complexity), structured output.

All three scanner agents have the correct "No findings" return format, severity guidelines, and the standard three footer sections (CI, Branch Strategy, Dependency Installation).

---

integrator.md — fix correct

The CI example now uses container: gitea.tap-tap.win/taptap/fastsync-ci:v7 instead of sudo apt-get install. Correct per AGENTS.md dependency rules.

---

Verdict: Approve with 1 fix needed. The stale ENABLE_ASAN/ENABLE_TSAN/ENABLE_UBSAN section in cmake-expert.md should be updated or removed — it contradicts the correct pattern shown earlier in the same file.

## Review CI is green, new agents are well-structured, integrator fix correct. One stale section needs attention. --- ### 🔴 cmake-expert.md — stale "When Adding Sanitizer Support" section contradicts correct pattern The document has two contradictory sanitizer instructions: **Lines 148–163** (correct, "Sanitizer Configurations"): ```bash cmake -B build -S . -DSANITIZER=address # correct cmake -B build -S . -DSANITIZER=thread # correct ``` **Lines 165–191** (wrong, "When Adding Sanitizer Support to CMakeLists.txt"): ```cmake option(ENABLE_ASAN "Enable AddressSanitizer" OFF) option(ENABLE_TSAN "Enable ThreadSanitizer" OFF) option(ENABLE_UBSAN "Enable UndefinedBehaviorSanitizer" OFF) # ... then build with: cmake -B build -S . -DENABLE_ASAN=ON ``` This is the old stale pattern that was already flagged in the PR #26 review. The real CMakeLists.txt uses a single `SANITIZER` cache variable with `elseif` branches — not three separate `option()` booleans. An agent that reads the "When Adding Sanitizer Support" section will produce wrong CMake code. **Fix:** Replace lines 165–191 with instructions showing how to add a new value to the existing `SANITIZER` cache variable (e.g., add an `elseif(SANITIZER STREQUAL "undefined")` block). Or remove the section entirely — the correct pattern is already shown above and in AGENTS.md. --- ### 🟡 Dockerfile — lcov/valgrind addition conflicts with PR #27's AGENTS.md rule PR #27's AGENTS.md says "never add `apt-get install` to CI workflows — use the custom Docker image". PR #25 adds lcov/valgrind to the Dockerfile. This PR also adds them. Both PRs are correct individually, but whoever merges second should confirm the Dockerfile is clean. No action needed from this PR — just flagging the overlap. The lcov/valgrind addition in the Dockerfile is the right approach per AGENTS.md. --- ### New agents — well-structured - **issue-creator**: Good orchestrator pattern — dispatches sub-agents, collects findings, creates issues with `gh issue create`. Uses `mode: subagent` correctly. - **feature-scout**: Clear scan categories (TODOs, hardcoded values, protocol gaps), structured output format with severity/location/suggestion. - **security-screener**: Good attack-surface focus (buffer overflows, TLS, path traversal, crypto), separate from security-auditor (which does targeted TLS verification). - **code-quality-guardian**: Clear quality categories (god functions, duplication, complexity), structured output. All three scanner agents have the correct "No findings" return format, severity guidelines, and the standard three footer sections (CI, Branch Strategy, Dependency Installation). --- ### integrator.md — fix correct The CI example now uses `container: gitea.tap-tap.win/taptap/fastsync-ci:v7` instead of `sudo apt-get install`. Correct per AGENTS.md dependency rules. --- **Verdict: Approve with 1 fix needed.** The stale `ENABLE_ASAN/ENABLE_TSAN/ENABLE_UBSAN` section in cmake-expert.md should be updated or removed — it contradicts the correct pattern shown earlier in the same file.

TapTap added 1 commit 2026-07-20 14:42:04 +02:00

Merge branch 'main' into improve-agentic-workflow 91071a4ed5

TapTap commented 2026-07-20 14:44:04 +02:00

Author

Owner

Copy Link

Copy Source

Re-review: Approve ✅ (2 minor items)

The critical cmake-expert.md fix is clean — the stale ENABLE_ASAN/ENABLE_TSAN/ENABLE_UBSAN section is gone, replaced with correct SANITIZER cache variable documentation. All other changes since last review are the PR #26 merge (ci.yaml, client_cli.c, reviewer.md) which we already approved.

Verified fixes

Finding	Fix	Status
🔴 cmake-expert: stale ENABLE_ASAN pattern	Replaced with "Sanitizer Integration" section showing correct SANITIZER cache variable + elseif guidance	✅
🟡 cmake-expert: ordering vs real CMakeLists.txt	FetchContent block now correctly placed before Sanitizer option, matching real file	✅

Remaining minor items (non-blocking)

1. integrator.md line 110: inaccurate CI job description

2. **sanitizer** — ASan + UBSan build and test (separate job)

The CI matrix only has [address]. UBSan is not in the matrix. Should read "ASan build and test" or "Sanitizer build and test (address)".

2. cmake-expert.md: UBSan workaround section

The manual -DCMAKE_C_FLAGS="-fsanitize=undefined" workaround is documented with the accurate caveat "no -DSANITIZER=undefined option in CMakeLists.txt yet". This will become stale when PR #25 lands (which adds UBSan to the SANITIZER option + CI matrix), but is correct for now.

---

Verdict: Approve. All previous critical items fixed. The two minor items above are non-blocking — the integrator.md inaccuracy is cosmetic (agents will still produce correct CI YAML from the example), and the UBSan workaround is correctly caveated.

## Re-review: Approve ✅ (2 minor items) The critical cmake-expert.md fix is clean — the stale `ENABLE_ASAN/ENABLE_TSAN/ENABLE_UBSAN` section is gone, replaced with correct `SANITIZER` cache variable documentation. All other changes since last review are the PR #26 merge (ci.yaml, client_cli.c, reviewer.md) which we already approved. ### Verified fixes | Finding | Fix | Status | |---|---|---| | 🔴 cmake-expert: stale `ENABLE_ASAN` pattern | Replaced with "Sanitizer Integration" section showing correct `SANITIZER` cache variable + `elseif` guidance | ✅ | | 🟡 cmake-expert: ordering vs real CMakeLists.txt | FetchContent block now correctly placed before Sanitizer option, matching real file | ✅ | ### Remaining minor items (non-blocking) **1. integrator.md line 110: inaccurate CI job description** ``` 2. **sanitizer** — ASan + UBSan build and test (separate job) ``` The CI matrix only has `[address]`. UBSan is not in the matrix. Should read "ASan build and test" or "Sanitizer build and test (address)". **2. cmake-expert.md: UBSan workaround section** The manual `-DCMAKE_C_FLAGS="-fsanitize=undefined"` workaround is documented with the accurate caveat "no `-DSANITIZER=undefined` option in CMakeLists.txt yet". This will become stale when PR #25 lands (which adds UBSan to the SANITIZER option + CI matrix), but is correct for now. --- **Verdict: Approve.** All previous critical items fixed. The two minor items above are non-blocking — the integrator.md inaccuracy is cosmetic (agents will still produce correct CI YAML from the example), and the UBSan workaround is correctly caveated.

TapTap merged commit 51a984871c into main 2026-07-20 14:44:36 +02:00

TapTap deleted branch improve-agentic-workflow 2026-07-20 14:44:39 +02:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

bug

Defect or bug

enhancement

Feature or improvement

memory

Memory safety issue

needs-triage

Needs human review

performance

Performance issue

quality

Code quality improvement

security

Security vulnerability

threading

Threading or concurrency

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

TapTap

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: TapTap/FastSync#28