chore: add echo-secret script - #9
Conversation
…agement documentation
…es and comment templates
…cs-in-dev-research feat(dev.research): add vendor docs support
… sensitive artifacts
…Projects and trigger task intake
📝 WalkthroughWalkthroughThis PR introduces a new dev.watch module for monitoring GitHub Projects v2, adds automation shortcuts for PR merging, refines control flow in dev.loop and dev.do workflows, and refactors Codex documentation to emphasize progress logging and pattern consolidation. Changes
Sequence DiagramsequenceDiagram
participant Poller as dev_watch.py<br/>(Polling)
participant API as GitHub API<br/>(GraphQL)
participant State as State File<br/>(Persistence)
participant Loops as loops.sh<br/>(Execution)
participant Codex as Codex<br/>(Task Processing)
Poller->>Poller: Load config & state
Poller->>API: GraphQL query for Projects v2
API-->>Poller: Items + statuses
Poller->>Poller: Parse URLs, extract issue metadata
Poller->>Poller: Compare against state (detect transitions)
Poller->>State: Aggregate events (Todo transitions)
State-->>Poller: Updated state
alt Events detected
Poller->>Loops: For each event: build task string
Loops->>Loops: Resolve repo context
Loops->>Codex: Stream prompt + task + repo info
Codex-->>Loops: Processing result
Loops-->>Poller: Execution status
end
Poller->>Poller: Emit JSON (timestamp, events, errors)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7df58fba6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "repository": {"nameWithOwner": "kevinslin/notion-agent-skill"}, | ||
| }, |
There was a problem hiding this comment.
The sample data uses trailing commas (e.g., after the repository object), which makes the file invalid JSON. Any user who tries to load this example with json.load (or uses it as a template for config/test fixtures) will get a parse error. Since this file is presented as .json, it should be syntactically valid JSON without trailing commas.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@active/dev.watch/assets/project_example.json`:
- Around line 1-177: The JSON file contains trailing commas after objects and
array elements (e.g., after repository objects like "repository":
{"nameWithOwner": "kevinslin/notion-agent-skill"},, after item objects with
"fieldValues": {...}, and after elements inside the top-level array), which
makes the file invalid; remove all trailing commas across the file so every
object property and array element ends without a comma (check entries with keys
"id", "content", "fieldValues", the nested "repository" objects, and the "nodes"
arrays) and revalidate with a JSON parser (e.g., python -m json.tool) to ensure
the top-level array and all nested objects are syntactically correct.
In `@active/dev.watch/references/config.md`:
- Around line 5-8: Fix the grammar and the documented default config filename:
change the phrase "SKILL_ROOT by located in ~/.llm/skills/dev.watch" to
"SKILL_ROOT is located in ~/.llm/skills/dev.watch" and update the referenced
config path from "$SKILL_ROOT/config.json" to "$SKILL_ROOT/dev.watch.json" to
match the code default (dev.watch.json); ensure the subsequent sentence that
instructs creating a JSON file clarifies the default filename (default:
`dev.watch.json`) and retains the example structure heading.
In `@active/dev.watch/scripts/dev_watch.py`:
- Around line 273-312: In run_loops, subprocess.run is called with check=False
which ignores the child process exit status; update the call so the function
propagates loops.sh failures by either using check=True and catching
subprocess.CalledProcessError (return non-zero on exception) or by keeping
check=False and inspecting the CompletedProcess.returncode to return that code;
ensure you reference the cmd/env used for the call, adjust the except block to
handle CalledProcessError (or use the returncode path) and return a non-zero
exit code when loops.sh fails so run_loops reflects the script's real status.
- Around line 315-371: The main() flow currently assumes load_json(config_path)
and load_json(state_path) succeed; wrap those calls to catch JSONDecodeError
(and generic ValueError) from malformed JSON: when loading config (config =
load_json(config_path)) catch the error, eprint a clear message including
config_path and the error text, and return 2; when loading state (state =
load_json(state_path, default={}) or {}) catch the error, eprint a message
including state_path and the error, and recover by setting state = {}
(optionally back up or overwrite the corrupt file) so the watcher continues;
update references to config and state accordingly and ensure subsequent code
(e.g., save_json(state_path, state)) still runs with the recovered state.
In `@active/dev.watch/scripts/post_issue_comment.py`:
- Around line 15-27: load_json currently lets a JSONDecodeError bubble up
causing a traceback; update load_json to catch json.JSONDecodeError and handle
it by writing a clear error message via eprint (or raising a controlled
SystemExit with a friendly message/exit code) and then return None or exit, and
also factor out the duplicated helpers eprint, expand_path, and load_json into a
shared helper module used by both post_issue_comment.py and dev_watch.py to
avoid drift; reference the functions eprint, expand_path, and load_json when
making these changes.
- Around line 76-88: In resolve_message, opening the message_file can raise
OSError and currently produces a raw traceback; wrap the file open/read in a
try/except catching OSError (and optionally UnicodeError) and raise a ValueError
with a clear CLI-friendly message that includes the message_file path and the
underlying error text; keep the existing behavior for mutually exclusive args
and stdin, but replace the raw exception path in resolve_message so callers see
a clean ValueError describing the missing/invalid message file (reference
function name resolve_message and variable message_file/ path).
🧹 Nitpick comments (6)
active/dev.shortcuts/shortcuts/fix-pr.md (1)
9-10: Clarify how to verify “already on the PR branch.”To avoid false positives, specify that the current branch should be compared to the PR head ref (via
gh pr view <pr> --json headRefName) before skipping worktree creation.active/dev.shortcuts/shortcuts/merge-pr.md (1)
10-10: Fix terminology to avoid missing staged changes.“Unstaged commits” is inaccurate. Use “uncommitted changes (staged or unstaged)” so staged changes aren’t ignored.
Proposed edit
-1. Check if there are any unstaged commits. If so, use `@shortcut`:commit-code to commit unstaged changes. +1. Check if there are any uncommitted changes (staged or unstaged). If so, use `@shortcut`:commit-code to commit them.active/dev.do/SKILL.md (1)
38-44: Specify how/where to update “Status.”This instruction is ambiguous (Issue field? Projects v2? other tracker). Add the exact system and command (e.g.,
gh issue edit, Projects v2 field update) so the workflow is executable.active/dev.watch/SKILL.md (1)
23-25: Add language identifiers to fenced code blocks.markdownlint flags missing languages; these are shell commands, so
bashis appropriate.💡 Suggested update
-``` +```bash python scripts/dev_watch.py --config dev.watch.json --once -``` +``` -``` +```bash python scripts/dev_watch.py --config dev.watch.json --watch --interval 60 -``` +``` -``` +```bash python scripts/post_issue_comment.py --issue-url "<issue-url>" --message "Questions here" -``` +```Also applies to: 29-31, 45-47
docs/specs/active/2026-02-01-trigger-loops-sh-on-task-detection.md (1)
124-129: Align the risks table pipes to satisfy markdownlint (MD060).🧹 Suggested alignment
-| Risk | Impact | Probability | Mitigation Strategy | -|------|--------|-------------|---------------------| -| loops.sh output breaks JSON parsing for dev.watch consumers | Medium | Medium | Redirect loops output to stderr or a log file. | -| Repo path mismatch (`$HOME/code/<repo-name>` not present) | Medium | Medium | Warn and ask for guidance if the directory is missing. | -| Codex CLI missing or misconfigured | Medium | Low | Provide a clear error and exit early. | -| Long-running Codex blocks watch loop | Medium | Medium | Support `--parallel` to fork when desired; document default sync behavior. | +| Risk | Impact | Probability | Mitigation Strategy | +|--------------------------------------------------------------|--------|-------------|------------------------------------------------------------------| +| loops.sh output breaks JSON parsing for dev.watch consumers | Medium | Medium | Redirect loops output to stderr or a log file. | +| Repo path mismatch (`$HOME/code/<repo-name>` not present) | Medium | Medium | Warn and ask for guidance if the directory is missing. | +| Codex CLI missing or misconfigured | Medium | Low | Provide a clear error and exit early. | +| Long-running Codex blocks watch loop | Medium | Medium | Support `--parallel` to fork when desired; document default sync behavior. |references/codex.md (1)
15-25: Specify a language for the fenced template block.✍️ Suggested update
-``` +```text ## [Date/Time] - [Story ID] - What was implemented - Files changed - **Learnings for future iterations:** - Patterns discovered (e.g., "this codebase uses X for Y") - Gotchas encountered (e.g., "don't forget to update Z when changing W") - Useful context (e.g., "the evaluation panel is in component X") --- -``` +```
| [ | ||
| { | ||
| "id": "PVTI_lAHOAAY-6c4BN9FczgkmjzE", | ||
| "content": { | ||
| "__typename": "Issue", | ||
| "id": "I_kwDORF0dd87nTXSB", | ||
| "url": "https://github.com/kevinslin/notion-agent-skill/issues/1", | ||
| "title": "sync should support syncing to relation fields", | ||
| "number": 1, | ||
| "repository": {"nameWithOwner": "kevinslin/notion-agent-skill"}, | ||
| }, | ||
| "fieldValues": { | ||
| "nodes": [ | ||
| {"__typename": "ProjectV2ItemFieldRepositoryValue"}, | ||
| { | ||
| "__typename": "ProjectV2ItemFieldSingleSelectValue", | ||
| "name": "Backlog", | ||
| "field": { | ||
| "__typename": "ProjectV2SingleSelectField", | ||
| "name": "Status", | ||
| }, | ||
| }, | ||
| {"__typename": "ProjectV2ItemFieldTextValue"}, | ||
| ] | ||
| }, | ||
| }, | ||
| { | ||
| "id": "PVTI_lAHOAAY-6c4BN9Fczgknw2o", | ||
| "content": { | ||
| "__typename": "Issue", | ||
| "id": "I_kwDORF0dd87naQQm", | ||
| "url": "https://github.com/kevinslin/notion-agent-skill/issues/8", | ||
| "title": "Add integration test for testing filter with `fetch` command", | ||
| "number": 8, | ||
| "repository": {"nameWithOwner": "kevinslin/notion-agent-skill"}, | ||
| }, | ||
| "fieldValues": { | ||
| "nodes": [ | ||
| {"__typename": "ProjectV2ItemFieldRepositoryValue"}, | ||
| {"__typename": "ProjectV2ItemFieldTextValue"}, | ||
| { | ||
| "__typename": "ProjectV2ItemFieldSingleSelectValue", | ||
| "name": "Backlog", | ||
| "field": { | ||
| "__typename": "ProjectV2SingleSelectField", | ||
| "name": "Status", | ||
| }, | ||
| }, | ||
| ] | ||
| }, | ||
| }, | ||
| { | ||
| "id": "PVTI_lAHOAAY-6c4BN9FczgkmxoA", | ||
| "content": { | ||
| "__typename": "Issue", | ||
| "id": "I_kwDORF0dd87nVSgg", | ||
| "url": "https://github.com/kevinslin/notion-agent-skill/issues/5", | ||
| "title": "support fetch command", | ||
| "number": 5, | ||
| "repository": {"nameWithOwner": "kevinslin/notion-agent-skill"}, | ||
| }, | ||
| "fieldValues": { | ||
| "nodes": [ | ||
| {"__typename": "ProjectV2ItemFieldRepositoryValue"}, | ||
| {"__typename": "ProjectV2ItemFieldLabelValue"}, | ||
| { | ||
| "__typename": "ProjectV2ItemFieldSingleSelectValue", | ||
| "name": "Done", | ||
| "field": { | ||
| "__typename": "ProjectV2SingleSelectField", | ||
| "name": "Status", | ||
| }, | ||
| }, | ||
| {"__typename": "ProjectV2ItemFieldTextValue"}, | ||
| ] | ||
| }, | ||
| }, | ||
| { | ||
| "id": "PVTI_lAHOAAY-6c4BN9FczgkmW9Y", | ||
| "content": { | ||
| "__typename": "Issue", | ||
| "id": "I_kwDOQyyzWs7nR7-t", | ||
| "url": "https://github.com/kevinslin/skills/issues/4", | ||
| "title": "Support doc from vendor notes", | ||
| "number": 4, | ||
| "repository": {"nameWithOwner": "kevinslin/skills"}, | ||
| }, | ||
| "fieldValues": { | ||
| "nodes": [ | ||
| {"__typename": "ProjectV2ItemFieldRepositoryValue"}, | ||
| { | ||
| "__typename": "ProjectV2ItemFieldSingleSelectValue", | ||
| "name": "Done", | ||
| "field": { | ||
| "__typename": "ProjectV2SingleSelectField", | ||
| "name": "Status", | ||
| }, | ||
| }, | ||
| {"__typename": "ProjectV2ItemFieldTextValue"}, | ||
| ] | ||
| }, | ||
| }, | ||
| { | ||
| "id": "PVTI_lAHOAAY-6c4BN9FczgkmBPA", | ||
| "content": { | ||
| "__typename": "Issue", | ||
| "id": "I_kwDOQyyzWs7nP1vx", | ||
| "url": "https://github.com/kevinslin/skills/issues/1", | ||
| "title": "Support promoting shortcut to skill", | ||
| "number": 1, | ||
| "repository": {"nameWithOwner": "kevinslin/skills"}, | ||
| }, | ||
| "fieldValues": { | ||
| "nodes": [ | ||
| {"__typename": "ProjectV2ItemFieldRepositoryValue"}, | ||
| {"__typename": "ProjectV2ItemFieldTextValue"}, | ||
| { | ||
| "__typename": "ProjectV2ItemFieldSingleSelectValue", | ||
| "name": "Backlog", | ||
| "field": { | ||
| "__typename": "ProjectV2SingleSelectField", | ||
| "name": "Status", | ||
| }, | ||
| }, | ||
| ] | ||
| }, | ||
| }, | ||
| { | ||
| "id": "PVTI_lAHOAAY-6c4BN9Fczgknw9E", | ||
| "content": { | ||
| "__typename": "Issue", | ||
| "id": "I_kwDOQyyzWs7naRUe", | ||
| "url": "https://github.com/kevinslin/skills/issues/8", | ||
| "title": "dev.do auto mode", | ||
| "number": 8, | ||
| "repository": {"nameWithOwner": "kevinslin/skills"}, | ||
| }, | ||
| "fieldValues": { | ||
| "nodes": [ | ||
| {"__typename": "ProjectV2ItemFieldRepositoryValue"}, | ||
| {"__typename": "ProjectV2ItemFieldTextValue"}, | ||
| { | ||
| "__typename": "ProjectV2ItemFieldSingleSelectValue", | ||
| "name": "Backlog", | ||
| "field": { | ||
| "__typename": "ProjectV2SingleSelectField", | ||
| "name": "Status", | ||
| }, | ||
| }, | ||
| ] | ||
| }, | ||
| }, | ||
| { | ||
| "id": "PVTI_lAHOAAY-6c4BN9Fczgkm_zE", | ||
| "content": { | ||
| "__typename": "Issue", | ||
| "id": "I_kwDOQyyzWs7nWoYS", | ||
| "url": "https://github.com/kevinslin/skills/issues/7", | ||
| "title": "Test Issue", | ||
| "number": 7, | ||
| "repository": {"nameWithOwner": "kevinslin/skills"}, | ||
| }, | ||
| "fieldValues": { | ||
| "nodes": [ | ||
| {"__typename": "ProjectV2ItemFieldRepositoryValue"}, | ||
| { | ||
| "__typename": "ProjectV2ItemFieldSingleSelectValue", | ||
| "name": "Todo", | ||
| "field": { | ||
| "__typename": "ProjectV2SingleSelectField", | ||
| "name": "Status", | ||
| }, | ||
| }, | ||
| {"__typename": "ProjectV2ItemFieldTextValue"}, | ||
| ] | ||
| }, | ||
| } |
There was a problem hiding this comment.
Remove trailing commas — the file is invalid JSON.
JSON does not allow trailing commas after the last property/element, so parsers will fail. Please remove all trailing commas throughout the file (properties, objects, and arrays).
🛠️ Example fix (apply pattern across file)
- "repository": {"nameWithOwner": "kevinslin/notion-agent-skill"},
- },
+ "repository": {"nameWithOwner": "kevinslin/notion-agent-skill"}
+ }You can verify validity after the cleanup:
#!/bin/bash
python -m json.tool active/dev.watch/assets/project_example.json >/dev/null🧰 Tools
🪛 Biome (2.3.13)
[error] 11-11: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 21-21: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 22-22: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 24-24: Expected an array, an object, or a literal but instead found ']'.
Expected an array, an object, or a literal here.
(parse)
[error] 26-26: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 36-36: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 47-47: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 48-48: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 49-49: Expected an array, an object, or a literal but instead found ']'.
Expected an array, an object, or a literal here.
(parse)
[error] 51-51: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 61-61: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 72-72: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 73-73: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 75-75: Expected an array, an object, or a literal but instead found ']'.
Expected an array, an object, or a literal here.
(parse)
[error] 77-77: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 87-87: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 97-97: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 98-98: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 100-100: Expected an array, an object, or a literal but instead found ']'.
Expected an array, an object, or a literal here.
(parse)
[error] 102-102: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 112-112: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 123-123: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 124-124: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 125-125: Expected an array, an object, or a literal but instead found ']'.
Expected an array, an object, or a literal here.
(parse)
[error] 127-127: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 137-137: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 148-148: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 149-149: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 150-150: Expected an array, an object, or a literal but instead found ']'.
Expected an array, an object, or a literal here.
(parse)
[error] 152-152: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 162-162: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 172-172: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 173-173: Expected a property but instead found '}'.
Expected a property here.
(parse)
[error] 175-175: Expected an array, an object, or a literal but instead found ']'.
Expected an array, an object, or a literal here.
(parse)
[error] 177-177: Expected a property but instead found '}'.
Expected a property here.
(parse)
🤖 Prompt for AI Agents
In `@active/dev.watch/assets/project_example.json` around lines 1 - 177, The JSON
file contains trailing commas after objects and array elements (e.g., after
repository objects like "repository": {"nameWithOwner":
"kevinslin/notion-agent-skill"},, after item objects with "fieldValues": {...},
and after elements inside the top-level array), which makes the file invalid;
remove all trailing commas across the file so every object property and array
element ends without a comma (check entries with keys "id", "content",
"fieldValues", the nested "repository" objects, and the "nodes" arrays) and
revalidate with a JSON parser (e.g., python -m json.tool) to ensure the
top-level array and all nested objects are syntactically correct.
| SKILL_ROOT by located in ~/.llm/skills/dev.watch unless overridden elsewhere. | ||
| The dev.watch config can be found in $SKILL_ROOT/config.json | ||
|
|
||
| Create a JSON file (default: `dev.watch.json`) with the following structure: |
There was a problem hiding this comment.
Fix grammar and align the documented config path with the actual default.
The wording has a typo, and the doc references $SKILL_ROOT/config.json while the code defaults to dev.watch.json. This mismatch will confuse users setting up the config.
✏️ Proposed doc fix
-SKILL_ROOT by located in ~/.llm/skills/dev.watch unless overridden elsewhere.
-The dev.watch config can be found in $SKILL_ROOT/config.json
+SKILL_ROOT is located in ~/.llm/skills/dev.watch unless overridden elsewhere.
+The dev.watch config can be found in $SKILL_ROOT/dev.watch.json (or $DEV_WATCH_CONFIG).
-Create a JSON file (default: `dev.watch.json`) with the following structure:
+Create a JSON file (default: `dev.watch.json`) with the following structure:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SKILL_ROOT by located in ~/.llm/skills/dev.watch unless overridden elsewhere. | |
| The dev.watch config can be found in $SKILL_ROOT/config.json | |
| Create a JSON file (default: `dev.watch.json`) with the following structure: | |
| SKILL_ROOT is located in ~/.llm/skills/dev.watch unless overridden elsewhere. | |
| The dev.watch config can be found in $SKILL_ROOT/dev.watch.json (or $DEV_WATCH_CONFIG). | |
| Create a JSON file (default: `dev.watch.json`) with the following structure: |
🤖 Prompt for AI Agents
In `@active/dev.watch/references/config.md` around lines 5 - 8, Fix the grammar
and the documented default config filename: change the phrase "SKILL_ROOT by
located in ~/.llm/skills/dev.watch" to "SKILL_ROOT is located in
~/.llm/skills/dev.watch" and update the referenced config path from
"$SKILL_ROOT/config.json" to "$SKILL_ROOT/dev.watch.json" to match the code
default (dev.watch.json); ensure the subsequent sentence that instructs creating
a JSON file clarifies the default filename (default: `dev.watch.json`) and
retains the example structure heading.
| def run_loops(event, config): | ||
| script_dir = os.path.dirname(os.path.abspath(__file__)) | ||
| loops_script = os.path.join(script_dir, "loops.sh") | ||
| if not os.path.exists(loops_script): | ||
| eprint(f"loops.sh not found at {loops_script}") | ||
| return 1 | ||
|
|
||
| task = build_task_string(event) | ||
| if not task: | ||
| eprint("Skipping loops.sh: empty task string.") | ||
| return 1 | ||
|
|
||
| repo_name = "" | ||
| repo_full = event.get("issue_repo") | ||
| if repo_full and "/" in repo_full: | ||
| repo_name = repo_full.rsplit("/", 1)[-1] | ||
|
|
||
| cmd = [loops_script] | ||
| if config.get("loops_parallel", DEFAULT_LOOPS_PARALLEL): | ||
| cmd.append("--parallel") | ||
| if repo_name: | ||
| cmd.extend(["--repo", repo_name]) | ||
| cmd.append(task) | ||
|
|
||
| env = os.environ.copy() | ||
| token = config.get("_token") | ||
| if token: | ||
| env.setdefault("GITHUB_TOKEN", token) | ||
| env.setdefault("GH_TOKEN", token) | ||
| token_env = config.get("github_token_env", DEFAULT_TOKEN_ENV) | ||
| if token_env and re.match(r"^[A-Za-z_][A-Za-z0-9_]*$", token_env): | ||
| env.setdefault(token_env, token) | ||
|
|
||
| try: | ||
| eprint(f"running {cmd}") | ||
| subprocess.run(cmd, check=False, stdout=sys.stderr, stderr=sys.stderr, env=env) | ||
| except OSError as err: | ||
| eprint(f"Failed to run loops.sh: {err}") | ||
| return 1 | ||
| return 0 |
There was a problem hiding this comment.
Propagate loops.sh failures.
subprocess.run(..., check=False) ignores the return code, so failed loops are treated as success. This makes exit_code unreliable.
🛠️ Suggested fix to honor exit status
- try:
- eprint(f"running {cmd}")
- subprocess.run(cmd, check=False, stdout=sys.stderr, stderr=sys.stderr, env=env)
+ try:
+ eprint(f"running {cmd}")
+ result = subprocess.run(
+ cmd, check=False, stdout=sys.stderr, stderr=sys.stderr, env=env
+ )
+ if result.returncode != 0:
+ eprint(f"loops.sh exited with status {result.returncode}")
+ return result.returncode
except OSError as err:
eprint(f"Failed to run loops.sh: {err}")
return 1
return 0🧰 Tools
🪛 Ruff (0.14.14)
[error] 308-308: subprocess call: check for execution of untrusted input
(S603)
🤖 Prompt for AI Agents
In `@active/dev.watch/scripts/dev_watch.py` around lines 273 - 312, In run_loops,
subprocess.run is called with check=False which ignores the child process exit
status; update the call so the function propagates loops.sh failures by either
using check=True and catching subprocess.CalledProcessError (return non-zero on
exception) or by keeping check=False and inspecting the
CompletedProcess.returncode to return that code; ensure you reference the
cmd/env used for the call, adjust the except block to handle CalledProcessError
(or use the returncode path) and return a non-zero exit code when loops.sh fails
so run_loops reflects the script's real status.
| def main(): | ||
| args = parse_args() | ||
| if args.once and args.watch: | ||
| eprint("Choose either --once or --watch, not both.") | ||
| return 2 | ||
|
|
||
| config_path = expand_path(args.config) | ||
| config = load_json(config_path) | ||
| if not config: | ||
| eprint(f"Config not found or empty: {config_path}") | ||
| return 2 | ||
|
|
||
| token_env = config.get("github_token_env", DEFAULT_TOKEN_ENV) | ||
| token = os.getenv(token_env) | ||
| if not token and token_env.startswith(TOKEN_PREFIXES): | ||
| token = token_env | ||
| if not token: | ||
| eprint(f"Missing GitHub token in env var: {token_env}") | ||
| return 2 | ||
| config["_token"] = token | ||
|
|
||
| state_path = expand_path(config.get("state_file", DEFAULT_STATE_FILE)) | ||
| state = load_json(state_path, default={}) or {} | ||
|
|
||
| poll_interval = args.interval or config.get("poll_interval_seconds", DEFAULT_POLL_INTERVAL) | ||
| emit_on_first_run = args.emit_on_first_run or config.get("emit_on_first_run", False) | ||
|
|
||
| if not args.watch and not args.once: | ||
| args.once = True | ||
|
|
||
| exit_code = 0 | ||
| while True: | ||
| timestamp = datetime.now(timezone.utc).isoformat() | ||
| events, errors = collect_events(config, state, emit_on_first_run, args.force) | ||
| payload = { | ||
| "timestamp": timestamp, | ||
| "events": events, | ||
| "errors": errors, | ||
| } | ||
| print(json.dumps(payload, sort_keys=True)) | ||
| sys.stdout.flush() | ||
|
|
||
| if events: | ||
| for event in events: | ||
| result = run_loops(event, config) | ||
| if result != 0: | ||
| exit_code = 1 | ||
|
|
||
| save_json(state_path, state) | ||
| if errors: | ||
| exit_code = 1 | ||
|
|
||
| if args.once: | ||
| break | ||
| time.sleep(max(5, int(poll_interval))) | ||
|
|
||
| return exit_code |
There was a problem hiding this comment.
Handle invalid JSON for config/state files without crashing.
A corrupt config or state file will currently raise a JSONDecodeError and terminate the watcher. Please add a clean error path so the process exits with a readable message (or resets state for the state file).
🛠️ Example handling in `main()`
- config = load_json(config_path)
+ try:
+ config = load_json(config_path)
+ except json.JSONDecodeError as err:
+ eprint(f"Invalid JSON in config: {config_path} ({err})")
+ return 2
if not config:
eprint(f"Config not found or empty: {config_path}")
return 2
@@
- state = load_json(state_path, default={}) or {}
+ try:
+ state = load_json(state_path, default={}) or {}
+ except json.JSONDecodeError as err:
+ eprint(f"Invalid JSON in state file: {state_path} ({err})")
+ state = {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def main(): | |
| args = parse_args() | |
| if args.once and args.watch: | |
| eprint("Choose either --once or --watch, not both.") | |
| return 2 | |
| config_path = expand_path(args.config) | |
| config = load_json(config_path) | |
| if not config: | |
| eprint(f"Config not found or empty: {config_path}") | |
| return 2 | |
| token_env = config.get("github_token_env", DEFAULT_TOKEN_ENV) | |
| token = os.getenv(token_env) | |
| if not token and token_env.startswith(TOKEN_PREFIXES): | |
| token = token_env | |
| if not token: | |
| eprint(f"Missing GitHub token in env var: {token_env}") | |
| return 2 | |
| config["_token"] = token | |
| state_path = expand_path(config.get("state_file", DEFAULT_STATE_FILE)) | |
| state = load_json(state_path, default={}) or {} | |
| poll_interval = args.interval or config.get("poll_interval_seconds", DEFAULT_POLL_INTERVAL) | |
| emit_on_first_run = args.emit_on_first_run or config.get("emit_on_first_run", False) | |
| if not args.watch and not args.once: | |
| args.once = True | |
| exit_code = 0 | |
| while True: | |
| timestamp = datetime.now(timezone.utc).isoformat() | |
| events, errors = collect_events(config, state, emit_on_first_run, args.force) | |
| payload = { | |
| "timestamp": timestamp, | |
| "events": events, | |
| "errors": errors, | |
| } | |
| print(json.dumps(payload, sort_keys=True)) | |
| sys.stdout.flush() | |
| if events: | |
| for event in events: | |
| result = run_loops(event, config) | |
| if result != 0: | |
| exit_code = 1 | |
| save_json(state_path, state) | |
| if errors: | |
| exit_code = 1 | |
| if args.once: | |
| break | |
| time.sleep(max(5, int(poll_interval))) | |
| return exit_code | |
| def main(): | |
| args = parse_args() | |
| if args.once and args.watch: | |
| eprint("Choose either --once or --watch, not both.") | |
| return 2 | |
| config_path = expand_path(args.config) | |
| try: | |
| config = load_json(config_path) | |
| except json.JSONDecodeError as err: | |
| eprint(f"Invalid JSON in config: {config_path} ({err})") | |
| return 2 | |
| if not config: | |
| eprint(f"Config not found or empty: {config_path}") | |
| return 2 | |
| token_env = config.get("github_token_env", DEFAULT_TOKEN_ENV) | |
| token = os.getenv(token_env) | |
| if not token and token_env.startswith(TOKEN_PREFIXES): | |
| token = token_env | |
| if not token: | |
| eprint(f"Missing GitHub token in env var: {token_env}") | |
| return 2 | |
| config["_token"] = token | |
| state_path = expand_path(config.get("state_file", DEFAULT_STATE_FILE)) | |
| try: | |
| state = load_json(state_path, default={}) or {} | |
| except json.JSONDecodeError as err: | |
| eprint(f"Invalid JSON in state file: {state_path} ({err})") | |
| state = {} | |
| poll_interval = args.interval or config.get("poll_interval_seconds", DEFAULT_POLL_INTERVAL) | |
| emit_on_first_run = args.emit_on_first_run or config.get("emit_on_first_run", False) | |
| if not args.watch and not args.once: | |
| args.once = True | |
| exit_code = 0 | |
| while True: | |
| timestamp = datetime.now(timezone.utc).isoformat() | |
| events, errors = collect_events(config, state, emit_on_first_run, args.force) | |
| payload = { | |
| "timestamp": timestamp, | |
| "events": events, | |
| "errors": errors, | |
| } | |
| print(json.dumps(payload, sort_keys=True)) | |
| sys.stdout.flush() | |
| if events: | |
| for event in events: | |
| result = run_loops(event, config) | |
| if result != 0: | |
| exit_code = 1 | |
| save_json(state_path, state) | |
| if errors: | |
| exit_code = 1 | |
| if args.once: | |
| break | |
| time.sleep(max(5, int(poll_interval))) | |
| return exit_code |
🤖 Prompt for AI Agents
In `@active/dev.watch/scripts/dev_watch.py` around lines 315 - 371, The main()
flow currently assumes load_json(config_path) and load_json(state_path) succeed;
wrap those calls to catch JSONDecodeError (and generic ValueError) from
malformed JSON: when loading config (config = load_json(config_path)) catch the
error, eprint a clear message including config_path and the error text, and
return 2; when loading state (state = load_json(state_path, default={}) or {})
catch the error, eprint a message including state_path and the error, and
recover by setting state = {} (optionally back up or overwrite the corrupt file)
so the watcher continues; update references to config and state accordingly and
ensure subsequent code (e.g., save_json(state_path, state)) still runs with the
recovered state.
| def eprint(message): | ||
| print(message, file=sys.stderr) | ||
|
|
||
|
|
||
| def expand_path(path): | ||
| return os.path.expandvars(os.path.expanduser(path)) | ||
|
|
||
|
|
||
| def load_json(path): | ||
| if not path or not os.path.exists(path): | ||
| return None | ||
| with open(path, "r", encoding="utf-8") as handle: | ||
| return json.load(handle) |
There was a problem hiding this comment.
Handle invalid JSON gracefully and consider consolidating shared helpers.
load_jsonwill currently raise a rawJSONDecodeErroron malformed config. That leads to a traceback instead of a clean CLI error.eprint/expand_path/load_jsonduplicate logic fromactive/dev.watch/scripts/dev_watch.py. Consider extracting a shared helper module to avoid drift.
🛠️ Suggested error handling for invalid JSON
def load_json(path):
if not path or not os.path.exists(path):
return None
with open(path, "r", encoding="utf-8") as handle:
- return json.load(handle)
+ try:
+ return json.load(handle)
+ except json.JSONDecodeError as err:
+ eprint(f"Invalid JSON in {path}: {err}")
+ return None🤖 Prompt for AI Agents
In `@active/dev.watch/scripts/post_issue_comment.py` around lines 15 - 27,
load_json currently lets a JSONDecodeError bubble up causing a traceback; update
load_json to catch json.JSONDecodeError and handle it by writing a clear error
message via eprint (or raising a controlled SystemExit with a friendly
message/exit code) and then return None or exit, and also factor out the
duplicated helpers eprint, expand_path, and load_json into a shared helper
module used by both post_issue_comment.py and dev_watch.py to avoid drift;
reference the functions eprint, expand_path, and load_json when making these
changes.
| def resolve_message(message, message_file): | ||
| if message and message_file: | ||
| raise ValueError("Provide either --message or --message-file, not both.") | ||
| if message_file: | ||
| path = expand_path(message_file) | ||
| with open(path, "r", encoding="utf-8") as handle: | ||
| return handle.read().strip() | ||
| if message: | ||
| return message.strip() | ||
| stdin = sys.stdin.read().strip() | ||
| if stdin: | ||
| return stdin | ||
| raise ValueError("Missing comment body. Provide --message, --message-file, or stdin.") |
There was a problem hiding this comment.
Add friendly handling for missing/invalid message files.
open() can raise OSError (missing file, permissions), which currently bubbles as a traceback. Convert to a clean CLI error.
🛠️ Suggested error handling for message files
if message_file:
path = expand_path(message_file)
- with open(path, "r", encoding="utf-8") as handle:
- return handle.read().strip()
+ try:
+ with open(path, "r", encoding="utf-8") as handle:
+ return handle.read().strip()
+ except OSError as err:
+ raise ValueError(f"Unable to read message file: {path}") from err📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def resolve_message(message, message_file): | |
| if message and message_file: | |
| raise ValueError("Provide either --message or --message-file, not both.") | |
| if message_file: | |
| path = expand_path(message_file) | |
| with open(path, "r", encoding="utf-8") as handle: | |
| return handle.read().strip() | |
| if message: | |
| return message.strip() | |
| stdin = sys.stdin.read().strip() | |
| if stdin: | |
| return stdin | |
| raise ValueError("Missing comment body. Provide --message, --message-file, or stdin.") | |
| def resolve_message(message, message_file): | |
| if message and message_file: | |
| raise ValueError("Provide either --message or --message-file, not both.") | |
| if message_file: | |
| path = expand_path(message_file) | |
| try: | |
| with open(path, "r", encoding="utf-8") as handle: | |
| return handle.read().strip() | |
| except OSError as err: | |
| raise ValueError(f"Unable to read message file: {path}") from err | |
| if message: | |
| return message.strip() | |
| stdin = sys.stdin.read().strip() | |
| if stdin: | |
| return stdin | |
| raise ValueError("Missing comment body. Provide --message, --message-file, or stdin.") |
🧰 Tools
🪛 Ruff (0.14.14)
[warning] 78-78: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 88-88: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In `@active/dev.watch/scripts/post_issue_comment.py` around lines 76 - 88, In
resolve_message, opening the message_file can raise OSError and currently
produces a raw traceback; wrap the file open/read in a try/except catching
OSError (and optionally UnicodeError) and raise a ValueError with a clear
CLI-friendly message that includes the message_file path and the underlying
error text; keep the existing behavior for mutually exclusive args and stdin,
but replace the raw exception path in resolve_message so callers see a clean
ValueError describing the missing/invalid message file (reference function name
resolve_message and variable message_file/ path).
2 participants
chore: add echo-secret script
Context
Adds scripts/echo-secret.sh that outputs 42 and records the execution plan for Issue #7.
Related: #7
Testing
Summary by CodeRabbit
New Features
Documentation
Improvements