[pull] main from affaan-m:main - #249
Merged
Merged
Conversation
…ation Adds a community skill that turns the agent into the translation layer for JSON locale files, delegating the deterministic work (key diffing, lockfile staleness tracking, placeholder/glossary validation, safe JSON writes) to the locakit CLI (npm, MIT, zero dependencies). What makes it different from API-based translation flows: - No translation API keys or SaaS accounts; the agent itself translates - The skill instructs the agent to read each key's real usage context in the codebase before translating (nav label vs page title vs toast) - locakit apply rejects keys that don't exist in the source locale, containing hallucinated keys - locakit check gives CI-enforceable validation incl. a Turkish language pack (suffix-after-placeholder, dotless-uppercase-i) Validated end-to-end on a production Next.js 16 portfolio: 175 keys translated tr->en in one pass, locakit check reporting 0 errors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Add Prompt Defense Baseline to agent-evaluator (was the only agent missing it) - Update AGENTS.md to document all 67 agents (36 were previously unlisted) - Add routing guidance for 11 additional agents in orchestration section - Count in header kept at 67 (matches actual agents/ folder count) Audit findings (documented but not changed): - 3 agents (doc-updater, opensource-forker, opensource-packager) use model:haiku with Write/Edit tools; intentional for lightweight pipeline tasks - 6 agents have non-standard color: field (loop-operator, harness-optimizer, gan-*, performance-optimizer); may be harness-specific UI metadata - 7 agents are <=60 lines; thin but sufficient for narrow scope Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…tion Santa-loop's Reviewer B only tried codex and gemini before falling back to a same-family Claude reviewer, losing model diversity when neither was installed. Detect agy (Antigravity CLI, ~/.local/bin/agy) as a third option, using gemini-3.6-flash-high — despite the "flash" name, it currently outranks gemini-3.1-pro-high on every published coding/agentic benchmark (SWE-Bench Pro, Terminal-Bench, MLE-Bench), with Pro only ahead on PhD-level reasoning benchmarks that don't apply to code review. Also documents never pointing agy at a Claude model, which would collapse Reviewer A/B model diversity entirely.
context-budget inventories components that load once at startup. On a reconnect the app re-injects much of that inventory into the session transcript as attachment lines, and that lands in the same context window. Measured on a session that had reconnected once: 33,884 of 49,830 bytes was scaffolding — skill_listing 14.6KB, mcp_instructions_delta 8.1KB, deferred_tools_delta 7.1KB, agent_listing_delta 2.6KB — against 15.9KB of actual conversation. Adds a measure-it-yourself snippet, a >40% flag, and the consequence for the rest of the skill: a trimmed skill or MCP server saves tokens once per reconnect, not once per session. Also notes that transcript size is a poor proxy for context fullness unless attachment lines are excluded.
Both reviewers were right and the original guidance was self-contradictory: it said attachments are charged to the context window, then told the report to exclude them when stating remaining room. Greptile's P1 is the sharper half — after a compaction, pre-compaction turns remain in the JSONL while the model's active context holds only the summary, so counting persisted bytes misreports headroom in the other direction too. The section now says plainly that a transcript is a durable log, not a view of the context window, and that remaining room must not be reported from its size. The number is framed as a cost signal instead: scaffolding is rebuilt on every reconnect, so trimming a component saves tokens once per reconnect rather than once per session. Snippet fixes: emits an explicit >40% warning with one decimal place rather than leaving the threshold in prose, guards a missing transcript, exits cleanly on an empty one, counts bytes rather than characters, and treats a malformed line as conversation instead of crashing. Verified against all four cases.
Splice artifact from the previous commit — the replacement text ended with the same anchor string it was spliced against, so the heading was emitted twice. Caught in review.
…entry points
`PROTECTED_FILES` matches exact basenames, so it only ever guarded a tool's
canonical entry point. Real repos split flat config across files — a shared
`eslint.config.base.mjs` holding the ignore list and rule severities, imported
by per-workspace `eslint.config.mjs` files, is the common monorepo shape.
That meant the hook protected the leaves and left the trunk wide open:
eslint.config.base.mjs <- ignore list + rule severities UNPROTECTED
frontend/eslint.config.mjs <- imports the base protected
backend/eslint.config.mjs <- imports the base protected
An agent blocked from touching the two leaves could silently rewrite every rule
severity in the file they both import. Hit in practice: two edits to
`eslint.config.js` were correctly blocked, then an edit to
`eslint.config.base.mjs` in the same repo went through unchallenged.
Adds `PROTECTED_PATTERNS` alongside the existing Set, covering
`<tool>.config.<qualifier>.<ext>` and `.<tool>rc.<qualifier>.<ext>` for the
linters and formatters already listed. Case-insensitive, for the same reason
the Set lookup is (#2543).
Deliberately NOT matched: `vite.config.ts`, `vitest.config.ts`,
`jest.config.js`, `playwright.config.ts`, `tsconfig.json`. This hook exists to
stop a LINTER config being weakened in place of fixing the code; editing a
bundler or test-runner config is ordinary work, and sweeping those in would
make the hook obstructive. A second test pins that boundary so a future
widening of the patterns cannot quietly cross it.
The exact-name Set is untouched, so nothing previously protected becomes
unprotected, and first-time creation stays allowed (the bootstrap path).
Tests: 11 pass. The new regression test was verified failing against the
unpatched hook first; the boundary test passes either way by design and is
there as the control.
…ings The hook blocks edits to linter/formatter configs so an agent fixes the code instead of weakening the checks. It did not cover the cheapest way to weaken them: adding one path to an ignore file. Measured against the hook, with the file already on disk: .eslintignore -> allow .prettierignore -> allow .stylelintignore -> allow .markdownlintignore -> allow Nor did it cover the current config names — only the legacy `.stylelintrc*` and `.markdownlint.json` spellings were listed, so a project using the documented `stylelint.config.js`, `.markdownlint.jsonc` or markdownlint-cli2 had no protection at all. First-time creation stays allowed by the existing existence check, so scaffolding a fresh ignore file is unaffected.
Stylelint resolves stylelint.config.{ts,mts,cts} through cosmiconfig and
markdownlint reads .markdownlint.{cjs,mjs}; both were still editable while the
hook was active.
…er status (#2859) `start-observer.sh status` globbed `*.yaml`, but the observer prompt tells the analyzer to write `${INSTINCTS_DIR}/<id>.md` and the loader accepts `.yaml`, `.yml`, and `.md` (ALLOWED_INSTINCT_EXTENSIONS in scripts/instinct-cli.py). The one command an operator runs to confirm that learning works therefore reported `Instincts: 0` on a healthy install — which is indistinguishable from a silently dead observer, exactly the failure the status check exists to surface. Match the loader instead of one of its three extensions, and match how it enumerates them: `Path.iterdir()` is top level only and `is_file()` skips directories, so `-maxdepth 1 -type f`; `suffix.lower()` makes the comparison case-insensitive, so `-iname`. `tr` drops the column padding BSD `wc` emits, which is why the reported output read `Instincts: 0`. Verified end to end against the shipped script with 3 `.md`, one `.yaml`, one `.yml`, one `.YAML`, a `notes.txt`, and a nested `.md`: 1 before, 6 after — the same six files the loader picks up.
…agent `scripts/ci/validate-agents.js` reads only `agents/`, so the translated copies under `docs/<locale>/agents/` were never validated against the agent they describe — and drifted. Two kinds of drift, both machine-checkable and both wrong in the same direction (the translations were made from an older revision and never re-synced): - **Model tier, 39 files.** Every one names a costlier tier than ships: haiku -> sonnet, sonnet -> opus. `security-reviewer` reads `opus` in ja-JP and zh-TW; it ships `sonnet`. - **Tool set, 15 files.** `security-reviewer` and `database-reviewer` list `Write` and `Edit` in **all seven locales** — both agents ship read-only (`Read, Grep, Glob, Bash`). `seo-specialist` in zh-CN adds a `Bash` the canonical agent does not have. Documenting a reviewer as able to write is the kind of inaccuracy someone auditing what these agents can touch would act on. Sync `model` and the `tools` set to canonical, preserving each locale's existing list style so the diff is only the values that were wrong. The `["a", "b"]` vs `a, b` formatting difference is left alone: it is consistent per locale and carries no meaning. Add `tests/ci/locale-agent-frontmatter.test.js` so this cannot drift back. `tools` is compared as a set, not a string, so the style difference stays legal; prose is not compared at all. The last case pins the specific failure: a canonical read-only agent may never be documented with Write or Edit. `.kiro/agents/` is deliberately excluded — it uses a different schema (`allowedTools: [read, shell]`, no `model`), not a translation of this one.
Review feedback on #2878: the static assertions ran at the top level, so a failure exited the process before the `Passed:`/`Failed:` lines. tests/run-all.js totals those tokens, so the per-case counts were lost — it still went red via the non-zero exit, but the granular numbers were not in the totals. Route every case through the same `runTest()` wrapper the file already used for the integration cases, and build the case list inside a try so a missing or renamed `instinct-cli.py` / `start-observer.sh` is a reported failure rather than a crash. Reverting the fix now prints `Passed: 4, Failed: 7` and exits 1, naming all seven broken expectations instead of stopping at the first.
Review feedback on #2879: a locale file with no counterpart in `agents/` was skipped, so every other case here silently passed over it. Retiring an agent would leave its seven translations behind with nothing to compare against and nothing to report it. Collect those files while building the list and assert the collection is empty. Zero today across 199 locale docs, so this changes no current result; planting one orphan takes the file to `Passed: 6, Failed: 1` and exit 1.
Review feedback on #2879, citing rules/common/coding-style.md ("Immutability (CRITICAL): ALWAYS create new objects, NEVER mutate existing ones"). Build one list of locale entries, then derive the matched and orphaned sets from it with filter/map, and build the canonical Map the same way. Behaviour is unchanged and both mutations still fail: reintroducing the drift in one locale file gives `Passed: 4, Failed: 3`, an orphan gives `Passed: 6, Failed: 1`, both exit 1.
Review feedback on #2879. stdout is async when it is a pipe, which is exactly how tests/run-all.js runs these files, and process.exit() does not wait for pending writes — so exiting that way can drop the Passed:/Failed: lines the aggregator totals, defeating the point of printing them. The sibling tests/ci/ito-*-skill.test.js files already use process.exitCode. Still exits 1 on drift and 0 when clean.
…shes Review feedback on #2878. stdout is async when it is a pipe, which is exactly how tests/run-all.js runs these files, and process.exit() does not wait for pending writes — so exiting that way can drop the Passed:/Failed: lines the aggregator totals, defeating the previous commit. The sibling tests/ci/ito-*-skill.test.js files already use process.exitCode. Still exits 1 on a broken counter (Passed: 4, Failed: 7) and 0 when clean.
…doc line The scanner locked onto a `git commit` literal that sat inside a quoted string of an OUTER command (a python heredoc's string, a VAR="..." assignment, a printf JSON payload), then hasNoVerifyFlag() re-tokenized from that inner offset with a fresh quote state. When quote parity flipped, the "segment" ran past the enclosing string and picked up an unrelated later `bash -n` / `sed -n` / `grep -n` as `commit -n`. Two legitimate commands were blocked this way in one working session. The first attempt at this PR (d5c428c) fixed the false positive by SKIPPING `git` tokens judged to be in "data" context. That was the wrong lever: data becomes executable the moment it is piped to a shell (`echo '...' | bash`, `bash <<< '...'`, `cat <<EOF | bash`), and no regex list of shell invocations is complete (`bash --noprofile -c` slipped). Three independent reviewers (codex, Greptile, CodeRabbit) demonstrated executing bypasses; that commit is squashed away here and none of its behaviour survives. This version keeps every `git` candidate visible and keeps main's blocking surface intact. A single O(n) preprocessing pass (buildScanBoundaries) records, for each enclosed character, the closing quote or the current heredoc-body line end. Subcommand discovery and flag tokenization are capped at that boundary, so tokens after the enclosing data can never leak into a candidate's argument list. Also: comment detection is a single mask pass instead of a repeated prefix scan; heredoc parsing accepts quoted hyphenated delimiters and CRLF terminators and preserves the `<<` vs `<<-` tab distinction; quote-assembled words (`g''it`) are recognized. Verified (all payloads run through the hook as JSON on stdin): - red-list, 24 executing forms incl. every reviewer-reported bypass -> exit 2 on main AND here (CRLF heredoc trailer: 0 on main, 2 here); - green-list, 16 leak-class false positives -> exit 2 on main, 0 here; - tests/hooks/block-no-verify.test.js 84/84 (main's 25 retained verbatim, d5c428c's 8 removed, 59 table-driven red/green cases added); tests/hooks/cursor-block-no-verify.test.js 14/14; - 204,807-byte command with 8,905 non-command `git` literals: 8.5 ms. Co-Authored-By: Codex (GPT) <noreply@openai.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EyuZY1LZNuShHiewExekLT
Merge buildCommentMask into buildScanBoundaries so one pass owns comment mask and scan bounds. Extract shell-scan primitives to scripts/hooks/lib/shell-scan.js; keep policy in the hook. Use a sticky heredoc regex instead of copying the remaining input on every <<. Replace the short-circuiting chain test with clean-leading sequences that exercise checkCommand advance. Add quoted-executable non-leakage coverage for the scanEnd=length path. Treat $'…' ANSI-C quoting as a quoted executable word. Join unquoted backslash-newline for the flag scan and heredoc-line bound. Co-Authored-By: grok (fleet lane) <noreply@fleet.local> Co-authored-by: groy75 <groy75@users.noreply.github.com>
project-stack-mappings.json had no `fastapi` entry, so /project-init detected FastAPI projects as generic `python` and never resolved the fastapi-patterns skill that already ships in the repo. api-design is included for the same reason: FastAPI work is REST surface work. Indicators match `fastapi` in requirements.txt or pyproject.toml, following the existing django entry pattern. Data-only change; no code reads this file, it is the lookup table /project-init consults.
Signed-off-by: Samarjeet Singh Tomar <samar_tomar@hotmail.com>
DESTRUCTIVE_SQL_DD shared one trailing \b across every alternation arm. `dd\s+if=` ends in `=`, and a \b after a non-word character only holds when the NEXT character is a word character, so the arm matched `dd if=x` and missed every path starting with `/`, `.` or a quote: dd if=/dev/zero of=/dev/sda allowed dd if=./disk.img of=/dev/sdb allowed dd if="/dev/zero" of=/dev/sda allowed The word-boundary suffix now applies only to the arms that end in a word character. The split is what keeps the widening bounded: dropping the trailing \b outright would let `truncate` match `truncated`, and dropping the leading \b would let `dd if=` match inside `add if=`. Both are covered. This is the half of #2642 that survived the structural findGitSubcommand() parser, which already handles `git checkout -- .`. Not addressed here: `dd of=/dev/sda if=/dev/zero` with the operands reversed is still allowed, before and after, because the pattern requires `if=` immediately after `dd`. That is a different defect from the boundary bug and widening a P0 gate's pattern shape is a maintainer call. Refs #2642
Review on #2829 found the regex fix widened a false positive: matching `\bdd\s+if=` against the whole flattened line gated `echo dd if=/dev/zero` and `grep dd if=/dev/zero file`, neither of which runs dd. That class was already present before — `echo dd if=x` matched the old arm too — but the boundary fix extended it to the slash and dot spellings, so the arm now decides on text position rather than on what is being executed. dd moves to isDestructiveDd(tokens), next to isDestructiveRm and isDestructiveGit, and DESTRUCTIVE_SQL_DD goes back to SQL only. The per-segment loop already tokenizes every executable body, so the check runs where the command word is known. This resolves four things the text match could not: dd if=/dev/zero of=/dev/sda was allowed -> denied (the reported bug) sudo dd if=/dev/zero was allowed -> denied dd of=/dev/sda if=/dev/zero was allowed -> denied (operands are order-free) echo dd if=x was denied -> allowed (pre-existing false positive) Leading sudo/doas/env, their flags, and VAR=value assignment prefixes are skipped so a wrapped invocation still resolves to dd; flags are only skipped once a wrapper has been seen, so the scan cannot walk into an unrelated command's arguments. Tests: 6 fail on upstream main, 155 pass with this change. Refs #2642
The "does not gate as destructive" cases guarded the decision behind `if (output && output.hookSpecificOutput)`, so a crashed or silent hook made parseOutput return null and the test passed having asserted nothing. Assert the exit code and that output parsed first, then branch: a decision object must not be a Destructive deny, and pass-through must echo the input back — the same shape the existing retry case already checks. Raised in review on #2829.
Named in review on #2829: `dd bs=1M if=/dev/zero of=/dev/sda` is the same class as the reversed-operand case and is denied by the token-based check, but nothing pinned it.
Preserve prior agent/docs/package changes; bind actual 293-skill registry. Source-PR: #2444
Preserve exact-file protections, all shared assertions and private cleanup. Source-PR: #2835
Source-PR: #2879 Review-Manifest-SHA256: d561b12170431bc2f5ec9baa80548a656fca9e55b4bb6b11bd756be4e6cab199
Source-PR: #3241 Review-Manifest-SHA256: 281ec1d297672ff9986406c47e210b41590e00926b8da5071eceeedef8c51833
Keep the original wrapper assertions while supervising owned child groups through pipe closure, reporting failed capability probes, and preserving aggregate counts. Reviewed-Base: 807f07a Source-Manifest-SHA256: 75176a5cfe5345cc93c20fb748ba5a1f159fdeb171b12ae7a8619979fa55edd9 Final-Source-SHA256: 691cb15f7356845a2cc4f84277ff5fd24cd6210ec9995ffefea0ab19509ba076
Update four stale skill totals to the293 canonical entries verified by the existing catalog checker. Reviewed-Parent: 5d4cca8 Source-Manifest-SHA256: 5281891022c3405cfa0549bb15a15ce4b1df101b7c64780da9cbbd839df3d971
Preserve the original contributor histories and apply the exact independently reviewed repair. Source-Parent: aa0ea42 Review-Manifest-SHA256: 3d35033783c03f7660b6795e20c1de588fc1cd48850b208e36add955e2446ad7
Preserve the original contributor histories and apply the exact independently reviewed repair. Source-Parent: dee8846 Review-Manifest-SHA256: 0c7eeef7c2722a605d049daf2dc3c2473c10dd616005afcba21049fc036d8ce9
Preserve the original contributor histories and apply the exact independently reviewed repair. Source-Parent: e41c924 Review-Manifest-SHA256: df9ec6b308f1935e55f13af1845a99b87a0fc921fb30b16e52fbf70d5120ee81
Preserve the original contributor histories and apply the exact independently reviewed repair. Source-Parent: 74023e5 Review-Manifest-SHA256: 208a6350119857df7c1e5b83a46cdba0b7f7399f58ed60167397920c7a21b454
… resize The malformed-drawing rollback test took its marker baseline before the malformed poll but only checked it after resizeAgain(). Resize redraws the accepted view on its own, so the test still passed with the immediate rollback redraw removed and the canvas left blank until the operator resized. Record the log position alongside the baseline and assert the retained markers are on the canvas immediately after the malformed poll, before any resize. Scoped to the drawing case, since the events and lanes cases fail before draw() starts and correctly do not redraw. Mutating the implementation confirms the assertion bites: disabling the rollback draw and removing it entirely both fail it with the message "the rollback must redraw the retained markers immediately". Test-only change. control-plane-view-ui 27/27, control-plane-view 13/13, control-plane-view-ui-a11y 18/18, ESLint clean on the test file.
Anchor the PreToolUse and PostToolUseFailure MCP health-check matchers so non-MCP tool calls do not spawn the health hook.
…owups-20260928 fix: integrate reviewed contributor repairs and bounded installer tests
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
19 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )