Skip to content

Add Slack diligence workflow starter - #28

Open
pratikbuilds wants to merge 2 commits into
slack-internal-processingfrom
agent/add-slack-diligence
Open

Add Slack diligence workflow starter#28
pratikbuilds wants to merge 2 commits into
slack-internal-processingfrom
agent/add-slack-diligence

Conversation

@pratikbuilds

Copy link
Copy Markdown
Collaborator

Summary

  • add starter/slack-diligence, a sourced Slack diligence snapshot workflow
  • parse a company and URL from a Slack mention or DM
  • research public evidence, draft a structured snapshot, post the result in-thread, and upload a PDF
  • reuse the shared Corbits Tag submodule introduced by PR Add Slack internal processing workflow #27

Stack

This PR is intentionally stacked on #27 (slack-internal-processing) and adds only starter/slack-diligence on top of that branch.

Verification

  • bun run typecheck (from starter/slack-diligence)
  • bun test (7 passing)
  • git diff --check

No demo video, audio, narration, PDF renders, screenshots, or generated temporary artifacts are included.

@pratikbuilds
pratikbuilds marked this pull request as ready for review August 10, 2026 06:55
@linear-code

linear-code Bot commented Aug 10, 2026

Copy link
Copy Markdown

DVRL-53

@TheGreatAxios
TheGreatAxios force-pushed the slack-internal-processing branch from 544565f to fc744bb Compare August 18, 2026 04:36
@TheGreatAxios
TheGreatAxios force-pushed the agent/add-slack-diligence branch from 650242f to 9efa6e0 Compare August 18, 2026 04:36

@TheGreatAxios TheGreatAxios left a comment

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.

code-review

Adds a Slack diligence starter: Tag with Company | URL, Exa/Firecrawl research → draft workflow, Slack snapshot card, and PDF upload via filesUploadV2.

Comment

  1. starter/slack-diligence/src/pdf.ts:15-149 — sync throw before document.end() leaves the returned promise unsettled

    renderDiligencePdf builds a complete promise from document.on("end"|"error"), then does sync PDFKit work and only then calls document.end(). If that sync work throws, complete never settles. session.ts treats PDF failure as soft-fail (still post the Slack card), but a hang defeats that path and leaves the thread in active forever.

    Could we wrap the sync render in try/catch and reject (or document.end() in a finally after marking failure) so await renderDiligencePdf always settles?

Notes (non-blocking)

  • Thread-id parsing in slack-upload.ts matches Chat SDK slack:CHANNEL:TS (parts.pop() twice) — verified against chat-sdk docs.
  • cli.ts matches slack-fact-check for subscribeOnMention: false (no registerSingleton / initialize); that is consistent for this shape.
  • bun test for package tests: 7 pass. (Untracked tmp/critique-tests/ from review probing not part of the PR.)

Checks

  • Single commit subject ≤72, empty body: Add Slack diligence workflow starter
  • Diff scope is starter/slack-diligence/** only vs origin/slack-internal-processing

@TheGreatAxios TheGreatAxios left a comment

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.

code-review (re-review)

No new commits since the prior review. Prior soft-fail note still applies; one license follow-up.

Comment

  1. starter/slack-diligence/src/pdf.ts — sync throw before document.end() can leave the returned promise unsettled

    Still true on tip 9efa6e0. Wrapping the sync render so complete always settles keeps the session soft-fail path (Slack card without PDF) from hanging the thread.

  2. package.json declares "license": "LGPL-2.1-only"

    Please drop the per-package license field to match the repo-level MIT direction in #30.

Checks

  • Diff vs origin/slack-internal-processing is still starter/slack-diligence/** only
  • Thread-id parsing in slack-upload.ts still matches Chat SDK slack:CHANNEL:TS

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.

2 participants