mirror of
https://github.com/J3vb/OwnCord.git
synced 2026-09-03 03:50:00 +03:00
fix(workflows): drop hardcoded absolute repo path from bughunt prompts (#1373)
* fix: enhance bugfix workflow documentation with detailed clustering and staging instructions * fix(workflows): drop hardcoded absolute repo path from bughunt prompts The bughunt and bughunt-fix agent prompts told every finder, verifier, fix and prove agent that the repo lives at a specific absolute path from one contributor's machine. Anywhere else - a cloud session, CI, another checkout - that path does not exist, and the churn recon agent ran `git -C <that path> log ...` outright, so the most-churned-files inventory came back empty and every finder prompt lost its churn context. Point the prompts at the agent's working directory instead, which is the repo root on every platform. Both harnesses pass (bughunt.harness.mjs, bughunt-fix.harness.mjs). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ENMDTh8gDLiHCaRFdMYRiL --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -115,6 +115,37 @@ Workflow({
|
|||||||
Create and check out the branch first — the workflow commits to whatever branch
|
Create and check out the branch first — the workflow commits to whatever branch
|
||||||
is current and does not create one.
|
is current and does not create one.
|
||||||
|
|
||||||
|
### Composing the batch
|
||||||
|
|
||||||
|
The workflow clusters findings by the file the BUG is in, but its same-run
|
||||||
|
overlap guard fires on the files the FIX touches. Compose the batch so the two
|
||||||
|
can never disagree:
|
||||||
|
|
||||||
|
- **Close over the file relation.** Pull in every open finding that shares a
|
||||||
|
file with anything already selected, regardless of severity — a same-file
|
||||||
|
finding left behind is a future cross-cluster block.
|
||||||
|
- **Re-check coordinates against the working tree** (file exists, line within
|
||||||
|
length) before launching. render-run-stats checked them at hunt time only;
|
||||||
|
merges since then can stale them.
|
||||||
|
- **Scan fix-touchpoints, then read only the hits.** Token-scan each finding's
|
||||||
|
`suggestedFix`/`why` for path-like tokens owned by another cluster, then read
|
||||||
|
just the flagged records to separate evidence citations from actual fix
|
||||||
|
edits — the scan over-predicts (a measured run: 6 flagged, 1 real). True
|
||||||
|
collisions go into **sequential waves**: launch the later wave after the
|
||||||
|
earlier wave's commits land, and the same-run guard never fires. Batch 4
|
||||||
|
skipped this and self-blocked 20/27 findings; batch 6 ran it and blocked
|
||||||
|
zero.
|
||||||
|
|
||||||
|
### Security findings
|
||||||
|
|
||||||
|
Fixed security findings ship in normal PRs at this project's stage (alpha,
|
||||||
|
~zero external deployments): the fix and its disclosure land atomically.
|
||||||
|
Commit subjects and PR text describe the fix, never the exploit — no
|
||||||
|
severity labels, repro steps, or attack narratives in public text — and a
|
||||||
|
release should follow soon after merge. The GHSA advisory route is reserved
|
||||||
|
for coordinated disclosure once there is a real deployed user base. Never
|
||||||
|
decline a finding merely for routing.
|
||||||
|
|
||||||
## When a run trips the breaker
|
## When a run trips the breaker
|
||||||
|
|
||||||
The run stops early if more than `threshold` of attempted findings fail, once at
|
The run stops early if more than `threshold` of attempted findings fail, once at
|
||||||
@@ -156,6 +187,16 @@ assertion caught it).
|
|||||||
Check `result.gate`. A failed gate leaves the commits in place on the branch —
|
Check `result.gate`. A failed gate leaves the commits in place on the branch —
|
||||||
fix it yourself, do not re-run the workflow over it.
|
fix it yourself, do not re-run the workflow over it.
|
||||||
|
|
||||||
|
A fix that tightens a guard and fails ONLY e2e/CI while unit tests and local
|
||||||
|
gates stay green is usually a test-infrastructure defect, not a bad fix:
|
||||||
|
triage the mock/harness first. Check that mock echoes carry the same fields
|
||||||
|
the real server sends (real channel ids, not sentinels), and never assert
|
||||||
|
broadcast delivery on a socket the same operation force-closes. Mocks
|
||||||
|
calibrated against lenient code silently decay into lies — every
|
||||||
|
guard-tightening fix is also a fidelity audit of the mocks that exercise it.
|
||||||
|
The Playwright trace's console stream (grep the .trace file for the app's log
|
||||||
|
lines) locates the mechanism in minutes.
|
||||||
|
|
||||||
## 4. Verify the fixes independently — REQUIRED
|
## 4. Verify the fixes independently — REQUIRED
|
||||||
|
|
||||||
The workflow's prove agent *self-reports* that each test went RED with the fix
|
The workflow's prove agent *self-reports* that each test went RED with the fix
|
||||||
@@ -167,9 +208,16 @@ workflow made:
|
|||||||
node .superpowers/verify-fixes.mjs <sha> <sha> ...
|
node .superpowers/verify-fixes.mjs <sha> <sha> ...
|
||||||
```
|
```
|
||||||
|
|
||||||
It reverts each commit's source files to the parent, runs that stack's tests,
|
It reverse-applies each commit's own source diff onto the current tree, runs
|
||||||
and requires them to FAIL — then restores and requires them to PASS. This is the
|
that commit's tests, and requires them to FAIL — then restores to HEAD and
|
||||||
only check in the pipeline no agent can fabricate.
|
requires them to PASS. This is the only check in the pipeline no agent can
|
||||||
|
fabricate. The reverse-apply/restore-to-HEAD shape is load-bearing for
|
||||||
|
**stacked waves**: sequential waves pile commits onto shared files, and an
|
||||||
|
older per-commit-snapshot restore silently corrupts every later verification
|
||||||
|
(a mid-branch snapshot left in the tree once made a whole package
|
||||||
|
uncompilable for five subsequent runs). Rust commits with in-file
|
||||||
|
`#[cfg(test)]` tests fail the file-level source/test split entirely — prove
|
||||||
|
those by hand at hunk level, splitting at the `mod tests` boundary.
|
||||||
|
|
||||||
While it runs it checkouts and restores source files, so the working tree is
|
While it runs it checkouts and restores source files, so the working tree is
|
||||||
not a stable read surface: anything reading concurrently — review agents,
|
not a stable read surface: anything reading concurrently — review agents,
|
||||||
@@ -181,11 +229,19 @@ Any `FAIL ... VACUOUS TEST` means the fix was committed behind a test that
|
|||||||
proves nothing. Revert that commit and set its findings back to `open`; do not
|
proves nothing. Revert that commit and set its findings back to `open`; do not
|
||||||
talk yourself into keeping it because the code change looks right.
|
talk yourself into keeping it because the code change looks right.
|
||||||
|
|
||||||
One class is exempt from the insta-revert: a race/deadlock-class fix whose
|
Two classes are exempt from the insta-revert, both artifacts of the verifier
|
||||||
test only fails under its detector. verify-fixes escalates to `-race` before
|
choosing the wrong detector rather than of a vacuous test:
|
||||||
declaring vacuity; if an older copy of the script reports VACUOUS on such a
|
|
||||||
fix, re-prove red/green by hand under the class's detector
|
- **Race/deadlock-class fixes** whose test only fails under its detector.
|
||||||
(`go test -race ./<pkg>/`) before reverting anything.
|
verify-fixes escalates to `-race` before declaring vacuity; if an older copy
|
||||||
|
reports VACUOUS on such a fix, re-prove red/green by hand under the class's
|
||||||
|
detector (`go test -race ./<pkg>/`) before reverting anything.
|
||||||
|
- **Cross-stack commits** whose test files belong to a different stack than
|
||||||
|
their source files (e.g. a vitest file pinning a `src-tauri/` config). The
|
||||||
|
script now keys the runner off the TEST files; an older copy keyed it off
|
||||||
|
the sources and ran the wrong stack's suite, which never executes the proof.
|
||||||
|
On any VACUOUS verdict for a cross-stack commit, re-prove by hand with the
|
||||||
|
test file's own runner before reverting.
|
||||||
|
|
||||||
For every commit `verify-fixes.mjs` reports `PASS`, upgrade that commit's
|
For every commit `verify-fixes.mjs` reports `PASS`, upgrade that commit's
|
||||||
findings' `fix.revertProof` from `"self-reported"` to `"pass"` — this
|
findings' `fix.revertProof` from `"self-reported"` to `"pass"` — this
|
||||||
@@ -195,9 +251,30 @@ was already reverted and its findings set back to `open` above.
|
|||||||
|
|
||||||
Re-render. Before you review the branch and open the PR, inspect the working
|
Re-render. Before you review the branch and open the PR, inspect the working
|
||||||
tree: a blocked or declined cluster can leave its edits and any new failing
|
tree: a blocked or declined cluster can leave its edits and any new failing
|
||||||
test it wrote sitting uncommitted. Discard what you don't want — a reflexive
|
test it wrote sitting uncommitted. The gate's "N uncommitted modifications at
|
||||||
`git add -A` would commit tests that describe unfixed defects into a public
|
gate time" note is the trigger list. **Classify before discarding** — an
|
||||||
repo. Audit what got COMMITTED, too: parallel fix agents share the tree, so a
|
uncommitted modification to a TRACKED test file may be a required companion to
|
||||||
|
a committed fix, not debris. Two known mechanisms: the old test locked the old
|
||||||
|
buggy behavior, and the fix widened an interface that a fake/mock in a test
|
||||||
|
file the cluster never named must now implement (without it the committed
|
||||||
|
package does not even compile). The decisive test: `git stash push -- <file>`,
|
||||||
|
then compile/test the committed state alone — if it fails, the modification is
|
||||||
|
a companion; fold it into the causing commit (fixup + autosquash keeps the
|
||||||
|
history coherent for verify-fixes) or commit it with attribution. Only then
|
||||||
|
discard true debris — a reflexive `git add -A` would commit tests that
|
||||||
|
describe unfixed defects into a public repo.
|
||||||
|
|
||||||
|
**Capture before destroying, as separate verified steps.** Preserve blocked or
|
||||||
|
declined edits by copying the files aside (or `git stash push -u` after
|
||||||
|
confirming it actually saved something) BEFORE any rm/checkout, and never
|
||||||
|
chain the capture and the destructive step into one command — a capture
|
||||||
|
failure then becomes data loss (`git diff /dev/null <file>` fails outright on
|
||||||
|
Windows git and has deleted debris before it was saved). Never blind
|
||||||
|
`git stash pop`: a paired push on a clean tree saves nothing, and the pop then
|
||||||
|
grabs whatever foreign stash sits on top (a preserved debris stash, in the
|
||||||
|
measured case — ~40 conflicted paths). Pop only a ref you verified your own
|
||||||
|
push created; for temporary file comparisons, skip stash entirely and read old
|
||||||
|
versions from the object store (`git show <ref>:<path>`). Audit what got COMMITTED, too: parallel fix agents share the tree, so a
|
||||||
prove agent can commit a sibling cluster's content that happened to sit in a
|
prove agent can commit a sibling cluster's content that happened to sit in a
|
||||||
shared test file or in regenerated output. Grep the committed tests for
|
shared test file or in regenerated output. Grep the committed tests for
|
||||||
finding ids outside the run's fixed set, and re-run the generated-code
|
finding ids outside the run's fixed set, and re-run the generated-code
|
||||||
|
|||||||
@@ -117,10 +117,15 @@ phase('Fix')
|
|||||||
|
|
||||||
function fixPrompt(cluster) {
|
function fixPrompt(cluster) {
|
||||||
return (
|
return (
|
||||||
`You are fixing confirmed bugs in ONE file of the OwnCord repo (D:/Local-Lab/Repos/OwnCord).\n` +
|
`You are fixing confirmed bugs in ONE file of the OwnCord repo (checked out at your current working ` +
|
||||||
|
`directory - do not assume any absolute path; use repo-relative paths).\n` +
|
||||||
`Your file: ${cluster.file}\n\n` +
|
`Your file: ${cluster.file}\n\n` +
|
||||||
`You own this file for this run. No other agent will touch it, so fix ALL of the findings below ` +
|
`You own this CLUSTER for this run: no other agent has this file as its cluster, so fix ALL of the ` +
|
||||||
`together rather than one at a time.\n\n` +
|
`findings below together rather than one at a time. But cluster fixes may share a file (rule 4 cuts ` +
|
||||||
|
`both ways), so another cluster's root-cause edit can still land in this file while you work. ` +
|
||||||
|
`Re-read the exact region you are about to edit immediately before each edit rather than trusting a ` +
|
||||||
|
`full-file read from the start of your turn, and treat any editor warning that the file changed ` +
|
||||||
|
`since you last read it as a signal to re-read and re-target, never to force the edit through.\n\n` +
|
||||||
`RULES\n` +
|
`RULES\n` +
|
||||||
` 1. Test first. For each finding, write a test that FAILS against the current code before you ` +
|
` 1. Test first. For each finding, write a test that FAILS against the current code before you ` +
|
||||||
`change anything, and run it to watch it fail. A test that passes before the fix does not pin the ` +
|
`change anything, and run it to watch it fail. A test that passes before the fix does not pin the ` +
|
||||||
@@ -287,7 +292,7 @@ const PROVE_RESULT = {
|
|||||||
function provePrompt(cluster, fixedIds, testPaths, sourcePaths) {
|
function provePrompt(cluster, fixedIds, testPaths, sourcePaths) {
|
||||||
return (
|
return (
|
||||||
`You are proving and committing ONE cluster of fixes in the OwnCord repo ` +
|
`You are proving and committing ONE cluster of fixes in the OwnCord repo ` +
|
||||||
`(D:/Local-Lab/Repos/OwnCord), on branch ${BRANCH}.\n\n` +
|
`(checked out at your current working directory - do not assume any absolute path), on branch ${BRANCH}.\n\n` +
|
||||||
`Source file(s): ${sourcePaths.join(', ')}\n` +
|
`Source file(s): ${sourcePaths.join(', ')}\n` +
|
||||||
`Findings fixed here: ${fixedIds.join(', ')}\n` +
|
`Findings fixed here: ${fixedIds.join(', ')}\n` +
|
||||||
`Test files written: ${testPaths.join(', ') || '(none reported)'}\n\n` +
|
`Test files written: ${testPaths.join(', ') || '(none reported)'}\n\n` +
|
||||||
@@ -320,8 +325,13 @@ function provePrompt(cluster, fixedIds, testPaths, sourcePaths) {
|
|||||||
`regenerated output can carry their hunks. A test function or comment citing a finding id not ` +
|
`regenerated output can carry their hunks. A test function or comment citing a finding id not ` +
|
||||||
`listed above, or a hunk in a generated/shared file unrelated to your findings, must NOT be ` +
|
`listed above, or a hunk in a generated/shared file unrelated to your findings, must NOT be ` +
|
||||||
`committed - set committed=false, name the foreign content in note, and STOP.\n` +
|
`committed - set committed=false, name the foreign content in note, and STOP.\n` +
|
||||||
` 8. Stage ALL source file(s) listed above (git add ${sourcePaths.join(' ')}) AND the test files, ` +
|
` 8. Stage ALL source file(s) listed above (git add ${sourcePaths.join(' ')}) AND the test files. ` +
|
||||||
`then commit with subject:\n` +
|
`Then check git status --porcelain for OTHER modified tracked test files in the same package(s)/` +
|
||||||
|
`directory(ies) as your source files: a fix in this cluster may have rewritten a pre-existing test ` +
|
||||||
|
`that locked the old behavior, or widened an interface that a fake/mock in a sibling test file must ` +
|
||||||
|
`now implement - leaving such a companion uncommitted makes the committed branch fail or not compile ` +
|
||||||
|
`on its own. If the modification's content belongs to THIS cluster's fix (per the step-7 check), ` +
|
||||||
|
`stage it too; if it cites another cluster's findings, leave it. Commit with subject:\n` +
|
||||||
` fix(<area>): ${fixedIds.length} defect(s) (${fixedIds.join(', ')})\n` +
|
` fix(<area>): ${fixedIds.length} defect(s) (${fixedIds.join(', ')})\n` +
|
||||||
` Use a conventional-commit area matching the file (voice, ws, client, identity...). Do not add a ` +
|
` Use a conventional-commit area matching the file (voice, ws, client, identity...). Do not add a ` +
|
||||||
`Co-Authored-By trailer.\n` +
|
`Co-Authored-By trailer.\n` +
|
||||||
|
|||||||
@@ -86,7 +86,8 @@ const VERDICTS = {
|
|||||||
|
|
||||||
// ---------- rules ----------
|
// ---------- rules ----------
|
||||||
const RULES = `
|
const RULES = `
|
||||||
Repo: OwnCord, at D:/Local-Lab/Repos/OwnCord. Go 1.26 server in Server/, Tauri v2 client in Client/tauri-client/
|
Repo: OwnCord, checked out at your current working directory (the repo root - do not assume any absolute
|
||||||
|
path; run every command from there and use repo-relative paths). Go 1.26 server in Server/, Tauri v2 client in Client/tauri-client/
|
||||||
(Rust in src-tauri/src/, TypeScript in src/lib/ and src/stores/).
|
(Rust in src-tauri/src/, TypeScript in src/lib/ and src/stores/).
|
||||||
|
|
||||||
You are hunting REAL BUGS: wrong behavior, not style. In scope:
|
You are hunting REAL BUGS: wrong behavior, not style. In scope:
|
||||||
@@ -450,7 +451,7 @@ const recon = await parallel([
|
|||||||
() =>
|
() =>
|
||||||
agent(
|
agent(
|
||||||
`${RULES}\n\nRECON TASK (mechanical, do not hunt bugs yourself):\n` +
|
`${RULES}\n\nRECON TASK (mechanical, do not hunt bugs yourself):\n` +
|
||||||
`Run: git -C D:/Local-Lab/Repos/OwnCord log --since="8 weeks ago" --name-only --pretty=format: -- Server Client\n` +
|
`Run, from the repo root: git log --since="8 weeks ago" --name-only --pretty=format: -- Server Client\n` +
|
||||||
`Count how often each non-test source file changed. Return the 25 most-churned files with their counts, ` +
|
`Count how often each non-test source file changed. Return the 25 most-churned files with their counts, ` +
|
||||||
`plus any file that changed in more than 6 distinct commits. High churn = where bugs concentrate.\n` +
|
`plus any file that changed in more than 6 distinct commits. High churn = where bugs concentrate.\n` +
|
||||||
`Return plain text: one "path count" per line, most-churned first. No commentary.`,
|
`Return plain text: one "path count" per line, most-churned first. No commentary.`,
|
||||||
|
|||||||
Reference in New Issue
Block a user