Skip to content

[codex] Respect --brand lark in config init --new#4

Open
jnuyao wants to merge 1 commit intolarksuite:mainfrom
jnuyao:codex/honor-lark-brand-init
Open

[codex] Respect --brand lark in config init --new#4
jnuyao wants to merge 1 commit intolarksuite:mainfrom
jnuyao:codex/honor-lark-brand-init

Conversation

@jnuyao
Copy link
Copy Markdown

@jnuyao jnuyao commented Mar 28, 2026

Summary

  • respect --brand lark in config init --new
  • preserve the requested Lark brand in the saved config and JSON output
  • surface the existing Lark endpoint retry message during app registration
  • add a regression test for the --new --brand lark path

Root cause

config init --new hard-coded core.BrandFeishu when entering the create-app flow, so the requested brand never reached the setup URL generation path.

Testing

  • go test ./cmd/config
  • go test ./internal/auth ./internal/core
  • go test ./...
    • this still fails in internal/registry on pre-existing tests:
      • TestColdStart_UsesEmbedded
      • TestCacheHit_WithinTTL

@jnuyao jnuyao marked this pull request as ready for review March 28, 2026 08:21
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 30, 2026

CLA assistant check
All committers have signed the CLA.

chanthuang added a commit that referenced this pull request Apr 15, 2026
- Escape sigID in BuildSignatureHTML to prevent HTML attribute injection
- Add SSRF protection to downloadSignatureImage: enforce https, validate
  host, use context timeout (30s), do not send Bearer token — aligned
  with existing downloadAttachmentContent security practices
- Add nil guard for child in removeMIMEPartByCID loop
- Expose insert_signature/remove_signature in supported_ops flat list
- Fix +send doc: remove "引用块之前" (new emails have no quote block)

Note: CodeRabbit #4 (splitAtSignature boundary bug) is a false positive —
FindMatchingCloseDiv returns the position AFTER </div>, so slice
boundaries are correct.

Change-Id: I5363d8a070e2e01200efdcb8235832627d1ff9a5
Co-Authored-By: AI
chanthuang added a commit that referenced this pull request Apr 15, 2026
- Escape sigID in BuildSignatureHTML to prevent HTML attribute injection
- Add SSRF protection to downloadSignatureImage: enforce https, validate
  host, use context timeout (30s), do not send Bearer token — aligned
  with existing downloadAttachmentContent security practices
- Add nil guard for child in removeMIMEPartByCID loop
- Expose insert_signature/remove_signature in supported_ops flat list
- Fix +send doc: remove "引用块之前" (new emails have no quote block)

Note: CodeRabbit #4 (splitAtSignature boundary bug) is a false positive —
FindMatchingCloseDiv returns the position AFTER </div>, so slice
boundaries are correct.

Change-Id: I5363d8a070e2e01200efdcb8235832627d1ff9a5
Co-Authored-By: AI
fangshuyu-768 added a commit that referenced this pull request Apr 30, 2026
PR #467's review thread had three substantive comments
(`fangshuyu-768`, 2026-04-21) that the prior reply messages claimed
were fixed in commit 7d4b556 — but that commit no longer exists on the
branch (lost in a rebase / squash), and the head still ships the
original buggy code. This commit makes the fixes real.

Three behavior fixes in shortcuts/doc/markdown_fix.go:

1. (#5) Tighten the type= and background-color= regex anchors. \b sits
   at any word/non-word boundary, and `-` is a non-word char, so
   `\btype=` also matched the suffix of `data-type=` — a tag like
   `<callout data-type="warning">` would emit a bogus light-yellow
   hint. Switched both regexes to `(?:^|\s)…` so a real attribute
   separator is required. The same anchor on background-color closes
   the symmetric case where a `data-background-color=` attribute
   would silently suppress the real hint.

2. (#4) WarnCalloutType is now a fence-aware line walker. Previously
   the regex ran over the entire markdown body, so a callout sample
   inside a documentation code fence (```markdown … ```) would
   generate a phantom stderr hint every time the docs mentioned the
   feature. The walker tracks fence state via the existing
   codeFenceOpenMarker / isCodeFenceClose helpers from
   docs_update_check.go, which handle both backtick and tilde fences
   per CommonMark §4.5.

3. (#3) Drop the ReplaceAllStringFunc-as-iterator pattern. The
   previous code routed callout iteration through a rewrite primitive
   whose rebuilt-string return value was discarded, then ran the same
   regex a second time inside the callback to recover the capture
   groups. New scanCalloutTagsForWarning helper uses
   FindAllStringSubmatch — one pass, no thrown-away allocation,
   intent matches the surface (read-only scan, not a mutator).

Tests: 5 new TestWarnCalloutType subtests pin each contract:

- data-type attribute does not trigger hint (#5)
- data-background-color does not suppress hint (#5, symmetric)
- callout inside backtick fence emits no hint (#4)
- callout inside tilde fence emits no hint (#4)
- callout after fence close still emits hint (#4, fence-state reset)

All 14 TestWarnCalloutType cases pass; go vet / golangci-lint
--new-from-rev=origin/main both clean.
fangshuyu-768 pushed a commit that referenced this pull request Apr 30, 2026
* feat(doc): expand callout type= shorthand into background-color and border-color

When users write <callout type="warning" emoji="📝"> without an explicit
background-color, the Feishu doc renders the block with no color. This
commit adds fixCalloutType() which maps the semantic type= attribute to
the corresponding background-color/border-color pair accepted by create-doc.

- warning → light-yellow/yellow
- info/note → light-blue/blue
- tip/success/check → light-green/green
- error/danger → light-red/red
- caution → light-orange/orange
- important → light-purple/purple

Explicit background-color or border-color attributes are always preserved.
The fix is applied via prepareMarkdownForCreate() in both +create and
+update paths, and also inside fixExportedMarkdown() for round-trip fidelity.

* refactor(doc): replace silent callout type→color injection with hint output

Per reviewer feedback (SunPeiYang996), silently rewriting user Markdown is
the wrong layer for this adaptation. The type→color mapping is not part of
the Feishu spec, and covert transforms make debugging harder.

Replace fixCalloutType() (which rewrote the Markdown) with WarnCalloutType()
which leaves the Markdown unchanged and instead writes a hint line to stderr
for each callout tag that has type= but no background-color, telling the user
the recommended explicit attributes to add:

  hint: callout type="warning" has no background-color; consider: background-color="light-yellow" border-color="yellow"

Also fixes CodeRabbit feedback: the type= regex now accepts both single-quoted
and double-quoted attribute values (type='warning' and type="warning").

* fix(doc): harden background-color detection in WarnCalloutType

CodeRabbit flagged that the previous strings.Contains(attrs,
"background-color=") check missed forms like 'background-color =
"light-red"' with whitespace around the equals sign. Replace with a
regex that tolerates optional whitespace, and add a regression test.

* fix(doc): close real review gaps left over after rebase

PR #467's review thread had three substantive comments
(`fangshuyu-768`, 2026-04-21) that the prior reply messages claimed
were fixed in commit 7d4b556 — but that commit no longer exists on the
branch (lost in a rebase / squash), and the head still ships the
original buggy code. This commit makes the fixes real.

Three behavior fixes in shortcuts/doc/markdown_fix.go:

1. (#5) Tighten the type= and background-color= regex anchors. \b sits
   at any word/non-word boundary, and `-` is a non-word char, so
   `\btype=` also matched the suffix of `data-type=` — a tag like
   `<callout data-type="warning">` would emit a bogus light-yellow
   hint. Switched both regexes to `(?:^|\s)…` so a real attribute
   separator is required. The same anchor on background-color closes
   the symmetric case where a `data-background-color=` attribute
   would silently suppress the real hint.

2. (#4) WarnCalloutType is now a fence-aware line walker. Previously
   the regex ran over the entire markdown body, so a callout sample
   inside a documentation code fence (```markdown … ```) would
   generate a phantom stderr hint every time the docs mentioned the
   feature. The walker tracks fence state via the existing
   codeFenceOpenMarker / isCodeFenceClose helpers from
   docs_update_check.go, which handle both backtick and tilde fences
   per CommonMark §4.5.

3. (#3) Drop the ReplaceAllStringFunc-as-iterator pattern. The
   previous code routed callout iteration through a rewrite primitive
   whose rebuilt-string return value was discarded, then ran the same
   regex a second time inside the callback to recover the capture
   groups. New scanCalloutTagsForWarning helper uses
   FindAllStringSubmatch — one pass, no thrown-away allocation,
   intent matches the surface (read-only scan, not a mutator).

Tests: 5 new TestWarnCalloutType subtests pin each contract:

- data-type attribute does not trigger hint (#5)
- data-background-color does not suppress hint (#5, symmetric)
- callout inside backtick fence emits no hint (#4)
- callout inside tilde fence emits no hint (#4)
- callout after fence close still emits hint (#4, fence-state reset)

All 14 TestWarnCalloutType cases pass; go vet / golangci-lint
--new-from-rev=origin/main both clean.
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