2 Commits

Author SHA1 Message Date
tuxedomm
fbed6beac3 refactor: split Execute into Build + Execute with explicit IO and keychain injection (#371)
* refactor(cmd): split Execute into Build with IO/Keychain injection

Introduce a public cmd.Build entry point so external consumers (cli-server,
MCP server, other embedders) can assemble the full CLI command tree without
going through os.Args or the platform keychain. Build takes an
InvocationContext plus functional BuildOptions:

  * WithIO(in, out, errOut) — inject custom streams; terminal detection
    is derived from the input's underlying *os.File when present.
  * WithKeychain(kc)        — swap the credential store.
  * HideProfile(bool)       — registered later in cmd.HideProfile.

The existing Execute() keeps using the internal buildInternal (which
still returns the Factory so error handling can attribute exit codes),
and SetDefaultFS replaces the global VFS implementation at startup.

Hardening applied up front:

  * cmdutil.NewIOStreams(in, out, errOut) centralizes terminal detection
    so SystemIO() and WithIO share one path.
  * cmdutil.NewDefault normalizes partial IOStreams — callers may pass
    &IOStreams{Out: buf} without tripping nil-writer panics in the
    RoundTripper warnings, Cobra, or the credential provider.
  * Build guards against nil functional options.
  * An API contract test (cmd/build_api_test.go) exercises Build +
    WithIO + WithKeychain + HideProfile + SetDefaultFS so the public
    surface is reachable by deadcode analysis.

Change-Id: I7c895e6019817401accbde2db3ef800da40ad319

* feat(schema): filter methods by strict mode in schema output

When strict mode is active, schema output now excludes methods that
are incompatible with the forced identity. This applies to both
pretty and JSON output formats at the resource and method levels.

Change-Id: I39647d5578466c3e23dc545bfb917ae075203ad7

* refactor: centralize strict-mode as flag registration

Change-Id: Iec11151c5002c2f58a8aa067d08747db2e4d2d8c

* fix(cmd): align strict-mode completion and build context; drop dead register shims

Thread a context.Context through RegisterShortcuts, RegisterServiceCommands,
and service.registerService/Resource/Method by introducing explicit
*WithContext variants. Pass that context into NewCmdServiceMethodWithContext
so shortcut and service command construction can honor cancellation and
strict-mode pruning consistently.

Also drop the context-less registerMethod and registerResource shims —
they became unreachable once the WithContext variants took over, and
were the source of new deadcode warnings. registerService is retained
because service_test.go still calls it directly.

Change-Id: I3fe5673aed663c7383bbbc5b0ae94d1f3491f22d

* refactor(cmd): hide --profile in single-app mode via build option

- GlobalOptions gains HideProfile; RegisterGlobalFlags stays pure and reads
  the policy off the struct. No boolean-trap parameter, one call per site.
- buildConfig holds GlobalOptions inline so HideProfile(bool) BuildOption
  mutates it directly. buildInternal stays a pure assembly function and
  requires callers to supply WithIO — no implicit os.Std* fallback.
- Add WithIO BuildOption (wrapping raw io.Reader/Writer with automatic
  *os.File TTY detection); Execute injects streams explicitly and decides
  profile visibility via HideProfile(isSingleAppMode()).
- installTipsHelpFunc force-shows hidden root flags while rendering the
  root command's own help, so single-app users still discover --profile
  via lark-cli --help without it polluting subcommand helps.

Change-Id: I7755387e993992ca969e0a4a6f54441cc1993eef

* feat(transport): extension abort hook and shared base transport

Two transport-layer changes bundled because both reshape the base
round-tripper contract used by the HTTP client, the Lark SDK client,
and the in-process updater.

1. Extension abort hook (PreRoundTripE).

   Extensions implementing exttransport.AbortableInterceptor can now
   return an error from PreRoundTripE to skip the built-in chain. The
   post hook still fires with (nil, reason) so extensions can unwind
   resources. extensionMiddleware captures the provider name so the
   returned *AbortError carries attribution.

2. Shared base transport to stop RPC leak.

   util.NewBaseTransport cloned http.DefaultTransport on every call, so
   each cmdutil.Factory produced a fresh *http.Transport whose
   persistConn readLoop/writeLoop goroutines lingered until
   IdleConnTimeout (~90s). Invisible in a single-process CLI, but the
   fork is consumed by cli-server where each RPC request constructs a
   new Factory, causing linear memory + goroutine growth under load.

   Replace NewBaseTransport with SharedTransport — returns
   http.DefaultTransport (the stdlib-wide singleton) by default, and
   a cached proxy-disabled clone only when LARK_CLI_NO_PROXY is set.
   Return type is http.RoundTripper to discourage in-place mutation of
   the shared instance. FallbackTransport is kept as a thin
   *http.Transport wrapper so existing callers in internal/auth and
   internal/cmdutil transport decorators (which were already on the
   singleton path) do not have to migrate.

   Leak-site migrations: factory_default.go (HTTP + SDK base) and
   update.go now call SharedTransport directly.

Change-Id: Ia82462134c5c5ee838be878b887860f41446a235

* fix: unblock Build() zero-opts path and sidecar demo build

Two regressions surfaced on refactor/build-execute-split:

1. cmd.Build(ctx, inv) without WithIO panicked at rootCmd.SetIn/Out/Err
   because cfg.streams stayed nil — NewDefault normalized internally
   but cmd/build.go never saw the normalized value. Default cfg.streams
   to cmdutil.SystemIO() before the root command wires them, and add a
   TestBuild_NoOptions regression guard.

2. sidecar/server-demo/main.go still called cmdutil.NewDefault(inv),
   so `go build -tags authsidecar_demo ./sidecar/server-demo` failed
   with "not enough arguments". Pass nil for the new streams parameter
   to preserve the prior behavior (NewDefault substitutes SystemIO).

Change-Id: I20227b2355cde7d19e22eba3eb841c6d8611e8a7
2026-04-21 14:48:40 +08:00
liangshuo-1
8db4528269 feat: add strict mode identity filter, profile management and credential extension (#252)
* feat: add strict mode identity filter, profile management and credential extension

Port changes from feat/strict-mode-identity-filter_3 branch:
- Add strict mode for identity filtering and configuration
- Add profile management commands (add/list/remove/rename/use)
- Add credential extension framework (registry, env provider)
- Add VFS abstraction layer
- Refactor factory default and client options
- Update shortcuts to use new credential and validation patterns

Change-Id: I8c104c6b147e1901d94aefcefe35a174932c742b
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* chore: go mod tidy

Change-Id: I0f610ccea6bc874248e84c24770944a3071dcc57
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: fix test failures from credential provider migration

- Remove unused TAT stub registrations in api and service tests
  (CredentialProvider manages tokens, SDK no longer calls TAT endpoint)
- Update strict mode integration test: +chat-create now supports user
  identity, so it should succeed under strict mode user

Change-Id: Iab51c2e12a97995e0b95dcd71df212d2d1f76570
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: migrate remaining os calls to internal/vfs

Replace direct os.Stat/Open/MkdirAll/OpenFile/Remove/ReadDir/UserHomeDir
with vfs equivalents in shortcuts/minutes, shortcuts/drive, and
internal/keychain. Add ReadDir to the vfs interface and OsFs implementation.

Change-Id: I8f97e5fb3e1731b4684d276644fcb10fae823067

* fix: resolve gofmt and goimports formatting issues

Change-Id: If61578631f5698f7ca2d9a946ca59753651463fb

* feat: add Flag.Input support for @file and stdin input sources

Add framework-level support for reading flag values from files (@path)
or stdin (-), solving the fundamental problem of passing complex text
(markdown, multi-line content) via CLI arguments where shell escaping
breaks content. Closes #239, fixes #163.

- Add File/Stdin constants and Input field to Flag struct
- Add resolveInputFlags() in runner pipeline (pre-Validate)
- Support @@ escape for literal @ prefix
- Guard against multiple stdin consumers
- Auto-append "(supports @file, - for stdin)" to help text
- Apply to: docs +create/+update --markdown, im +messages-send/+reply
  --text/--markdown/--content, task +comment --content,
  drive +add-comment --content

Change-Id: I305a326d972417542aeadd70f37b74ea456461ef
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: fix pre-existing test failures in task, minutes, and registry

- task/minutes: remove unused tenant_access_token httpmock stubs
  (TestFactory's testDefaultToken provides tokens directly, so the
  HTTP stub was never consumed and failed verification)
- registry: fix hasEmbeddedData() to check for actual services instead
  of just byte length (meta_data_default.json has empty services array)

Change-Id: Ic7b5fc7f9de09137a7254fe1ddf47d24ade40587
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: suppress nilerr lint for intentional nil returns

Both cases intentionally return nil on error for graceful degradation:
- profile list: show friendly message when config is not initialized
- service: skip scope check when token resolution fails

Change-Id: I7285c37277c9b0361a421ab00359244c2cd150b3
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: address CodeRabbit review feedback

- runner.go: fail fast when Input is used on non-string flags
- remote_test.go: rename hasEmbeddedData → hasEmbeddedServices
- profile/list.go: add omitempty to optional JSON fields
- service.go: surface context cancellation errors in scope check

Change-Id: I7072d41f8c711b4b37c542e32dfd8150f42b13c0
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: tighten credential resolution and profile flows

Change-Id: I83f6d424540eab9b1708944b9b6e26e8477cc60d

* refactor: centralize identity hint resolution

Change-Id: I38d5f98160b92adb62dc929ae73697ae5b3d64f8

* fix: surface unverified extension identities

Change-Id: Ia86d9bd19add9010176339ec4cc89deb033f5b4f

* fix: honor runtime credential sources in config views

Change-Id: I40b2ffedc5c1db5e08e86b9472ea2b84fa02bb29

* fix: prefer runtime values in config show commands

Change-Id: I5663a53e147577f0f1f533f67d12bea504e6b839

* Revert "fix: prefer runtime values in config show commands"

This reverts commit 4f9db3a227.

* Revert "fix: honor runtime credential sources in config views"

This reverts commit b3bfd526c5.

* fix: harden profile flows and credential boundaries

Change-Id: Ica61cd2730a639f71516cb1b237a639cb6511f7a

* fix: optimize profile and config inspection for agents

Change-Id: I19c368102f19654952638180ab947788a6971563

* refactor: unify credential env contracts

Change-Id: I0ff2c0a650ea53589a0626333e8f6e628ef10a54

* docs: expand AGENTS guidance

Change-Id: I289027dfd364c92205012feef6f05037066c035b

* fix: resolve regression bugs found during PR #252 review

- im: fix double SafeInputPath in resolveLocalMedia → uploadImageToIM/
  uploadFileToIM chain that rejected all local image/file uploads
- credential: stop writing plain-text warnings to stderr, preserving
  JSON envelope contract for AI agent consumers
- profile add: reject duplicate app-id to prevent keychain credential
  collisions across profiles
- profile rename: exclude self when checking name uniqueness so renaming
  to own appId works correctly
- config: replace bare fmt.Errorf with output.Errorf in save-failure
  paths (default_as, strict_mode ×2, profile add)
- factory: remove unused resolveDefaultAs method (lint)

Change-Id: I6aa0d064414016f367f1edb08dd0604adf7bf13d
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: remove flaky TestColdStart_UsesEmbedded (race in registry)

The test triggers a data race: resetInit() writes package globals while
a background goroutine from a previous test may still be reading them.
The embedded-data path is covered by other tests.

Change-Id: I7a0c3bf85a9fb337b9279c9053697f40a0c0a0d4
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: type-strengthen Brand and DefaultAs across credential chain

Replace raw string fields with typed enums for compile-time safety:
- extension/credential: add Brand and Identity named types
- internal/core: AppConfig.DefaultAs and CliConfig.DefaultAs → Identity
- internal/credential: Account.DefaultAs and IdentityHint.DefaultAs → core.Identity

The full data flow is now typed end-to-end:
  extcred.Brand → core.LarkBrand (named-type cast)
  extcred.Identity → core.Identity (named-type cast)

No string intermediaries, no implicit conversions.

Change-Id: I715b3b3f033fcb624010f1af9619e3562740ef08
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* style: fix gofmt alignment in extension/credential/types.go

Change-Id: Ibfac0703a5a28f3c6ba4a47bf40696028d0f3b90
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: remove file/stdin input support from task comment content flag

Change-Id: If49704ca4612465a23bd30b755d6e72a35fc2349
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor(cmdutil): remove dead code autoDetectIdentity

autoDetectIdentity() is only called from tests, never from production
code. Remove it along with its 3 test cases to reduce surface area
before the upcoming ctx propagation refactor.

Change-Id: I35a188860f17656f3e1fe9874f87f284985ae196

* refactor(cmdutil): add ctx parameter to resolveIdentityHint

Private method resolveIdentityHint now accepts context.Context and
passes it to CredentialProvider.ResolveIdentityHint instead of using
context.Background(). The caller (ResolveAs) still uses
context.Background() temporarily until its own signature is updated.

Change-Id: I14634a4e0dc1d657d56936ba61a7b7a206da8ac4

* refactor(cmdutil): add ctx parameter to ResolveStrictMode

ResolveStrictMode now accepts context.Context and passes it to
CredentialProvider.ResolveAccount instead of using context.Background().

Callers in cobra RunE pass cmd.Context(); callers outside RunE
(cmd/root.go startup, tests) use context.Background() explicitly.

Change-Id: I31be48e548ac5ac5640a65f3bfdde4a53ed1dc7e

* refactor(cmdutil): add ctx parameter to CheckStrictMode

CheckStrictMode now accepts context.Context and forwards it to
ResolveStrictMode. Callers pass cmd.Context() (cobra RunE) or
opts.Ctx (APIOptions/ServiceMethodOptions).

Change-Id: I47888519d4cae8c94054771c32aff075565a8cdc

* refactor(cmdutil): add ctx parameter to ResolveAs

ResolveAs now accepts context.Context as first parameter and forwards
it to ResolveStrictMode and resolveIdentityHint. This completes the
ctx propagation chain: all Factory methods that call
CredentialProvider now receive ctx from cobra cmd.Context().

No more context.Background() calls remain in factory.go for
credential provider operations.

Change-Id: I6d10b6350e3b149470660de3e7855614314e8b29

* test: fix gofmt in cmdutil factory tests

Change-Id: I4a87d5a815b959f14cc4371b73dee4aae106932f

* fix: remove file/stdin input support from im send/reply and drive comment

The Input (file/stdin) feature is not yet ready for these flags:
- im send/reply: --content, --text, --markdown
- drive add-comment: --content

Retained only in doc create/update where markdown from file is essential.

Change-Id: I582b6349528fccb639ad9edc84650cca3b68535c
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: liushiyao <liushiyao.1206@bytedance.com>
2026-04-07 15:21:14 +08:00