Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions .github/workflows/deepseek-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ name: 'DeepSeek CI (peer Omni ↔ OpenRouter HTTPS + opt-in web-wrapper)'
# The DeepSeek web-wrapper is intentionally a separate, explicit third-peer path
# (see the deepseek-webwrapper job) and stays disabled until a real, non-simulated
# invocation exists. Never caches/uploads/commits session cookies or tokens.
#
# Review job runs only on non-fork, non-draft PRs and uses a distinct comment
# marker to avoid double-counting free-tier quota against gemini-after-peers.
Comment on lines +14 to +16

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Header comment claims a distinct comment marker that this PR does not add

The new header text says the review job "uses a distinct comment marker to avoid double-counting free-tier quota against gemini-after-peers", but no marker is set anywhere in this workflow. Markers are derived inside the shared action from provider+role (<!-- omni-review --> / <!-- openrouter-review -->, .github/actions/http-llm-invoke/action.yml:97-107), so the distinctness is incidental to provider choice, not something added here. If a gemini job also runs with provider omni/openrouter and role review, comments would still collide.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


on:
pull_request:
Expand All @@ -31,7 +34,10 @@ permissions: {}

jobs:
review:
if: github.event_name == 'pull_request'
if: |
github.event_name == 'pull_request' &&
github.event.pull_request.head.repo.fork == false &&
github.event.pull_request.draft == false
runs-on: ubuntu-latest
permissions:
contents: read
Expand Down Expand Up @@ -71,16 +77,19 @@ jobs:
fi
DIFF=$(gh api "repos/${REPO}/pulls/${TARGET_PR}" -H "Accept: application/vnd.github.v3.diff" 2>/dev/null \
| head -c 24000 || true)
# Random delimiter prevents PR-diff lines that equal a fixed token from
# terminating the multiline GITHUB_OUTPUT early (step-output injection).
DELIM="DSCR_$(openssl rand -hex 8)"
{
echo "prompt<<EOF"
echo "prompt<<${DELIM}"
cat <<PROMPT
You are the free-tier DeepSeek-CI peer reviewer for termux-monorepo. Concise. ONE response.
Target PR #${TARGET_PR}.
Check: correctness, security (no secrets/PII), AGENTS.md rules, minimal diff.
Diff (truncated):
${DIFF}
PROMPT
echo "EOF"
echo "${DELIM}"
} >> "$GITHUB_OUTPUT"
Comment on lines 89 to 93

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Random GITHUB_OUTPUT delimiter fixes only half of the injection path

The randomized DSCR_<hex> delimiter does prevent a PR diff line equal to a fixed EOF from closing the multiline step output early. The inner cat <<PROMPT heredoc is safe for a different reason: heredoc terminators are matched at parse time on literal script lines, not on the expanded contents of ${DIFF}, so diff text containing a line PROMPT cannot end it. Worth noting the prompt is still interpolated into a step output that is later passed as an action input, so the diff content itself remains attacker-influenced text (mitigated in this PR by the new non-fork/non-draft guard).

(Refers to lines 80-93)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


- name: OmniRoute review (peer)
Expand Down Expand Up @@ -118,6 +127,7 @@ jobs:
github.event_name == 'workflow_dispatch' &&
(inputs.enable_deepseek_webwrapper == true || vars.DEEPSEEK_WEBWRAPPER_ENABLED == '1')
runs-on: ubuntu-latest
timeout-minutes: 10
permissions:
contents: read
steps:
Expand All @@ -126,12 +136,12 @@ jobs:
persist-credentials: false

- name: Setup Node.js (PoW WASM solver)
uses: actions/setup-node@v4
uses: actions/setup-node@39370e3970a6d050c480ffad4ff0ed4d3fdee5af # v4.1.0
with:
node-version: '20'

- name: Setup Python
uses: actions/setup-python@v5
uses: actions/setup-python@0b93645e9fea7318ecaed2b4117511b8a72f1f5 # v5.3.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Opt-in DeepSeek smoke-test job always fails because a pinned action reference is malformed

The Python setup step points at a commit reference that is one character short of a valid 40-character commit id (actions/setup-python@0b93645e9fea7318ecaed2b4117511b8a72f1f5 at .github/workflows/deepseek-ci.yml:144), so the job cannot start and fails immediately whenever it is triggered.
Impact: The opt-in DeepSeek web-wrapper smoke test can never run; every manual dispatch of it errors out.

Action reference resolution requires a full 40-hex commit SHA

0b93645e9fea7318ecaed2b4117511b8a72f1f5 is 39 hex characters. GitHub Actions resolves uses: owner/repo@ref as a tag, branch, or full commit SHA; a truncated SHA is not accepted and the run fails with "Unable to resolve action ... unable to find version". The v5.3.0 SHA is 0b93645e9fea7318ecaed2b4117511b8a72f1f5f (trailing f missing). The neighbouring actions/setup-node pin at .github/workflows/deepseek-ci.yml:139 is correctly 40 characters.

Suggested change
uses: actions/setup-python@0b93645e9fea7318ecaed2b4117511b8a72f1f5 # v5.3.0
uses: actions/setup-python@0b93645e9fea7318ecaed2b4117511b8a72f1f5f # v5.3.0
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

with:
python-version: '3.11'

Expand Down
8 changes: 4 additions & 4 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,13 @@ cli-synthegration/workspace/provenance/
cli-synthegration/codex/

# === DeepSeek web-wrapper opt-in CI smoke test (transient; $RUNNER_TEMP only) ===
# Only ignore runtime session state — never ignore tracked sources such as
# deepcli/pow_solver.js (required for PoW). Bare patterns like pow_solver.js
# or session.json would hide legitimate tracked files if re-added.
deepseek-webwrapper-home/
deepseek_output.json
deepcli/session.json
deepcli/pow_solver.js
session.json
cookies_2.json
pow_solver.js
deepcli/cookies_2.json
Comment on lines +127 to +133

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Session-state files outside one folder are no longer excluded from version control

The catch-all exclusion for session state files was narrowed to a single folder (removal of the bare session.json entry at .gitignore:131), so session files produced elsewhere in the repo are now eligible to be committed.
Impact: Exported session data containing conversation history and tokens can be accidentally committed, which the repo's agent rules explicitly forbid.

Which paths lose protection

AGENTS.md hard rule: "No Class 3/4 artifacts in git (session stores, browser profiles, tokens)". The old bare patterns session.json and cookies_2.json matched at any depth; the new list only keeps deepcli/session.json and deepcli/cookies_2.json (.gitignore:129-133). cookies_2.json is still covered by the pre-existing *cookies*.json pattern (.gitignore:64), but nothing else covers session.json. Tools such as cli-synthegration/synthegration_index.py:193 read/write session.json inside live-exported session directories, which would now be untracked-but-not-ignored. The stated goal (stop hiding tracked deepcli/pow_solver.js) only required removing the pow_solver.js patterns, not the session ones.

Suggested change
# Only ignore runtime session state — never ignore tracked sources such as
# deepcli/pow_solver.js (required for PoW). Bare patterns like pow_solver.js
# or session.json would hide legitimate tracked files if re-added.
deepseek-webwrapper-home/
deepseek_output.json
deepcli/session.json
deepcli/pow_solver.js
session.json
cookies_2.json
pow_solver.js
deepcli/cookies_2.json
# Only ignore runtime session state — never ignore tracked sources such as
# deepcli/pow_solver.js (required for PoW). Bare patterns like pow_solver.js
# would hide legitimate tracked files if re-added.
deepseek-webwrapper-home/
deepseek_output.json
deepcli/session.json
deepcli/cookies_2.json
session.json
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +127 to +133

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟨 Repo-wide ignore for session state files removed, risking commit of session/token artifacts

The bare session.json ignore pattern was dropped (.gitignore:131 removed), leaving only deepcli/session.json and deepcli/cookies_2.json. Session manifests written elsewhere (e.g. by cli-synthegration/synthegration_index.py:193) are no longer ignored and can be committed. AGENTS.md hard rule states: "No Class 3/4 artifacts in git (session stores, browser profiles, tokens)".

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


# === Build artifacts and caches ===
workspace/maxc/target/
Expand Down
6 changes: 6 additions & 0 deletions deepcli/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "deepcli-pow-solver",
"private": true,
"type": "module",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Adding type:module scopes ESM to the whole deepcli directory

The new deepcli/package.json marks every .js file under deepcli/ (excluding nested node_modules, which carry their own manifests) as ESM. Currently only deepcli/pow_solver.js and deepcli/extract-token.js are tracked JS and both already use import, so nothing breaks today; but any future CommonJS helper dropped into this directory will fail at load unless renamed .cjs.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

"description": "ESM context so pow_solver.js (import syntax) runs under Node 20 without .mjs rename"
}
Loading