Repository navigation
Import YAAS v2 snapshot - #1
Conversation
Dependency ReviewThe following issues were found:
License Issues.github/workflows/scan.yml
Allowed Licenses: BSD-1-Clause, BSD-2-Clause, BSD-3-Clause, MIT, MIT-0, Apache-1.1, Apache-2.0, Artistic-1.0, Artistic-2.0, PHP-3.0, PHP-3.01, PSF-2.0, Zlib, zlib-acknowledgement, BSL-1.0, OpenSSL, WTFPL, CC0-1.0, CC-PDDC, CC-BY-1.0, CC-BY-2.0, CC-BY-2.5, CC-BY-3.0, CC-BY-4.0, Unlicense, ISC, BlueOak-1.0.0, BSD-2-Clause-Patent, ADSL, Apache-2.0, APAFML, BSD-1-Clause, BSD-2-Clause, BSD-2-Clause-FreeBSD, BSD-2-Clause-NetBSD, BSD-2-Clause-Views, BSL-1.0, DSDP, ECL-1.0, ECL-2.0, ImageMagick, ISC, Linux-OpenIB, MIT, MIT-Modern-Variant, MS-PL, MulanPSL-1.0, Mup, PostgreSQL, Spencer-99, UPL-1.0, Xerox, 0BSD, AFL-1.1, AFL-1.2, AFL-2.0, AFL-2.1, AFL-3.0, AMDPLPA, AML, AMPAS, ANTLR-PD, ANTLR-PD-fallback, Apache-1.0, Apache-1.1, Artistic-2.0, Bahyph, Barr, BSD-3-Clause, BSD-3-Clause-Attribution, BSD-3-Clause-Clear, BSD-3-Clause-LBNL, BSD-3-Clause-Modification, BSD-3-Clause-No-Nuclear-License-2014, BSD-3-Clause-No-Nuclear-Warranty, BSD-3-Clause-Open-MPI, BSD-4-Clause, BSD-4-Clause-Shortened, BSD-4-Clause-UC, BSD-Source-Code, bzip2-1.0.5, bzip2-1.0.6, CC0-1.0, CNRI-Jython, CNRI-Python, CNRI-Python-GPL-Compatible, Cube, curl, eGenix, Entessa, FTL, HTMLTIDY, IBM-pibs, ICU, Info-ZIP, Intel, JasPer-2.0, Libpng, libpng-2.0, libtiff, LPPL-1.3c, MIT-0, MIT-advertising, MIT-open-group, MIT-CMU, MIT-enna, MIT-feh, MITNFA, MTLL, MulanPSL-2.0, Multics, Naumen, NCSA, Net-SNMP, NetCDF, NTP, OLDAP-2.0, OLDAP-2.0.1, OLDAP-2.1, OLDAP-2.2, OLDAP-2.2.1, OLDAP-2.2.2, OLDAP-2.3, OLDAP-2.4, OLDAP-2.5, OLDAP-2.6, OLDAP-2.7, OLDAP-2.8, OML, OpenSSL, PHP-3.0, PHP-3.01, Plexus, PSF-2.0, Python-2.0, Ruby, Saxpath, SGI-B-2.0, SMLNJ, SWL, TCL, TCP-wrappers, Unicode-DFS-2015, Unicode-DFS-2016, Unlicense, VSL-1.0, W3C, X11, XFree86-1.1, Xnet, xpp, Zlib, zlib-acknowledgement, ZPL-2.0, ZPL-2.1, AAL, Adobe-2006, Afmparse, Artistic-1.0, Artistic-1.0-cl8, Artistic-1.0-Perl, Beerware, blessing, Borceux, CECILL-B, ClArtistic, Condor-1.1, Crossword, CrystalStacker, diffmark, DOC, EFL-1.0, EFL-2.0, Fair, FSFUL, FSFULLR, Giftware, HPND, IJG, Leptonica, LPL-1.0, LPL-1.02, MirOS, mpich2, NASA-1.3, NBPL-1.0, Newsletr, NLPL, NRL, OGTSL, OLDAP-1.1, OLDAP-1.2, OLDAP-1.3, OLDAP-1.4, psutils, Qhull, Rdisc, RSA-MD, Spencer-86, Spencer-94, TU-Berlin-1.0, TU-Berlin-2.0, Vim, W3C-19980720, W3C-20150513, Wsuipa, WTFPL, xinetd, Zed, Zend-2.0, ZPL-1.1, GPL-2.0, GPL-2.0+, GPL-2.0-or-later, GPL-3.0, GPL-3.0+, GPL-3.0-or-later, LGPL-2.1, LGPL-2.1+, LGPL-2.1-or-later, LGPL-3.0, LGPL-3.0+, LGPL-3.0-or-later, MPL-1.1, MPL-2.0, MPL-2.0-no-copyleft-exception, CDDL-1.0, CDDL-1.1, CPL-1.0, IPL-1.0, EPL-1.0, EPL-2.0, Apache-1.0, CC-BY-SA-1.0, CC-BY-SA-2.0 Excluded from license check: OpenSSF Scorecard
Scanned Files
|
816dc1d to
7579000
Compare
Source: .git-yaas-v2/main@378baa783b342783ce282b3eda15bf932e804d71 Replaces the previous PR head while preserving c16bd1d as the parent. The scan workflow targets Circlefin master; uncommitted local files are excluded.
7579000 to
dc4ea18
Compare
|
@nexx88 As discussed, this PR is ready for the first public release. Would you be the right person to review and merge this? |
Source SHA: 4e0feedb677d76a98395e6c4e9ccac23b871145c (.git-yaas-v2/main). One-commit snapshot; full local history stays on .git-yaas-v2/main. Uncommitted and untracked working-tree files are excluded. scan.yml branch filters remain [master] in this delivery branch only. A watch that made no progress on three dispatches used to be parked as misconfig and surfaced as an approval card, frozen until a human cleared it. It now backs off (5m doubling to a 24h cap) and retries forever. A tick with no network no longer runs at all: every dispatch it makes is guaranteed to fail and charge a no-progress strike to a watch that did nothing wrong. The probe targets the configured backend's API and fails open on anything that is not unambiguously a network failure. The dashboard attributes both kinds of backoff to their quest and labels which kind, and review cards no longer render buttons that cannot work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Source SHA: 8be8a06a20b74668bb3b0b0722eeb0752dc65748 (.git-yaas-v2/main). One-commit snapshot; full local history stays on .git-yaas-v2/main. Uncommitted and untracked working-tree files are excluded. scan.yml branch filters remain [master] in this delivery branch only. An audit against the implementation found nine mismatches. Three would have produced a broken quest: the thread-retirement default was documented as 30 days in three places when the runtime uses 14; telegram_chat was offered as a watch type that has no checker, so such a quest scaffolds cleanly and is then skipped silently forever; and one-shot schedules (next_fire_ts) were supported by the runtime but undocumented and rejected by the scaffolder. The rest align the documented contracts with the code: slack_dm needs a channel_id, the validation section separates what the script enforces from what the caller must check, and the ops and dispatch references no longer describe an unacked watch as promoted to misconfig. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Watching a repo's issues needed a checker of its own: github_pr only sees pull
requests, and `gh search issues` excludes PRs, so the two watches cover the two
halves without ever double-reporting the same item.
github_issue.py inherits the watermark doctrine from github_pr.py wholesale --
the bounded ascending window, the tie-safety hold on a saturated page, and the
convention that `complete` means "everything up to advance_to", not "the backlog
is done". That doctrine is asserted again in its own suite rather than trusted
because the sibling is tested: a copied fix is exactly what rots when someone
simplifies the query back to a suffix scan and every count assertion still
passes.
Two things are new, and both are also backported to github_pr.py:
* `gh_account` resolves a token per subprocess via `gh auth token -u`, for a
repo that is private to a second GitHub account. Never `gh auth switch`,
which mutates global state and breaks every other repo in the same tick.
* `search` now refuses any token starting with `-`. The string is spliced into
argv ahead of gh's own flags, so a leading dash lands as a real flag:
`--include-prs` would put PRs back into an issues query and bill two
dispatches for one event. An adversarial review of the new checker found
this, along with a 404 classified as a retryable error rather than the
misconfig it is -- both fixed here, both with tests.
The skill is the other half. A checker is registered in seven places, none
derived from the others, and a missed one fails quietly rather than loudly: the
type scaffolds fine and then no watch of it ever fires. yaas-checker-authoring
lists all seven with the consequence of skipping each, plus the seven docs that
go stale, the tests a checker has to pass, and the two incidents (the 14-hour
suffix-scan stall, the $1k dispatch storm) that motivate the rules.
Source commit: 92654cfb03e0bf2a236d2d14392d2ad943feed97
a2e85ad to
441374e
Compare
…ip, stalled-item reclaim Acting on a review card now removes it immediately and offers a 6s undo toast instead of leaving the card on screen until the next poll. In-flight items (reviewed / executing / needs_reply) move out of the carousel into the strip below it, which is read-only: a worker is either mid-run or rewriting the text, so an edit box there is a lost edit. A stalled item is promoted back into the carousel with a single Reclaim control, since nobody is coming for it otherwise. Also: draft edits POST to /api/edit instead of overloading /api/review, the 'other' divider counts off surviving items so a dismissal cannot shift it onto the wrong card, and a drained queue reports what is still in flight instead of claiming 'All clear'.
d843f70 to
309aab8
Compare
|
@nexx88 final changes made, ready for you to merge. |
nexx88
left a comment
nexx88
left a comment
There was a problem hiding this comment.
Automated code review (Claude Code). No high-severity correctness bug; findings below range from unsafe-delete edge cases to doc/dead-code cleanups. The new log-event.py helper and its behaviour test (23/23 pass) are clean.
| # A quest id is a directory name. Reject traversal before touching the path. | ||
| if "/" in quest_id or quest_id in ("", ".", ".."): | ||
| die(f"invalid quest id '{quest_id}'") | ||
|
|
There was a problem hiding this comment.
Uncaught crash on NUL byte in quest_id (low). The guard only rejects /, so a quest_id with an embedded NUL byte passes, reaches Path.is_dir(), and raises an uncaught ValueError('embedded null byte') — a raw traceback instead of the intended error: invalid quest id exit 1. Unlikely from real callers, but it's an unhandled crash at an input-validation trust boundary. Consider rejecting \x00 (or any control char) alongside /.
| # checkers read them. | ||
| # | ||
| # Advisory, NOT a whitelist. Real timelines carry ~27 distinct event names, | ||
| # most of them quest-specific (brief_written, weekly_recap_posted, pr_merged, |
There was a problem hiding this comment.
Three helpers duplicated from slack-send.py (low). _quest_dir, _append_timeline, and _utc_now are byte-for-byte copies. The docstring justifies duplicating _repo_root (its path-bootstrap depth dependency is the very bug being fixed), but these three don't participate in bootstrap — once REPO_ROOT is resolved they could live in a shared module. Two copies means a fix to timeline-locking or quest-lookup logic must land in both files and can silently drift.
| # | ||
| # Advisory, NOT a whitelist. Real timelines carry ~27 distinct event names, | ||
| # most of them quest-specific (brief_written, weekly_recap_posted, pr_merged, | ||
| # tests_run). Rejecting those would push a worker straight back to hand-writing |
There was a problem hiding this comment.
PEP 604 union without from __future__ import annotations (very low / portability). -> Path | None is evaluated at definition time and raises TypeError on Python < 3.10 at import. Runtime here is 3.14 and this matches slack-send.py's convention, so this only bites a 3.9 deployment — flagging for completeness.
| - **A DM arrived from a watched partner** → read the thread context, compose a response (draft first unless quest explicitly authorizes `allow_send`), log action. | ||
| - **A new top-level message in a watched channel** → apply the quest's `context.md` decision rules. If the common fast-path is "log and ignore," just exit without any file edits. | ||
| - **A new email matching a watched query** → read the full message, apply the quest's `context.md` decision rules. **Always acknowledge the email** with a reply (via `yaas-triage/skills/yaas-gmail-reply/gmail-reply.py`) before or immediately after taking action — even if the action is just "request submitted, will follow up." Exception: bulk, automated, or notification emails where a human reply would be inappropriate. Log `info_received` or `message_sent` to `timeline.ndjson`. | ||
|
|
There was a problem hiding this comment.
Residual "log to timeline.ndjson" guidance (low). Some instructions still tell workers to write timeline.ndjson directly, contradicting the PR's "never write the NDJSON line yourself" rule. It passes the behaviour test (no <utc_iso> template for the grep to catch), but still nudges a worker toward hand-writing an entry with an invented ts — the exact failure log-event.py was added to prevent. Route these through the helper.
| """A slack_thread whose parent is older than the cutoff. cutoff_epoch None → never.""" | ||
| if cutoff_epoch is None: | ||
| return False | ||
| return w.get("type") == "slack_thread" and _thread_epoch(w) < cutoff_epoch |
There was a problem hiding this comment.
retire_thread deletes on unknown age — fails UNSAFE (higher severity). A slack_thread watch with a missing or non-numeric thread_ts makes _thread_epoch(w) return 0.0, so 0.0 < cutoff_epoch (~now-14d) is always true and the watch is silently retired/deleted — dropping tracking of a possibly-live thread. This contradicts the file's own ephemeral rule (_created_epoch/retire_ephemeral), which deliberately backfills unknown age and keeps the watch. Deletion is the highest-consequence op in this file; unknown age should be treated as young, not infinitely old.
| if not m or not page_ts: | ||
| break | ||
| cursor = m.group(1) | ||
| if pages >= MAX_PAGES: |
There was a problem hiding this comment.
Reaction sweep never early-stops — burns rate-limit budget. Results are sorted timestamp desc, so all new reactions are on the first page(s) and everything older is already in known. But the loop only breaks on cursor exhaustion or MAX_PAGES (30), so on a busy workspace it walks ~30 pages × 4 emojis ≈ 120 Slack searches every run — exactly the budget the tick's fairness rotation tries to conserve. Break once a full page yields no unknown ts.
| if not line: | ||
| continue | ||
| try: | ||
| ts = json.loads(line).get("ts", "") |
There was a problem hiding this comment.
Archives untimestamped run-log events regardless of age. An event line that parses as JSON but has no ts yields ts="", and "" < cutoff is always true, so the line is moved to the monthly archive out of the live run log. spend-window.py reads only the live log for its 24h budget window, so any untimestamped dispatch/cost event silently drops out of spend accounting even if recent. Skip (or keep) lines with no ts rather than treating them as ancient.
| breach = "" | ||
| if over(out["spend_1h"], cap_1h): | ||
| breach = f"1h spend ${out['spend_1h']:.2f} over cap ${float(cap_1h):.2f}" | ||
| elif over(out["spend_6h"], cap_6h): |
There was a problem hiding this comment.
Docstring/behavior contradiction for --cap-6h. Line ~44 documents --cap-6h as "accepted but unused," yet the breach ladder here (elif over(out['spend_6h'], cap_6h)) actively enforces it. A caller relying on the documented no-op would get dispatch blocked with a "6h spend over cap" breach. Reconcile the docstring with the enforcement (or vice versa).
nexx88
left a comment
nexx88
left a comment
There was a problem hiding this comment.
Re-reviewed the fixes against 6708d3a..c4be3cd — 10 of 11 findings resolved, the reactions early-stop correctly rejected, and the touched suites pass. One new gap the fixes introduced; otherwise everything is clean.
nexx88
left a comment
nexx88
left a comment
There was a problem hiding this comment.
Approved. Re-reviewed all fixes against the latest commit: 10 of 11 original findings resolved, the reactions early-stop correctly rejected as unsafe, and the follow-up PEP 604 gap in timeline_io.py is now fixed (annotation deferred, both CLIs import cleanly on the 3.9 floor). Touched suites pass; real test suites 44/44 + differential 29/0. Clean.
2 participants
Updates the committed YAAS v2 snapshot from source commit 378baa783b342783ce282b3eda15bf932e804d71.
The replacement upper commit remains parented to c16bd1d, preserving the existing PR history. Local history remains on .git-yaas-v2/main. Modified and untracked workspace files were excluded by importing the captured Git tree directly.
Delivery-only adjustment:
Verification: