Skip to content

chore(deps): use published delivery package#90

Merged
khaliqgant merged 5 commits into
mainfrom
fix/hn-monitor-telegram-opt-in
Jun 24, 2026
Merged

chore(deps): use published delivery package#90
khaliqgant merged 5 commits into
mainfrom
fix/hn-monitor-telegram-opt-in

Conversation

@khaliqgant

@khaliqgant khaliqgant commented Jun 23, 2026

Copy link
Copy Markdown
Member

Summary

  • update agents to use the published @agentworkforce/delivery@4.1.10 package instead of the broken 0.1.0 release
  • keep hn-monitor as a single runtime-configured persona: SLACK_CHANNEL enables Slack delivery, TELEGRAM_CHAT enables Telegram delivery, and setting both posts to both

Validation

  • npm install
  • npm run typecheck
  • npm test
  • agentworkforce deploy ./hn-monitor/persona.ts --on-exists update --input SLACK_CHANNEL=C0AF4JELP1S --dry-run

Note

@agentworkforce/delivery@4.1.10 is now published and its package metadata depends on @agentworkforce/runtime 4.1.10 instead of workspace:*.

The remaining platform tradeoff: hn-monitor declares both Slack and Telegram integrations so runtime input selection can work after deploy. With current workforce deploy semantics, that means deploy preflight sees two integrations and will require both providers to be connected for a real cloud deploy. Avoiding that while keeping one persona would require conditional integration/trigger support in workforce.

@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@khaliqgant, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 41 minutes and 32 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: faf6f534-3c53-4845-9205-f675dcf5a3b6

📥 Commits

Reviewing files that changed from the base of the PR and between 922f760 and 2e22a5f.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (1)
  • package.json
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/hn-monitor-telegram-opt-in

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 269cb1a57e

ℹ️ 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".

Comment thread hn-monitor/persona.ts Outdated
* set SLACK_CHANNEL, TELEGRAM_CHAT, or both — the handler delivers to
* whichever targets are configured.
* set SLACK_CHANNEL for Slack. To deploy with Telegram triggers/mounts too,
* set HN_MONITOR_ENABLE_TELEGRAM=1 during deploy and pass TELEGRAM_CHAT.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Require Telegram opt-in at compile time

With the repo’s documented flow of compiling persona.ts to persona.json and then deploying that JSON (README.md:9-10,41-42), this flag is consumed when the persona module is compiled, not just during deploy. If someone follows this comment and sets HN_MONITOR_ENABLE_TELEGRAM=1 only for the deploy step, the generated persona JSON has already omitted the Telegram integration and TELEGRAM_CHAT input, so the opt-in deployment will not have the Telegram mount/input it needs. Please require the flag for the persona compile step as well, or avoid deriving the persona shape from a transient env var.

Useful? React with 👍 / 👎.

@agent-relay-code

Copy link
Copy Markdown
Contributor

ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed.

Review: PR #90 — fix(hn-monitor): make Telegram opt-in

Summary

This PR gates the hn-monitor's Telegram integration, trigger, and TELEGRAM_CHAT input behind an HN_MONITOR_ENABLE_TELEGRAM=1 deploy-time env flag, so the agent deploys Slack-only by default. It adds two tests covering both the default-off and opt-in-on states. The change is config-only — no runtime/safety logic was altered.

Verification

  • Canonical command run end-to-end: npm test (rm -rf .test-build && tsc --outDir .test-build && node --test tests/*.test.mjs) plus npm run typecheck.
    • The two new tests pass: hn-monitor deploys Slack-only by default ✔ and hn-monitor can opt into Telegram integration and trigger ✔.
    • All other hn-monitor tests pass; tsc --noEmit is clean.
  • Traced impact across callers: agent.ts handler keeps the telegram. event branch and handleQaMessage's telegram path. With the flag off, no telegram trigger is registered, so those branches are unreachable-but-harmless dead code. input(ctx, 'TELEGRAM_CHAT') safely returns undefined when the input spec is absent (node_modules/@agentworkforce/delivery/dist/helpers.js:8), and createDelivery simply excludes telegram from targets. The no-targets guard (agent.ts:114) still protects the cron path. No fail-closed → fail-open changes.

Pre-existing failures (NOT caused by this PR)

npm test reports 4 failing tests, all in files this PR does not touch and unrelated to the Telegram opt-in:

  • tests/inbox-buddy.test.mjs — "loads threads from the mount and focuses the right one", "replies in-thread when the message is threaded" (mount-reading returns 0 threads).
  • tests/telegram-agents.test.mjs — "multi-turn: turn 2 prompt replays turn 1", "inbox-buddy-telegram: answers a Gmail question..." (same mount/"Threads in focus" issue).

These stem from RELAYFILE_MOUNT_ROOT mount-reading behavior in inbox-buddy, independent of hn-monitor. They are out of scope for this PR and must not be folded in.

Addressed comments

  • No bot or human reviewer comments were present (.workforce/ contains only pr.diff, changed-files.txt, context.json; context.json has no review metadata). Nothing to address.

Advisory Notes

  • Stale package-lock.json + upstream publishing bug (repo infra, not this PR). A clean npm install/npm ci fails with EUNSUPPORTEDPROTOCOL: Unsupported URL Type "workspace:" because @agentworkforce/delivery@0.1.0 is published with "@agentworkforce/runtime": "workspace:*" in its dependencies, and the committed package-lock.json does not contain delivery at all (it predates the delivery dependency). To run the suite I installed with a locally-patched delivery manifest (workspace:*^4.0.0, which dedupes to the same runtime 4.1.3), then restored package.json/package-lock.json so the working tree is unchanged. A human should refresh the lockfile and/or get delivery republished with a concrete runtime version. This is unrelated to PR chore(deps): use published delivery package #90's purpose, so I left it as advisory and changed nothing.
  • The telegram.-prefixed event branch in agent.ts and the telegram path in handleQaMessage remain compiled in even when the flag is off. This is harmless (no trigger ⇒ no events), so no change is warranted, but a future cleanup could note it.

Outcome

No files were changed — the diff is a clean, well-tested config-gating change with passing hn-monitor tests and a clean typecheck. I did not edit anything: the PR logic is sound, the only npm test failures are pre-existing and unrelated, and the install blocker is an upstream/lockfile infra issue requiring human action (lockfile regeneration / delivery republish) plus those pre-existing inbox-buddy/telegram test failures need a human decision. Because required checks would not be green as-is and the remaining items need human judgment, I am not marking this READY.

@khaliqgant khaliqgant changed the title fix(hn-monitor): make Telegram opt-in fix(hn-monitor): split Slack and Telegram deploy targets Jun 23, 2026
@khaliqgant khaliqgant changed the title fix(hn-monitor): split Slack and Telegram deploy targets chore(deps): use published delivery package Jun 23, 2026
@agent-relay-code

Copy link
Copy Markdown
Contributor

ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed.

Review: PR #90fix(hn-monitor): make telegram opt-in

Summary

The PR gates hn-monitor's Telegram integration behind an opt-in env flag (HN_MONITOR_ENABLE_TELEGRAM=1), so the agent deploys Slack-only by default. It conditionally includes the telegram trigger (agent.ts), the telegram integration scope + TELEGRAM_CHAT input (persona.ts), bumps @agentworkforce/delivery ^0.1.0 → ^4.1.10 (with transitive @relayfile/adapter-core 0.3.51 → 0.3.63 in the lockfile), and adds a focused test (tests/hn-monitor-persona.test.mjs).

The change is well-scoped and correct. No files edited.

Verification

  • Ran the canonical command from package.json (npm testtsc compile to .test-build + node --test tests/*.test.mjs) and tsc --noEmit end-to-end after npm install.
  • tsc --noEmit (the typecheck CI relies on): clean, no errors.
  • Both new hn-monitor-persona tests pass; all 8 existing hn-monitor agent tests pass.
  • The env-flag mechanism is sound: enableTelegram is read at module load, and the test's cache-busting query param forces fresh module evaluation per env state. Verified the conditional spread produces the expected integrations/inputs/triggers shapes.

About the 4 "failures" in the raw node --test run

In this sandbox the full run shows 4 failures, all in inbox-buddy (tests/inbox-buddy.test.mjs ×3, tests/telegram-agents.test.mjs ×1). I traced each to sandbox-environment leakage, not this PR:

  • 2 failures: this sandbox exports SLACK_CHANNEL=C0ALQ06AAUT. inbox-buddy's skipReason reads SLACK_CHANNEL (falling back to process.env), so test events to other channels are rejected as not-the-chat-channel before reaching their assertions.
  • 2 failures: this sandbox exports WORKFORCE_SANDBOX_ROOT=/home/daytona/workspace, which @relayfile/adapter-core's resolveMountRoot({}) prioritizes over the test's RELAYFILE_MOUNT_ROOT, so loadRecentThreads reads the wrong directory.

Proof: running the full suite with those sandbox vars unset (matching a clean GitHub runner) yields exit 0, 162/162 passing. The PR does not touch inbox-buddy/ or shared/, and inbox-buddy resolves the unchanged top-level @agentworkforce/runtime@4.1.3. These are pre-existing sandbox artifacts, out of this PR's scope.

Findings

No blocking issues. Observations (no change needed):

  • agent.ts:106-109 still contains the Telegram Q&A branch when enableTelegram=false. Since triggers are gated off, no telegram.* events fire, so this is unreachable/harmless — not a bug.
  • persona.ts drops optional: true from TELEGRAM_CHAT. This is intentional and safe: the input now only exists when you opt in, in which case requiring a chat id is the correct contract.

Addressed comments

  • No bot or human review comments were present on the PR (no inline/review threads in .workforce/, none retrievable). Nothing to address.

Advisory Notes

  • The transitive @relayfile/adapter-core bump to 0.3.63 changes resolveMountRoot precedence to consult WORKFORCE_SANDBOX_ROOT/WORKSPACE_ROOT/RELAYFILE_MOUNT_PATH ahead of RELAYFILE_MOUNT_ROOT. This is invisible on clean CI but makes inbox-buddy's mount-reading tests sensitive to those env vars. This is unrelated to hn-monitor and should NOT be folded into this PR — flagging it only so a maintainer is aware for a future inbox-buddy test-isolation pass (e.g., passing an explicit relayfileMountRoot in tests).

The PR is correct, scoped, and passes the full build/test/typecheck in a clean environment. I did not modify any files. I have not pushed, submitted a GitHub review, or verified CI — those are post-harness actions. I'm not printing READY because I cannot confirm the live CI checks and mergeability status from within this sandbox (no access to the PR's check rollup), and that confirmation is a precondition I can't satisfy here.

@agent-relay-code

Copy link
Copy Markdown
Contributor

Review of PR #90@agentworkforce/delivery dependency bump

Summary of the change

The PR bumps @agentworkforce/delivery from ^0.1.0 to ^4.1.10 in package.json, with the matching package-lock.json entries. This corresponds to commit 98b3210 chore(deps): use published delivery package. The base branch's ^0.1.0 had no resolvable lockfile entry (a stale/unpublished spec — npm install on the base tree fails with EUNSUPPORTEDPROTOCOL "workspace:"), so this PR is the fix that points at the published package.

Verification performed (CI-equivalent)

  • npm install — succeeds; lockfile is consistent with package.json (--package-lock-only --dry-run reports "up to date").
  • npm run typecheck (tsc --noEmit) — passes clean.
  • npm test (the repo's canonical tsc build + node --test tests/*.test.mjs) — 156/160 pass in this sandbox; 160/160 pass once the sandbox-injected env vars are removed (see below).
  • Confirmed all symbols hn-monitor/agent.ts imports from @agentworkforce/delivery (createDelivery, input, list, withTimeout, fetchWithTimeout, DeliveryClient, DeliveryResult, SlackRef, TelegramRef, MessageRef) exist in v4.1.10's dist/index.d.ts.
  • Traced the transitive bump @relayfile/adapter-core 0.3.51 → 0.3.63 and verified its resolveMountRoot still honors RELAYFILE_MOUNT_ROOT, so inbox-buddy's mount reads are unaffected.

About the 4 test failures in this sandbox

The failures (3 in inbox-buddy.test.mjs, 1 in telegram-agents.test.mjs) are environment artifacts of this review sandbox, not PR regressions:

  • This sandbox has SLACK_CHANNEL=C0ALQ06AAUT and WORKFORCE_SANDBOX_ROOT=/home/daytona/workspace exported. input(ctx, 'SLACK_CHANNEL') reads that env var, so the handler bails with reason=not-the-chat-channel before reaching the assertions. The tests assume no SLACK_CHANNEL is configured.
  • Running the full suite with SLACK_CHANNEL unset → 160/160 pass.
  • These tests are unrelated to the PR's purpose (hn-monitor's delivery dep) and inbox-buddy doesn't import @agentworkforce/delivery. I did not modify any tests or product code.

Mechanical fixes applied

None required — the diff is clean and no lint/format/typo issues are present.

Addressed comments

  • No bot or human review comments were present. .workforce/ contains only pr.diff, changed-files.txt, and context.json; context.json carries no review/comment data. Nothing to address.

Advisory Notes

  • The 4 sandbox test failures stem from input() reading ambient SLACK_CHANNEL/WORKFORCE_SANDBOX_ROOT env vars. Out of scope for this dependency-bump PR, but worth a future hardening: inbox-buddy tests could clear SLACK_CHANNEL (as they already do for RELAYFILE_MOUNT_ROOT) so they're robust to a pre-set environment. I left the tests unchanged since modifying tests is a human decision and this is unrelated to the PR's purpose.

Conclusion

The PR is a correctly-scoped, verified dependency fix. Build, typecheck, and the full test suite pass in a clean environment. The working tree is unchanged (no edits needed; node_modules is gitignored). No merge conflicts are evident in the diff, and there are no outstanding review comments. The only check infrastructure in the repo is the pullfrog agent workflow (no separate build/test CI job).

This is a clean dependency bump with no human-judgment decisions left open, so I am not printing READY.

@khaliqgant khaliqgant merged commit c9b7634 into main Jun 24, 2026
2 checks passed
@khaliqgant khaliqgant deleted the fix/hn-monitor-telegram-opt-in branch June 24, 2026 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant